Re: [PATCH v3 11/28] crypto: Add helper functions for DIAG 508 subcode 1

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Jun 04, 2025 at 05:56:39PM -0400, Zhuoying Cai wrote:
> Add helper functions for x509 certificate which will be used in the next
> patch for DIAG 508 subcode 1.
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>  crypto/x509-utils.c         | 61 +++++++++++++++++++++++++++++++++++++
>  include/crypto/x509-utils.h | 20 ++++++++++++
>  2 files changed, 81 insertions(+)
> 
> diff --git a/crypto/x509-utils.c b/crypto/x509-utils.c
> index dd8137210c..4c997cf5d4 100644
> --- a/crypto/x509-utils.c
> +++ b/crypto/x509-utils.c
> @@ -16,6 +16,7 @@
>  #include <gnutls/gnutls.h>
>  #include <gnutls/crypto.h>
>  #include <gnutls/x509.h>
> +#include <gnutls/pkcs7.h>
>  
>  static const int qcrypto_to_gnutls_hash_alg_map[QCRYPTO_HASH_ALGO__MAX] = {
>      [QCRYPTO_HASH_ALGO_MD5] = GNUTLS_DIG_MD5,
> @@ -364,6 +365,58 @@ cleanup:
>      return ret;
>  }
>  
> +static int qcrypto_import_pkcs7(gnutls_pkcs7_t pkcs7, gnutls_datum_t *datum)
> +{
> +    int rc;
> +
> +    rc = gnutls_pkcs7_import(pkcs7, datum , GNUTLS_X509_FMT_PEM);
> +    if (rc) {
> +        rc = gnutls_pkcs7_import(pkcs7, datum , GNUTLS_X509_FMT_DER);
> +    }
> +
> +    return rc;
> +}

Again use PEM exclusively which avoids this helper.

> +
> +int qcrypto_verify_x509_cert(uint8_t *cert, size_t cert_size,
> +                             uint8_t *comp, size_t comp_size,
> +                             uint8_t *sig, size_t sig_size, Error **errp)

This is a sub-optimal name as it is not verifying the cert,
it is using the cert to verify a signature.

Incidentally I've just remembered that crypto all methods introduced
by this patch series should use 'qcrypto_x509_' as the prefix.

IOW, call this qcrypto_x509_verify_signature, and similar pattern
in earlier patches.

> +{
> +    int rc = -1;
> +    gnutls_x509_crt_t crt;
> +    gnutls_pkcs7_t signature;
> +    gnutls_datum_t cert_datum = {.data = cert, .size = cert_size};
> +    gnutls_datum_t data_datum = {.data = comp, .size = comp_size};
> +    gnutls_datum_t sig_datum = {.data = sig, .size = sig_size};
> +
> +    if (gnutls_x509_crt_init(&crt) < 0) {
> +        error_setg(errp, "Failed to initialize certificate");
> +        return rc;
> +    }
> +
> +    if (qcrypto_import_x509_cert(crt, &cert_datum) != 0) {
> +        error_setg(errp, "Failed to import certificate");
> +        gnutls_x509_crt_deinit(crt);
> +        return rc;
> +    }
> +
> +    if (gnutls_pkcs7_init(&signature) < 0) {
> +        error_setg(errp, "Failed to initalize pkcs7 data.");

gnutls_strerror details please, and no full stop required.

> +        return rc;
> +    }
> +
> +    if (qcrypto_import_pkcs7(signature, &sig_datum) != 0) {
> +        error_setg(errp, "Failed to import signature");
> +        goto cleanup;
> +    }
> +
> +    rc = gnutls_pkcs7_verify_direct(signature, crt, 0, &data_datum, 0);

This is failing to set 'errp',  and also should not be returning
the raw gnutls value, only '-1' on failure, '0' on success

> +
> +cleanup:
> +    gnutls_x509_crt_deinit(crt);
> +    gnutls_pkcs7_deinit(signature);
> +    return rc;
> +}
> +
>  #else /* ! CONFIG_GNUTLS */
>  

> +/**
> + * qcrypto_verify_x509_cert
> + * @cert: pointer to the raw certiricate data
> + * @cert_size: size of the certificate
> + * @comp: pointer to the component to be verified
> + * @comp_size: size of the component
> + * @sig: pointer to the signature
> + * @sig_size: size of the signature
> + * @errp: error pointer
> + *
> + * Verify the provided @comp against the @sig and @cert.
> + *
> + * Returns: 0 on success,
> + *          negative error code on error,
> + *          -ENOTSUP if GNUTLS is not enabled.

Only -1 on error, no gnutls error codes nor errnos

> + */
> +int qcrypto_verify_x509_cert(uint8_t *cert, size_t cert_size,
> +                             uint8_t *comp, size_t comp_size,
> +                             uint8_t *sig, size_t sig_size, Error **errp);
> +
>  #endif
> -- 
> 2.49.0
> 

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 :|