Introduce new helper functions to extract certificate metadata needed for
DIAG 320 subcode 2:
qcrypto_x509_check_cert_times() - validates the certificate's validity period against the current time
qcrypto_x509_get_pk_algorithm() - returns the public key algorithm used in the certificate
qcrypto_x509_get_cert_key_id() - extracts the key ID from the certificate
qcrypto_x509_is_ecc_curve_p521() - determines the ECC public key algorithm uses P-521 curve
These functions provide support for metadata extraction and validity checking
for X.509 certificates.
Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
---
crypto/x509-utils.c | 248 ++++++++++++++++++++++++++++++++++++
include/crypto/x509-utils.h | 72 +++++++++++
2 files changed, 320 insertions(+)
diff --git a/crypto/x509-utils.c b/crypto/x509-utils.c
index 5d43b0ec96..763eccb190 100644
--- a/crypto/x509-utils.c
+++ b/crypto/x509-utils.c
@@ -27,6 +27,25 @@ static const int qcrypto_to_gnutls_hash_alg_map[QCRYPTO_HASH_ALGO__MAX] = {
[QCRYPTO_HASH_ALGO_RIPEMD160] = GNUTLS_DIG_RMD160,
};
+static const int gnutls_to_qcrypto_pk_alg_map[] = {
+ [GNUTLS_PK_RSA] = QCRYPTO_PK_ALGO_RSA,
+ [GNUTLS_PK_DSA] = QCRYPTO_PK_ALGO_DSA,
+ [GNUTLS_PK_ECDSA] = QCRYPTO_PK_ALGO_ECDSA,
+ [GNUTLS_PK_RSA_OAEP] = QCRYPTO_PK_ALGO_RSA_OAEP,
+ [GNUTLS_PK_EDDSA_ED25519] = QCRYPTO_PK_ALGO_ED25519,
+ [GNUTLS_PK_EDDSA_ED448] = QCRYPTO_PK_ALGO_ED448,
+};
+
+static const int qcrypto_to_gnutls_keyid_flags_map[] = {
+ [QCRYPTO_HASH_ALGO_MD5] = -1,
+ [QCRYPTO_HASH_ALGO_SHA1] = GNUTLS_KEYID_USE_SHA1,
+ [QCRYPTO_HASH_ALGO_SHA224] = -1,
+ [QCRYPTO_HASH_ALGO_SHA256] = GNUTLS_KEYID_USE_SHA256,
+ [QCRYPTO_HASH_ALGO_SHA384] = -1,
+ [QCRYPTO_HASH_ALGO_SHA512] = GNUTLS_KEYID_USE_SHA512,
+ [QCRYPTO_HASH_ALGO_RIPEMD160] = -1,
+};
+
int qcrypto_get_x509_cert_fingerprint(uint8_t *cert, size_t size,
QCryptoHashAlgo alg,
uint8_t *result,
@@ -122,6 +141,207 @@ cleanup:
return ret;
}
+int qcrypto_x509_check_cert_times(uint8_t *cert, size_t size, Error **errp)
+{
+ int rc;
+ int ret = -1;
+ gnutls_x509_crt_t crt;
+ gnutls_datum_t datum = {.data = cert, .size = size};
+ time_t now = time(0);
+ time_t exp_time;
+ time_t act_time;
+
+ if (now == ((time_t)-1)) {
+ error_setg_errno(errp, errno, "Cannot get current time");
+ return ret;
+ }
+
+ rc = gnutls_x509_crt_init(&crt);
+ if (rc < 0) {
+ error_setg(errp, "Failed to initialize certificate: %s", gnutls_strerror(rc));
+ return ret;
+ }
+
+ rc = gnutls_x509_crt_import(crt, &datum, GNUTLS_X509_FMT_PEM);
+ if (rc != 0) {
+ error_setg(errp, "Failed to import certificate: %s", gnutls_strerror(rc));
+ goto cleanup;
+ }
+
+ exp_time = gnutls_x509_crt_get_expiration_time(crt);
+ if (exp_time == ((time_t)-1)) {
+ error_setg(errp, "Failed to get certificate expiration time");
+ goto cleanup;
+ }
+ if (exp_time < now) {
+ error_setg(errp, "The certificate has expired");
+ goto cleanup;
+ }
+
+ act_time = gnutls_x509_crt_get_activation_time(crt);
+ if (act_time == ((time_t)-1)) {
+ error_setg(errp, "Failed to get certificate activation time");
+ goto cleanup;
+ }
+ if (act_time > now) {
+ error_setg(errp, "The certificate is not yet active");
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ gnutls_x509_crt_deinit(crt);
+ return ret;
+}
+
+int qcrypto_x509_get_pk_algorithm(uint8_t *cert, size_t size, Error **errp)
+{
+ int rc;
+ int ret = -1;
+ unsigned int bits;
+ gnutls_x509_crt_t crt;
+ gnutls_datum_t datum = {.data = cert, .size = size};
+
+ rc = gnutls_x509_crt_init(&crt);
+ if (rc < 0) {
+ error_setg(errp, "Failed to initialize certificate: %s", gnutls_strerror(rc));
+ return ret;
+ }
+
+ rc = gnutls_x509_crt_import(crt, &datum, GNUTLS_X509_FMT_PEM);
+ if (rc != 0) {
+ error_setg(errp, "Failed to import certificate: %s", gnutls_strerror(rc));
+ goto cleanup;
+ }
+
+ rc = gnutls_x509_crt_get_pk_algorithm(crt, &bits);
+ if (rc >= G_N_ELEMENTS(gnutls_to_qcrypto_pk_alg_map)) {
+ error_setg(errp, "Unknown public key algorithm %d", rc);
+ goto cleanup;
+ }
+
+ ret = gnutls_to_qcrypto_pk_alg_map[rc];
+
+cleanup:
+ gnutls_x509_crt_deinit(crt);
+ return ret;
+}
+
+int qcrypto_x509_get_cert_key_id(uint8_t *cert, size_t size,
+ QCryptoHashAlgo hash_alg,
+ uint8_t **result,
+ size_t *resultlen,
+ Error **errp)
+{
+ int rc;
+ int ret = -1;
+ gnutls_x509_crt_t crt;
+ gnutls_datum_t datum = {.data = cert, .size = size};
+
+ if (hash_alg >= G_N_ELEMENTS(qcrypto_to_gnutls_hash_alg_map)) {
+ error_setg(errp, "Unknown hash algorithm %d", hash_alg);
+ return ret;
+ }
+
+ if (qcrypto_to_gnutls_keyid_flags_map[hash_alg] == -1 ||
+ hash_alg >= G_N_ELEMENTS(qcrypto_to_gnutls_keyid_flags_map)) {
+ error_setg(errp, "Unsupported key id flag %d", hash_alg);
+ return ret;
+ }
+
+ rc = gnutls_x509_crt_init(&crt);
+ if (rc < 0) {
+ error_setg(errp, "Failed to initialize certificate: %s", gnutls_strerror(rc));
+ return ret;
+ }
+
+ rc = gnutls_x509_crt_import(crt, &datum, GNUTLS_X509_FMT_PEM);
+ if (rc != 0) {
+ error_setg(errp, "Failed to import certificate: %s", gnutls_strerror(rc));
+ goto cleanup;
+ }
+
+ *resultlen = gnutls_hash_get_len(qcrypto_to_gnutls_hash_alg_map[hash_alg]);
+ if (*resultlen == 0) {
+ error_setg(errp, "Failed to get hash algorithn length: %s", gnutls_strerror(rc));
+ goto cleanup;
+ }
+
+ *result = g_malloc0(*resultlen);
+ if (gnutls_x509_crt_get_key_id(crt,
+ qcrypto_to_gnutls_keyid_flags_map[hash_alg],
+ *result, resultlen) != 0) {
+ error_setg(errp, "Failed to get key ID from certificate");
+ g_clear_pointer(result, g_free);
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ gnutls_x509_crt_deinit(crt);
+ return ret;
+}
+
+static int qcrypto_x509_get_ecc_curve(uint8_t *cert, size_t size, Error **errp)
+{
+ int rc;
+ int ret = -1;
+ gnutls_x509_crt_t crt;
+ gnutls_datum_t datum = {.data = cert, .size = size};
+ gnutls_ecc_curve_t curve_id;
+ gnutls_datum_t x = {.data = NULL, .size = 0};
+ gnutls_datum_t y = {.data = NULL, .size = 0};
+
+ rc = gnutls_x509_crt_init(&crt);
+ if (rc < 0) {
+ error_setg(errp, "Failed to initialize certificate: %s", gnutls_strerror(rc));
+ return ret;
+ }
+
+ rc = gnutls_x509_crt_import(crt, &datum, GNUTLS_X509_FMT_PEM);
+ if (rc != 0) {
+ error_setg(errp, "Failed to import certificate: %s", gnutls_strerror(rc));
+ goto cleanup;
+ }
+
+ rc = gnutls_x509_crt_get_pk_ecc_raw(crt, &curve_id, &x, &y);
+ if (rc != 0) {
+ error_setg(errp, "Failed to get ECC public key curve: %s", gnutls_strerror(rc));
+ goto cleanup;
+ }
+
+ ret = curve_id;
+
+cleanup:
+ gnutls_x509_crt_deinit(crt);
+ gnutls_free(x.data);
+ gnutls_free(y.data);
+ return ret;
+}
+
+int qcrypto_x509_is_ecc_curve_p521(uint8_t *cert, size_t size, Error **errp)
+{
+ int curve_id;
+
+ curve_id = qcrypto_x509_get_ecc_curve(cert, size, errp);
+ if (curve_id == -1) {
+ return -1;
+ }
+
+ if (curve_id == GNUTLS_ECC_CURVE_INVALID) {
+ error_setg(errp, "Invalid ECC curve");
+ return -1;
+ }
+
+ if (curve_id == GNUTLS_ECC_CURVE_SECP521R1) {
+ return 1;
+ }
+
+ return 0;
+}
+
#else /* ! CONFIG_GNUTLS */
int qcrypto_get_x509_cert_fingerprint(uint8_t *cert, size_t size,
@@ -143,4 +363,32 @@ int qcrypto_x509_convert_cert_der(uint8_t *cert, size_t size,
return -1;
}
+int qcrypto_x509_check_cert_times(uint8_t *cert, size_t size, Error **errp)
+{
+ error_setg(errp, "GNUTLS is required to get certificate times");
+ return -1;
+}
+
+int qcrypto_x509_get_pk_algorithm(uint8_t *cert, size_t size, Error **errp)
+{
+ error_setg(errp, "GNUTLS is required to get public key algorithm");
+ return -1;
+}
+
+int qcrypto_x509_get_cert_key_id(uint8_t *cert, size_t size,
+ QCryptoHashAlgo hash_alg,
+ uint8_t **result,
+ size_t *resultlen,
+ Error **errp)
+{
+ error_setg(errp, "GNUTLS is required to get key ID");
+ return -1;
+}
+
+int qcrypto_x509_is_ecc_curve_p521(uint8_t *cert, size_t size, Error **errp)
+{
+ error_setg(errp, "GNUTLS is required to determine ecc curve");
+ return -1;
+}
+
#endif /* ! CONFIG_GNUTLS */
diff --git a/include/crypto/x509-utils.h b/include/crypto/x509-utils.h
index 4239e3e55a..6fc8d982b7 100644
--- a/include/crypto/x509-utils.h
+++ b/include/crypto/x509-utils.h
@@ -13,6 +13,15 @@
#include "crypto/hash.h"
+typedef enum {
+ QCRYPTO_PK_ALGO_RSA,
+ QCRYPTO_PK_ALGO_DSA,
+ QCRYPTO_PK_ALGO_ECDSA,
+ QCRYPTO_PK_ALGO_RSA_OAEP,
+ QCRYPTO_PK_ALGO_ED25519,
+ QCRYPTO_PK_ALGO_ED448,
+} QCryptoPkAlgo;
+
int qcrypto_get_x509_cert_fingerprint(uint8_t *cert, size_t size,
QCryptoHashAlgo hash,
uint8_t *result,
@@ -39,4 +48,67 @@ int qcrypto_x509_convert_cert_der(uint8_t *cert, size_t size,
size_t *resultlen,
Error **errp);
+/**
+ * qcrypto_x509_check_cert_times
+ * @cert: pointer to the raw certificate data
+ * @size: size of the certificate
+ * @errp: error pointer
+ *
+ * Check whether the activation and expiration times of @cert
+ * are valid at the current time.
+ *
+ * Returns: 0 if the certificate times are valid,
+ * -1 on error.
+ */
+int qcrypto_x509_check_cert_times(uint8_t *cert, size_t size, Error **errp);
+
+/**
+ * qcrypto_x509_get_pk_algorithm
+ * @cert: pointer to the raw certificate data
+ * @size: size of the certificate
+ * @errp: error pointer
+ *
+ * Determine the public key algorithm of the @cert.
+ *
+ * Returns: a value from the QCryptoPkAlgo enum on success,
+ * -1 on error.
+ */
+int qcrypto_x509_get_pk_algorithm(uint8_t *cert, size_t size, Error **errp);
+
+/**
+ * qcrypto_x509_get_cert_key_id
+ * @cert: pointer to the raw certificate data
+ * @size: size of the certificate
+ * @hash_alg: the hash algorithm flag
+ * @result: output location for the allocated buffer for key ID
+ (the function allocates memory which must be freed by the caller)
+ * @resultlen: pointer to the size of the buffer
+ (will be updated with the actual size of key id)
+ * @errp: error pointer
+ *
+ * Retrieve the key ID from the @cert based on the specified @flag.
+ *
+ * Returns: 0 if key ID was successfully stored in @result,
+ * -1 on error.
+ */
+int qcrypto_x509_get_cert_key_id(uint8_t *cert, size_t size,
+ QCryptoHashAlgo hash_alg,
+ uint8_t **result,
+ size_t *resultlen,
+ Error **errp);
+
+/**
+ * qcrypto_x509_is_ecc_curve_p521
+ * @cert: pointer to the raw certificate data
+ * @size: size of the certificate
+ * @errp: error pointer
+ *
+ * Determine whether the ECC public key in the given certificate uses the P-521 curve.
+ *
+ * Returns: 0 if ECC public key does not use P521 curve.
+ * 1 if ECC public key uses P521 curve.
+ -1 on error.
+ */
+int qcrypto_x509_is_ecc_curve_p521(uint8_t *cert, size_t size, Error **errp);
+
#endif
--
2.50.1On 18/09/2025 01.21, Zhuoying Cai wrote:
> Introduce new helper functions to extract certificate metadata needed for
> DIAG 320 subcode 2:
>
> qcrypto_x509_check_cert_times() - validates the certificate's validity period against the current time
> qcrypto_x509_get_pk_algorithm() - returns the public key algorithm used in the certificate
> qcrypto_x509_get_cert_key_id() - extracts the key ID from the certificate
> qcrypto_x509_is_ecc_curve_p521() - determines the ECC public key algorithm uses P-521 curve
>
> These functions provide support for metadata extraction and validity checking
> for X.509 certificates.
>
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
...
> +int qcrypto_x509_get_pk_algorithm(uint8_t *cert, size_t size, Error **errp)
> +{
> + int rc;
> + int ret = -1;
> + unsigned int bits;
> + gnutls_x509_crt_t crt;
> + gnutls_datum_t datum = {.data = cert, .size = size};
> +
> + rc = gnutls_x509_crt_init(&crt);
> + if (rc < 0) {
> + error_setg(errp, "Failed to initialize certificate: %s", gnutls_strerror(rc));
> + return ret;
> + }
> +
> + rc = gnutls_x509_crt_import(crt, &datum, GNUTLS_X509_FMT_PEM);
> + if (rc != 0) {
> + error_setg(errp, "Failed to import certificate: %s", gnutls_strerror(rc));
> + goto cleanup;
> + }
> +
> + rc = gnutls_x509_crt_get_pk_algorithm(crt, &bits);
> + if (rc >= G_N_ELEMENTS(gnutls_to_qcrypto_pk_alg_map)) {
gnutls_x509_crt_get_pk_algorithm can also return a negative value according
to the documentation, so I think you should also check for "rc < 0" in
addition here.
> + error_setg(errp, "Unknown public key algorithm %d", rc);
> + goto cleanup;
> + }
> +
> + ret = gnutls_to_qcrypto_pk_alg_map[rc];
> +
> +cleanup:
> + gnutls_x509_crt_deinit(crt);
> + return ret;
> +}
> +
> +int qcrypto_x509_get_cert_key_id(uint8_t *cert, size_t size,
> + QCryptoHashAlgo hash_alg,
> + uint8_t **result,
> + size_t *resultlen,
> + Error **errp)
> +{
> + int rc;
> + int ret = -1;
> + gnutls_x509_crt_t crt;
> + gnutls_datum_t datum = {.data = cert, .size = size};
> +
> + if (hash_alg >= G_N_ELEMENTS(qcrypto_to_gnutls_hash_alg_map)) {
> + error_setg(errp, "Unknown hash algorithm %d", hash_alg);
> + return ret;
> + }
> +
> + if (qcrypto_to_gnutls_keyid_flags_map[hash_alg] == -1 ||
> + hash_alg >= G_N_ELEMENTS(qcrypto_to_gnutls_keyid_flags_map)) {
Since "||" conditions are evaluated from left to right, please check for the
boundary first before using hash_alg as index into the array (i.e. swapt the
two sides of the "||").
> + error_setg(errp, "Unsupported key id flag %d", hash_alg);
> + return ret;
> + }
> +
> + rc = gnutls_x509_crt_init(&crt);
> + if (rc < 0) {
> + error_setg(errp, "Failed to initialize certificate: %s", gnutls_strerror(rc));
> + return ret;
> + }
> +
> + rc = gnutls_x509_crt_import(crt, &datum, GNUTLS_X509_FMT_PEM);
> + if (rc != 0) {
> + error_setg(errp, "Failed to import certificate: %s", gnutls_strerror(rc));
> + goto cleanup;
> + }
> +
> + *resultlen = gnutls_hash_get_len(qcrypto_to_gnutls_hash_alg_map[hash_alg]);
> + if (*resultlen == 0) {
> + error_setg(errp, "Failed to get hash algorithn length: %s", gnutls_strerror(rc));
> + goto cleanup;
> + }
> +
> + *result = g_malloc0(*resultlen);
> + if (gnutls_x509_crt_get_key_id(crt,
> + qcrypto_to_gnutls_keyid_flags_map[hash_alg],
> + *result, resultlen) != 0) {
> + error_setg(errp, "Failed to get key ID from certificate");
> + g_clear_pointer(result, g_free);
> + goto cleanup;
> + }
> +
> + ret = 0;
> +
> +cleanup:
> + gnutls_x509_crt_deinit(crt);
> + return ret;
> +}
...
> +int qcrypto_x509_is_ecc_curve_p521(uint8_t *cert, size_t size, Error **errp)
> +{
> + int curve_id;
> +
> + curve_id = qcrypto_x509_get_ecc_curve(cert, size, errp);
> + if (curve_id == -1) {
> + return -1;
> + }
> +
> + if (curve_id == GNUTLS_ECC_CURVE_INVALID) {
> + error_setg(errp, "Invalid ECC curve");
> + return -1;
> + }
> +
> + if (curve_id == GNUTLS_ECC_CURVE_SECP521R1) {
> + return 1;
> + }
> +
> + return 0;
> +}
Bikeshedding, but IMHO, if you name a function "..._is_something", I'd
prefer if it returns a bool, and not an "int". Otherwise this might get
confusing if you read something like this later in the code:
if (qcrypto_x509_is_ecc_curve_p521(...)) {
}
The caller could use errp to distinguish between the error case and a
simple "false" as answer to the question.
Thomas
On Tue, Oct 07, 2025 at 11:34:29AM +0200, Thomas Huth wrote:
> On 18/09/2025 01.21, Zhuoying Cai wrote:
> > Introduce new helper functions to extract certificate metadata needed for
> > DIAG 320 subcode 2:
> >
> > qcrypto_x509_check_cert_times() - validates the certificate's validity period against the current time
> > qcrypto_x509_get_pk_algorithm() - returns the public key algorithm used in the certificate
> > qcrypto_x509_get_cert_key_id() - extracts the key ID from the certificate
> > qcrypto_x509_is_ecc_curve_p521() - determines the ECC public key algorithm uses P-521 curve
> >
> > These functions provide support for metadata extraction and validity checking
> > for X.509 certificates.
> >
> > Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> > ---
> ...
> > +int qcrypto_x509_get_pk_algorithm(uint8_t *cert, size_t size, Error **errp)
> > +{
> > + int rc;
> > + int ret = -1;
> > + unsigned int bits;
> > + gnutls_x509_crt_t crt;
> > + gnutls_datum_t datum = {.data = cert, .size = size};
> > +
> > + rc = gnutls_x509_crt_init(&crt);
> > + if (rc < 0) {
> > + error_setg(errp, "Failed to initialize certificate: %s", gnutls_strerror(rc));
> > + return ret;
> > + }
> > +
> > + rc = gnutls_x509_crt_import(crt, &datum, GNUTLS_X509_FMT_PEM);
> > + if (rc != 0) {
> > + error_setg(errp, "Failed to import certificate: %s", gnutls_strerror(rc));
> > + goto cleanup;
> > + }
> > +
> > + rc = gnutls_x509_crt_get_pk_algorithm(crt, &bits);
> > + if (rc >= G_N_ELEMENTS(gnutls_to_qcrypto_pk_alg_map)) {
>
> gnutls_x509_crt_get_pk_algorithm can also return a negative value according
> to the documentation, so I think you should also check for "rc < 0" in
> addition here.
>
> > + error_setg(errp, "Unknown public key algorithm %d", rc);
> > + goto cleanup;
> > + }
> > +
> > + ret = gnutls_to_qcrypto_pk_alg_map[rc];
> > +
> > +cleanup:
> > + gnutls_x509_crt_deinit(crt);
> > + return ret;
> > +}
> > +
> > +int qcrypto_x509_get_cert_key_id(uint8_t *cert, size_t size,
> > + QCryptoHashAlgo hash_alg,
> > + uint8_t **result,
> > + size_t *resultlen,
> > + Error **errp)
> > +{
> > + int rc;
> > + int ret = -1;
> > + gnutls_x509_crt_t crt;
> > + gnutls_datum_t datum = {.data = cert, .size = size};
> > +
> > + if (hash_alg >= G_N_ELEMENTS(qcrypto_to_gnutls_hash_alg_map)) {
> > + error_setg(errp, "Unknown hash algorithm %d", hash_alg);
> > + return ret;
> > + }
> > +
> > + if (qcrypto_to_gnutls_keyid_flags_map[hash_alg] == -1 ||
> > + hash_alg >= G_N_ELEMENTS(qcrypto_to_gnutls_keyid_flags_map)) {
>
> Since "||" conditions are evaluated from left to right, please check for the
> boundary first before using hash_alg as index into the array (i.e. swapt the
> two sides of the "||").
>
> > + error_setg(errp, "Unsupported key id flag %d", hash_alg);
> > + return ret;
> > + }
> > +
> > + rc = gnutls_x509_crt_init(&crt);
> > + if (rc < 0) {
> > + error_setg(errp, "Failed to initialize certificate: %s", gnutls_strerror(rc));
> > + return ret;
> > + }
> > +
> > + rc = gnutls_x509_crt_import(crt, &datum, GNUTLS_X509_FMT_PEM);
> > + if (rc != 0) {
> > + error_setg(errp, "Failed to import certificate: %s", gnutls_strerror(rc));
> > + goto cleanup;
> > + }
> > +
> > + *resultlen = gnutls_hash_get_len(qcrypto_to_gnutls_hash_alg_map[hash_alg]);
> > + if (*resultlen == 0) {
> > + error_setg(errp, "Failed to get hash algorithn length: %s", gnutls_strerror(rc));
> > + goto cleanup;
> > + }
> > +
> > + *result = g_malloc0(*resultlen);
> > + if (gnutls_x509_crt_get_key_id(crt,
> > + qcrypto_to_gnutls_keyid_flags_map[hash_alg],
> > + *result, resultlen) != 0) {
> > + error_setg(errp, "Failed to get key ID from certificate");
> > + g_clear_pointer(result, g_free);
> > + goto cleanup;
> > + }
> > +
> > + ret = 0;
> > +
> > +cleanup:
> > + gnutls_x509_crt_deinit(crt);
> > + return ret;
> > +}
> ...
> > +int qcrypto_x509_is_ecc_curve_p521(uint8_t *cert, size_t size, Error **errp)
> > +{
> > + int curve_id;
> > +
> > + curve_id = qcrypto_x509_get_ecc_curve(cert, size, errp);
> > + if (curve_id == -1) {
> > + return -1;
> > + }
> > +
> > + if (curve_id == GNUTLS_ECC_CURVE_INVALID) {
> > + error_setg(errp, "Invalid ECC curve");
> > + return -1;
> > + }
> > +
> > + if (curve_id == GNUTLS_ECC_CURVE_SECP521R1) {
> > + return 1;
> > + }
> > +
> > + return 0;
> > +}
>
> Bikeshedding, but IMHO, if you name a function "..._is_something", I'd
> prefer if it returns a bool, and not an "int". Otherwise this might get
> confusing if you read something like this later in the code:
>
> if (qcrypto_x509_is_ecc_curve_p521(...)) {
> }
>
> The caller could use errp to distinguish between the error case and a
> simple "false" as answer to the question.
Overloading one 'false' return value to mean both success & failure
is an anti-pattern IMHO. We have 3 separate return values we need
for this function, so using -1/0/1 is the right approach.
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 :|
On 07/10/2025 11.38, Daniel P. Berrangé wrote:
> On Tue, Oct 07, 2025 at 11:34:29AM +0200, Thomas Huth wrote:
>> On 18/09/2025 01.21, Zhuoying Cai wrote:
>>> Introduce new helper functions to extract certificate metadata needed for
>>> DIAG 320 subcode 2:
>>>
>>> qcrypto_x509_check_cert_times() - validates the certificate's validity period against the current time
>>> qcrypto_x509_get_pk_algorithm() - returns the public key algorithm used in the certificate
>>> qcrypto_x509_get_cert_key_id() - extracts the key ID from the certificate
>>> qcrypto_x509_is_ecc_curve_p521() - determines the ECC public key algorithm uses P-521 curve
>>>
>>> These functions provide support for metadata extraction and validity checking
>>> for X.509 certificates.
>>>
>>> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
>>> ---
>> ...
>>> +int qcrypto_x509_get_pk_algorithm(uint8_t *cert, size_t size, Error **errp)
>>> +{
>>> + int rc;
>>> + int ret = -1;
>>> + unsigned int bits;
>>> + gnutls_x509_crt_t crt;
>>> + gnutls_datum_t datum = {.data = cert, .size = size};
>>> +
>>> + rc = gnutls_x509_crt_init(&crt);
>>> + if (rc < 0) {
>>> + error_setg(errp, "Failed to initialize certificate: %s", gnutls_strerror(rc));
>>> + return ret;
>>> + }
>>> +
>>> + rc = gnutls_x509_crt_import(crt, &datum, GNUTLS_X509_FMT_PEM);
>>> + if (rc != 0) {
>>> + error_setg(errp, "Failed to import certificate: %s", gnutls_strerror(rc));
>>> + goto cleanup;
>>> + }
>>> +
>>> + rc = gnutls_x509_crt_get_pk_algorithm(crt, &bits);
>>> + if (rc >= G_N_ELEMENTS(gnutls_to_qcrypto_pk_alg_map)) {
>>
>> gnutls_x509_crt_get_pk_algorithm can also return a negative value according
>> to the documentation, so I think you should also check for "rc < 0" in
>> addition here.
>>
>>> + error_setg(errp, "Unknown public key algorithm %d", rc);
>>> + goto cleanup;
>>> + }
>>> +
>>> + ret = gnutls_to_qcrypto_pk_alg_map[rc];
>>> +
>>> +cleanup:
>>> + gnutls_x509_crt_deinit(crt);
>>> + return ret;
>>> +}
>>> +
>>> +int qcrypto_x509_get_cert_key_id(uint8_t *cert, size_t size,
>>> + QCryptoHashAlgo hash_alg,
>>> + uint8_t **result,
>>> + size_t *resultlen,
>>> + Error **errp)
>>> +{
>>> + int rc;
>>> + int ret = -1;
>>> + gnutls_x509_crt_t crt;
>>> + gnutls_datum_t datum = {.data = cert, .size = size};
>>> +
>>> + if (hash_alg >= G_N_ELEMENTS(qcrypto_to_gnutls_hash_alg_map)) {
>>> + error_setg(errp, "Unknown hash algorithm %d", hash_alg);
>>> + return ret;
>>> + }
>>> +
>>> + if (qcrypto_to_gnutls_keyid_flags_map[hash_alg] == -1 ||
>>> + hash_alg >= G_N_ELEMENTS(qcrypto_to_gnutls_keyid_flags_map)) {
>>
>> Since "||" conditions are evaluated from left to right, please check for the
>> boundary first before using hash_alg as index into the array (i.e. swapt the
>> two sides of the "||").
>>
>>> + error_setg(errp, "Unsupported key id flag %d", hash_alg);
>>> + return ret;
>>> + }
>>> +
>>> + rc = gnutls_x509_crt_init(&crt);
>>> + if (rc < 0) {
>>> + error_setg(errp, "Failed to initialize certificate: %s", gnutls_strerror(rc));
>>> + return ret;
>>> + }
>>> +
>>> + rc = gnutls_x509_crt_import(crt, &datum, GNUTLS_X509_FMT_PEM);
>>> + if (rc != 0) {
>>> + error_setg(errp, "Failed to import certificate: %s", gnutls_strerror(rc));
>>> + goto cleanup;
>>> + }
>>> +
>>> + *resultlen = gnutls_hash_get_len(qcrypto_to_gnutls_hash_alg_map[hash_alg]);
>>> + if (*resultlen == 0) {
>>> + error_setg(errp, "Failed to get hash algorithn length: %s", gnutls_strerror(rc));
>>> + goto cleanup;
>>> + }
>>> +
>>> + *result = g_malloc0(*resultlen);
>>> + if (gnutls_x509_crt_get_key_id(crt,
>>> + qcrypto_to_gnutls_keyid_flags_map[hash_alg],
>>> + *result, resultlen) != 0) {
>>> + error_setg(errp, "Failed to get key ID from certificate");
>>> + g_clear_pointer(result, g_free);
>>> + goto cleanup;
>>> + }
>>> +
>>> + ret = 0;
>>> +
>>> +cleanup:
>>> + gnutls_x509_crt_deinit(crt);
>>> + return ret;
>>> +}
>> ...
>>> +int qcrypto_x509_is_ecc_curve_p521(uint8_t *cert, size_t size, Error **errp)
>>> +{
>>> + int curve_id;
>>> +
>>> + curve_id = qcrypto_x509_get_ecc_curve(cert, size, errp);
>>> + if (curve_id == -1) {
>>> + return -1;
>>> + }
>>> +
>>> + if (curve_id == GNUTLS_ECC_CURVE_INVALID) {
>>> + error_setg(errp, "Invalid ECC curve");
>>> + return -1;
>>> + }
>>> +
>>> + if (curve_id == GNUTLS_ECC_CURVE_SECP521R1) {
>>> + return 1;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>
>> Bikeshedding, but IMHO, if you name a function "..._is_something", I'd
>> prefer if it returns a bool, and not an "int". Otherwise this might get
>> confusing if you read something like this later in the code:
>>
>> if (qcrypto_x509_is_ecc_curve_p521(...)) {
>> }
>>
>> The caller could use errp to distinguish between the error case and a
>> simple "false" as answer to the question.
>
> Overloading one 'false' return value to mean both success & failure
> is an anti-pattern IMHO. We have 3 separate return values we need
> for this function, so using -1/0/1 is the right approach.
In that case, please rename the function to something like
"qcrypto_x509_check_ecc_curve_p521" or so ... it's IMHO getting
too confusing otherwise.
Thomas
On 9/17/2025 4:21 PM, Zhuoying Cai wrote: > Introduce new helper functions to extract certificate metadata needed for > DIAG 320 subcode 2: Since we are adding generic helper functions to extract certificate metadata, maybe we should avoid mentioning about "DIAG 320 subcode 2"? Thanks Farhan > > qcrypto_x509_check_cert_times() - validates the certificate's validity period against the current time > qcrypto_x509_get_pk_algorithm() - returns the public key algorithm used in the certificate > qcrypto_x509_get_cert_key_id() - extracts the key ID from the certificate > qcrypto_x509_is_ecc_curve_p521() - determines the ECC public key algorithm uses P-521 curve > > These functions provide support for metadata extraction and validity checking > for X.509 certificates. > > Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com> >
© 2016 - 2025 Red Hat, Inc.