all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH proxmox-perl-rs 1/2] pve-rs/tfa: fix off by one trimming
@ 2021-11-12  8:58 Dominik Csapak
  2021-11-12  8:58 ` [pve-devel] [PATCH proxmox-perl-rs 2/2] pve-rs/tfa: ignore and discard incomplete u2f entries Dominik Csapak
  2021-11-12  9:23 ` [pve-devel] applied-series: [PATCH proxmox-perl-rs 1/2] pve-rs/tfa: fix off by one trimming Wolfgang Bumiller
  0 siblings, 2 replies; 3+ messages in thread
From: Dominik Csapak @ 2021-11-12  8:58 UTC (permalink / raw)
  To: pve-devel

to is the last *valid* character, and ranges end by default with one
less, so extend the range to the actual last character

this fixes an issue that we could not parse old configs with
non-padded base64 values

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 pve-rs/src/tfa/mod.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pve-rs/src/tfa/mod.rs b/pve-rs/src/tfa/mod.rs
index df192b4..44cec74 100644
--- a/pve-rs/src/tfa/mod.rs
+++ b/pve-rs/src/tfa/mod.rs
@@ -660,7 +660,7 @@ fn trim_ascii_whitespace_start(data: &[u8]) -> &[u8] {
 
 fn trim_ascii_whitespace_end(data: &[u8]) -> &[u8] {
     match data.iter().rposition(|&c| !c.is_ascii_whitespace()) {
-        Some(to) => &data[..to],
+        Some(to) => &data[..=to],
         None => data,
     }
 }
-- 
2.30.2





^ permalink raw reply	[flat|nested] 3+ messages in thread

* [pve-devel] [PATCH proxmox-perl-rs 2/2] pve-rs/tfa: ignore and discard incomplete u2f entries
  2021-11-12  8:58 [pve-devel] [PATCH proxmox-perl-rs 1/2] pve-rs/tfa: fix off by one trimming Dominik Csapak
@ 2021-11-12  8:58 ` Dominik Csapak
  2021-11-12  9:23 ` [pve-devel] applied-series: [PATCH proxmox-perl-rs 1/2] pve-rs/tfa: fix off by one trimming Wolfgang Bumiller
  1 sibling, 0 replies; 3+ messages in thread
From: Dominik Csapak @ 2021-11-12  8:58 UTC (permalink / raw)
  To: pve-devel

it can happen that we have leftover entries with non-completed challenges.
since a user cannot continue here in a sensible way, ignore and discard
them

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 pve-rs/src/tfa/mod.rs | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/pve-rs/src/tfa/mod.rs b/pve-rs/src/tfa/mod.rs
index 44cec74..d91278d 100644
--- a/pve-rs/src/tfa/mod.rs
+++ b/pve-rs/src/tfa/mod.rs
@@ -490,10 +490,13 @@ fn decode_old_entry(ty: &[u8], data: &[u8], user: &str) -> Result<TfaUserData, E
         .map_err(|err| format_err!("failed to parse json data in tfa entry - {}", err))?;
 
     match ty {
-        b"u2f" => user_data.u2f.push(proxmox_tfa_api::TfaEntry::from_parts(
-            info,
-            decode_old_u2f_entry(value)?,
-        )),
+        b"u2f" => {
+            if let Some(entry) = decode_old_u2f_entry(value)? {
+                user_data
+                    .u2f
+                    .push(proxmox_tfa_api::TfaEntry::from_parts(info, entry))
+            }
+        }
         b"oath" => user_data.totp.extend(
             decode_old_oath_entry(value, user)?
                 .into_iter()
@@ -513,12 +516,17 @@ fn decode_old_entry(ty: &[u8], data: &[u8], user: &str) -> Result<TfaUserData, E
     Ok(user_data)
 }
 
-fn decode_old_u2f_entry(data: JsonValue) -> Result<proxmox_tfa::u2f::Registration, Error> {
+fn decode_old_u2f_entry(data: JsonValue) -> Result<Option<proxmox_tfa::u2f::Registration>, Error> {
     let mut obj = match data {
         JsonValue::Object(obj) => obj,
         _ => bail!("bad json type for u2f registration"),
     };
 
+    // discard old partial u2f registrations
+    if obj.get("challenge").is_some() {
+        return Ok(None);
+    }
+
     let reg = proxmox_tfa::u2f::Registration {
         key: proxmox_tfa::u2f::RegisteredKey {
             key_handle: base64::decode_config(
@@ -538,7 +546,7 @@ fn decode_old_u2f_entry(data: JsonValue) -> Result<proxmox_tfa::u2f::Registratio
         bail!("invalid extra data in u2f entry");
     }
 
-    Ok(reg)
+    Ok(Some(reg))
 }
 
 fn decode_old_oath_entry(
-- 
2.30.2





^ permalink raw reply	[flat|nested] 3+ messages in thread

* [pve-devel] applied-series: [PATCH proxmox-perl-rs 1/2] pve-rs/tfa: fix off by one trimming
  2021-11-12  8:58 [pve-devel] [PATCH proxmox-perl-rs 1/2] pve-rs/tfa: fix off by one trimming Dominik Csapak
  2021-11-12  8:58 ` [pve-devel] [PATCH proxmox-perl-rs 2/2] pve-rs/tfa: ignore and discard incomplete u2f entries Dominik Csapak
@ 2021-11-12  9:23 ` Wolfgang Bumiller
  1 sibling, 0 replies; 3+ messages in thread
From: Wolfgang Bumiller @ 2021-11-12  9:23 UTC (permalink / raw)
  To: Dominik Csapak; +Cc: pve-devel

applied both patches, thanks

On Fri, Nov 12, 2021 at 09:58:13AM +0100, Dominik Csapak wrote:
> to is the last *valid* character, and ranges end by default with one
> less, so extend the range to the actual last character
> 
> this fixes an issue that we could not parse old configs with
> non-padded base64 values
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  pve-rs/src/tfa/mod.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/pve-rs/src/tfa/mod.rs b/pve-rs/src/tfa/mod.rs
> index df192b4..44cec74 100644
> --- a/pve-rs/src/tfa/mod.rs
> +++ b/pve-rs/src/tfa/mod.rs
> @@ -660,7 +660,7 @@ fn trim_ascii_whitespace_start(data: &[u8]) -> &[u8] {
>  
>  fn trim_ascii_whitespace_end(data: &[u8]) -> &[u8] {
>      match data.iter().rposition(|&c| !c.is_ascii_whitespace()) {
> -        Some(to) => &data[..to],
> +        Some(to) => &data[..=to],
>          None => data,
>      }
>  }
> -- 
> 2.30.2




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-11-12  9:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-12  8:58 [pve-devel] [PATCH proxmox-perl-rs 1/2] pve-rs/tfa: fix off by one trimming Dominik Csapak
2021-11-12  8:58 ` [pve-devel] [PATCH proxmox-perl-rs 2/2] pve-rs/tfa: ignore and discard incomplete u2f entries Dominik Csapak
2021-11-12  9:23 ` [pve-devel] applied-series: [PATCH proxmox-perl-rs 1/2] pve-rs/tfa: fix off by one trimming Wolfgang Bumiller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal