Re: [PATCH v6 3/8] crypto: Introduce x509 utils

[Daniel P . Berrangé]

On Fri, Sep 06, 2024 at 01:57:30AM +0600, Dorjoy Chowdhury wrote:
> An utility function for getting fingerprint from X.509 certificate
> has been introduced. Implementation only provided using gnutls.
> 
> Signed-off-by: Dorjoy Chowdhury <dorjoychy...@gmail.com>
> ---
>  crypto/meson.build          |  4 ++
>  crypto/x509-utils.c         | 75 +++++++++++++++++++++++++++++++++++++
>  include/crypto/x509-utils.h | 22 +++++++++++
>  3 files changed, 101 insertions(+)
>  create mode 100644 crypto/x509-utils.c
>  create mode 100644 include/crypto/x509-utils.h


> +int qcrypto_get_x509_cert_fingerprint(uint8_t *cert, size_t size,
> +                                      QCryptoHashAlgorithm alg,
> +                                      uint8_t *result,
> +                                      size_t *resultlen,
> +                                      Error **errp)
> +{
> +    int ret;
> +    gnutls_x509_crt_t crt;
> +    gnutls_datum_t datum = {.data = cert, .size = size};
> +
> +    if (alg >= G_N_ELEMENTS(qcrypto_to_gnutls_hash_alg_map)) {
> +        error_setg(errp, "Unknown hash algorithm");
> +        return -1;
> +    }
> +
> +    if (result == NULL) {
> +        error_setg(errp, "No valid buffer given");
> +        return -1;
> +    }
> +
> +    gnutls_x509_crt_init(&crt);
> +
> +    if (gnutls_x509_crt_import(crt, &datum, GNUTLS_X509_FMT_PEM) != 0) {
> +        error_setg(errp, "Failed to import certificate");
> +        goto cleanup;
> +    }
> +
> +    ret = gnutls_hash_get_len(qcrypto_to_gnutls_hash_alg_map[alg]);
> +    if (*resultlen < ret) {
> +        error_setg(errp,
> +                   "Result buffer size %zu is smaller than hash %d",
> +                   *resultlen, ret);
> +        goto cleanup;
> +    }
> +
> +    if (gnutls_x509_crt_get_fingerprint(crt,
> +                                        qcrypto_to_gnutls_hash_alg_map[alg],
> +                                        result, resultlen) != 0) {
> +        error_setg(errp, "Failed to get fingerprint from certificate");
> +        goto cleanup;
> +    }
> +
> +    return 0;
> +
> + cleanup:
> +    gnutls_x509_crt_deinit(crt);
> +    return -1;
> +}

This fails to call gnutls_x509_crt_deinit in the success path.

I'm going to squash in the following change:


diff --git a/crypto/x509-utils.c b/crypto/x509-utils.c
index 593eb8968b..6e157af76b 100644
--- a/crypto/x509-utils.c
+++ b/crypto/x509-utils.c
@@ -31,7 +31,8 @@ int qcrypto_get_x509_cert_fingerprint(uint8_t *cert, size_t 
size,
                                       size_t *resultlen,
                                       Error **errp)
 {
-    int ret;
+    int ret = -1;
+    int hlen;
     gnutls_x509_crt_t crt;
     gnutls_datum_t datum = {.data = cert, .size = size};
 
@@ -52,11 +53,11 @@ int qcrypto_get_x509_cert_fingerprint(uint8_t *cert, size_t 
size,
         goto cleanup;
     }
 
-    ret = gnutls_hash_get_len(qcrypto_to_gnutls_hash_alg_map[alg]);
-    if (*resultlen < ret) {
+    hlen = gnutls_hash_get_len(qcrypto_to_gnutls_hash_alg_map[alg]);
+    if (*resultlen < hlen) {
         error_setg(errp,
                    "Result buffer size %zu is smaller than hash %d",
-                   *resultlen, ret);
+                   *resultlen, hlen);
         goto cleanup;
     }
 
@@ -67,9 +68,9 @@ int qcrypto_get_x509_cert_fingerprint(uint8_t *cert, size_t 
size,
         goto cleanup;
     }
 
-    return 0;
+    ret = 0;
 
  cleanup:
     gnutls_x509_crt_deinit(crt);
-    return -1;
+    return ret;
 }



With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|