tools: check for failure decoding secret base64 value

[PATCH] tools: check for failure decoding secret base64 value

Posted by Daniel P. Berrangé via Devel 3 weeks, 6 days ago

From: Daniel P. Berrangé <berrange@redhat.com>

The g_base64_decode function will return a valid pointer, but with
length of zero when it fails to decode data. Report an error in that
scenario, so avoid a later more obsecure error.

eg old behaviour

  # virsh secret-set-value  f52a81b2-424e-490c-823d-6bd4235bc507 foo
  warning: Passing secret value as command-line argument is insecure!
  error: Failed to set secret value
  error: value in virSecretSetValue must not be NULL

new behaviour

  # /home/berrange/virsh secret-set-value  f52a81b2-424e-490c-823d-6bd4235bc507 foo
  warning: Passing secret value as command-line argument is insecure!
  error: Secret value is not valid base64

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tools/virsh-secret.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c
index 26ecb41073..215e8e1017 100644
--- a/tools/virsh-secret.c
+++ b/tools/virsh-secret.c
@@ -267,6 +267,11 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd)
 
         secret_val = (char *) g_base64_decode(tmp, &secret_len);
         virSecureErase(tmp, tmp_len);
+
+        if (!secret_len) {
+            vshError(ctl, "%s", _("Secret value is not valid base64"));
+            return false;
+        }
     }
 
     res = virSecretSetValue(secret, (unsigned char *) secret_val, secret_len, 0);
-- 
2.52.0

Re: [PATCH] tools: check for failure decoding secret base64 value

Posted by Michal Prívozník via Devel 3 weeks, 5 days ago

On 1/5/26 18:49, Daniel P. Berrangé via Devel wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
> 
> The g_base64_decode function will return a valid pointer, but with
> length of zero when it fails to decode data. Report an error in that
> scenario, so avoid a later more obsecure error.
> 
> eg old behaviour
> 
>   # virsh secret-set-value  f52a81b2-424e-490c-823d-6bd4235bc507 foo
>   warning: Passing secret value as command-line argument is insecure!
>   error: Failed to set secret value
>   error: value in virSecretSetValue must not be NULL
> 
> new behaviour
> 
>   # /home/berrange/virsh secret-set-value  f52a81b2-424e-490c-823d-6bd4235bc507 foo
>   warning: Passing secret value as command-line argument is insecure!
>   error: Secret value is not valid base64
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tools/virsh-secret.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal