Re: [PATCH v2 02/25] hw/s390x/ipl: Create certificate store
On 5/14/25 1:43 AM, Thomas Huth wrote:
> On 09/05/2025 00.50, Zhuoying Cai wrote:
>> Create a certificate store for boot certificates used for secure IPL.
>>
>> Load certificates from the -boot-certificate option into the cert store.
>
> Nit: Remove the "-" before the "boot-certificate" here now, too.
>
>>
>> Currently, only x509 certificates in DER format and uses SHA-256 hashing
>> algorithm are supported, as these are the types required for secure boot
>> on s390.
>>
>> Signed-off-by: Zhuoying Cai
>> ---
>> crypto/meson.build | 5 +-
>> crypto/x509-utils.c | 163
>> hw/s390x/cert-store.c | 242
>> hw/s390x/cert-store.h | 39 ++
>> hw/s390x/ipl.c | 9 ++
>> hw/s390x/ipl.h | 3 +
>> hw/s390x/meson.build| 1 +
>> include/crypto/x509-utils.h | 6 +
>> include/hw/s390x/ipl/qipl.h | 3 +
>> qapi/crypto.json| 80
>> 10 files changed, 547 insertions(+), 4 deletions(-)
>> create mode 100644 hw/s390x/cert-store.c
>> create mode 100644 hw/s390x/cert-store.h
>>
>> diff --git a/crypto/meson.build b/crypto/meson.build
>> index 735635de1f..0614bfa914 100644
>> --- a/crypto/meson.build
>> +++ b/crypto/meson.build
>> @@ -22,12 +22,9 @@ crypto_ss.add(files(
>> 'tlscredsx509.c',
>> 'tlssession.c',
>> 'rsakey.c',
>> + 'x509-utils.c',
>> ))
>>
>> -if gnutls.found()
>> - crypto_ss.add(files('x509-utils.c'))
>> -endif
>
> Alternatively, you could put the "return -ENOTSUP;" functions into a
> x509-utils-stub.c file instead. Just as an idea. Not sure what is nicer
> here, though.
>
>> if nettle.found()
>> crypto_ss.add(nettle, files('hash-nettle.c', 'hmac-nettle.c',
>> 'pbkdf-nettle.c'))
>> if hogweed.found()
>> diff --git a/crypto/x509-utils.c b/crypto/x509-utils.c
>> index 8bad00a51b..0b8cfc9022 100644
>> --- a/crypto/x509-utils.c
>> +++ b/crypto/x509-utils.c
>> @@ -11,6 +11,12 @@
>> #include "qemu/osdep.h"
>> #include "qapi/error.h"
>> #include "crypto/x509-utils.h"
>> +
>> +/*
>> + * Surround with GNUTLS marco to ensure the interfaces are
>> + * still available when GNUTLS is not enabled.
>> + */
>> +#ifdef CONFIG_GNUTLS
>> #include
>> #include
>> #include
>> @@ -25,6 +31,94 @@ static const int
>> qcrypto_to_gnutls_hash_alg_map[QCRYPTO_HASH_ALGO__MAX] = {
>> [QCRYPTO_HASH_ALGO_RIPEMD160] = GNUTLS_DIG_RMD160,
>> };
>>
>> +static const int
>> qcrypto_to_gnutls_keyid_flags_map[QCRYPTO_KEYID_FLAGS__MAX] = {
>> +[QCRYPTO_KEYID_FLAGS_SHA1] = GNUTLS_KEYID_USE_SHA1,
>> +[QCRYPTO_KEYID_FLAGS_SHA256] = GNUTLS_KEYID_USE_SHA256,
>> +[QCRYPTO_KEYID_FLAGS_SHA512] = GNUTLS_KEYID_USE_SHA512,
>> +[QCRYPTO_KEYID_FLAGS_BEST_KNOWN] = GNUTLS_KEYID_USE_BEST_KNOWN,
>> +};
>> +
>> +static const int qcrypto_to_gnutls_cert_fmt_map[QCRYPTO_CERT_FMT__MAX] = {
>> +[QCRYPTO_CERT_FMT_DER] = GNUTLS_X509_FMT_DER,
>> +[QCRYPTO_CERT_FMT_PEM] = GNUTLS_X509_FMT_PEM,
>> +};
>> +
>> +int qcrypto_check_x509_cert_fmt(uint8_t *cert, size_t size,
>> + QCryptoCertFmt fmt, Error **errp)
>
> Indentation seems to be off by one?
>
>> +{
>> +int rc;
>> +int ret = 0;
>> +gnutls_x509_crt_t crt;
>> +gnutls_datum_t datum = {.data = cert, .size = size};
>> +
>> +if (fmt >= G_N_ELEMENTS(qcrypto_to_gnutls_cert_fmt_map)) {
>> +error_setg(errp, "Unknown certificate format");
>> +return ret;
> > +}> +
>> +if (gnutls_x509_crt_init(&crt) < 0) {
>> +error_setg(errp, "Failed to initialize certificate");
>> +goto cleanup;
>
> So this will do a gnutls_x509_crt_deinit() in case the init() failed ... is
> that OK or should deinit() only be called after init() succeeded?
>
>> +}
>> +
>> +rc = gnutls_x509_crt_import(crt, &datum,
>> qcrypto_to_gnutls_cert_fmt_map[fmt]);
>> +if (rc == GNUTLS_E_ASN1_TAG_ERROR) {
>> +ret = 0;
>> +goto cleanup;
>> +}
>> +
>> +ret = 1;
>> +
>> +cleanup:
>> +gnutls_x509_crt_deinit(crt);
>> +return ret;
>> +}
>
> The return code handling of this function is somewhat confusing. It looks
> like it is meant to return a boolean value (1 for success, 0 for failure),
> and that's also the way you use it in the function below, but the return
> type is "int". Even worse the stub function at the end of the file does a "
>return -ENOTSUP;". Although this does not seem to be a problem right now,
> this might be very fragile for future changes (future code that's expecting
> a 0 for failures and non-zero for succcess will fail with -ENOTSUP).
>
> I'd suggest to rework this function so that it 0 for success and a negative
> error code in case of errors to avoid the possibility of confusion.
>
>> +static int qcrypto_get_x509_cert_fmt(uint8_t *cert, size_t size, Error
>> **errp)
>> +{
>> +int fmt;
>
> If you initialize fmt with -1 here ...
>
>> +
Re: [PATCH v2 02/25] hw/s390x/ipl: Create certificate store
On 5/14/25 5:03 AM, Daniel P. Berrangé wrote:
> On Thu, May 08, 2025 at 06:50:18PM -0400, Zhuoying Cai wrote:
>> Create a certificate store for boot certificates used for secure IPL.
>>
>> Load certificates from the -boot-certificate option into the cert store.
>>
>> Currently, only x509 certificates in DER format and uses SHA-256 hashing
>> algorithm are supported, as these are the types required for secure boot
>> on s390.
>>
>> Signed-off-by: Zhuoying Cai
>> ---
>> crypto/meson.build | 5 +-
>> crypto/x509-utils.c | 163
>> hw/s390x/cert-store.c | 242
>> hw/s390x/cert-store.h | 39 ++
>> hw/s390x/ipl.c | 9 ++
>> hw/s390x/ipl.h | 3 +
>> hw/s390x/meson.build| 1 +
>> include/crypto/x509-utils.h | 6 +
>> include/hw/s390x/ipl/qipl.h | 3 +
>> qapi/crypto.json| 80
>
>
> Please split the crypto subsystem changes from the s390x subsystem
> changes, as separate commits. Also be sure to CC myself (as crypto
> maintainer) on patches that change the crypto subsystem.
>
>
>> diff --git a/crypto/x509-utils.c b/crypto/x509-utils.c
>> index 8bad00a51b..0b8cfc9022 100644
>> --- a/crypto/x509-utils.c
>> +++ b/crypto/x509-utils.c
>> @@ -11,6 +11,12 @@
>> #include "qemu/osdep.h"
>> #include "qapi/error.h"
>> #include "crypto/x509-utils.h"
>> +
>> +/*
>> + * Surround with GNUTLS marco to ensure the interfaces are
>> + * still available when GNUTLS is not enabled.
>
> This comment is redundant - we don't need to explain
> what an #ifdef does.
>
>> + */
>> +#ifdef CONFIG_GNUTLS
>> #include
>> #include
>> #include
>> @@ -25,6 +31,94 @@ static const int
>> qcrypto_to_gnutls_hash_alg_map[QCRYPTO_HASH_ALGO__MAX] = {
>> [QCRYPTO_HASH_ALGO_RIPEMD160] = GNUTLS_DIG_RMD160,
>> };
>>
>> +static const int
>> qcrypto_to_gnutls_keyid_flags_map[QCRYPTO_KEYID_FLAGS__MAX] = {
>> +[QCRYPTO_KEYID_FLAGS_SHA1] = GNUTLS_KEYID_USE_SHA1,
>> +[QCRYPTO_KEYID_FLAGS_SHA256] = GNUTLS_KEYID_USE_SHA256,
>> +[QCRYPTO_KEYID_FLAGS_SHA512] = GNUTLS_KEYID_USE_SHA512,
>> +[QCRYPTO_KEYID_FLAGS_BEST_KNOWN] = GNUTLS_KEYI_DUSE_BEST_KNOWN,
>> +};
>> +
>> +static const int qcrypto_to_gnutls_cert_fmt_map[QCRYPTO_CERT_FMT__MAX] = {
>> +[QCRYPTO_CERT_FMT_DER] = GNUTLS_X509_FMT_DER,
>> +[QCRYPTO_CERT_FMT_PEM] = GNUTLS_X509_FMT_PEM,
>> +};
>> +
>> +int qcrypto_check_x509_cert_fmt(uint8_t *cert, size_t size,
>> + QCryptoCertFmt fmt, Error **errp)
>> +{
>> +int rc;
>> +int ret = 0;
>> +gnutls_x509_crt_t crt;
>> +gnutls_datum_t datum = {.data = cert, .size = size};
>> +
>> +if (fmt >= G_N_ELEMENTS(qcrypto_to_gnutls_cert_fmt_map)) {
>> +error_setg(errp, "Unknown certificate format");
>> +return ret;
>> +}
>> +
>> +if (gnutls_x509_crt_init(&crt) < 0) {
>> +error_setg(errp, "Failed to initialize certificate");
>> +goto cleanup;
>> +}
>> +
>> +rc = gnutls_x509_crt_import(crt, &datum,
>> qcrypto_to_gnutls_cert_fmt_map[fmt]);
>> +if (rc == GNUTLS_E_ASN1_TAG_ERROR) {
>> +ret = 0;
>> +goto cleanup;
>> +}
>> +
>> +ret = 1;
>> +
>> +cleanup:
>> +gnutls_x509_crt_deinit(crt);
>> +return ret;
>> +}
>> +
>> +static int qcrypto_get_x509_cert_fmt(uint8_t *cert, size_t size, Error
>> **errp)
>> +{
>> +int fmt;
>> +
>> +if (qcrypto_check_x509_cert_fmt(cert, size, QCRYPTO_CERT_FMT_DER,
>> errp)) {
>> +fmt = GNUTLS_X509_FMT_DER;
>> +} else if (qcrypto_check_x509_cert_fmt(cert, size,
>> QCRYPTO_CERT_FMT_PEM, errp)) {
>> +fmt = GNUTLS_X509_FMT_PEM;
>> +} else {
>> +return -1;
>> +}
>> +
>> +return fmt;
>> +}
>> +
>> +int qcrypto_get_x509_hash_len(QCryptoHashAlgo alg, Error **errp)
>> +{
>> +if (alg >= G_N_ELEMENTS(qcrypto_to_gnutls_hash_alg_map)) {
>> +error_setg(errp, "Unknown hash algorithm");
>> +return 0;
>> +}
>> +
>> +return gnutls_hash_get_len(qcrypto_to_gnutls_hash_alg_map[alg]);
>> +}
>> +
>> +int qcrypto_get_x509_keyid_len(QCryptoKeyidFlags flag, Error **errp)
>> +{
>> +QCryptoHashAlgo alg;
>> +
>> +if (flag >= G_N_ELEMENTS(qcrypto_to_gnutls_keyid_flags_map)) {
>> +error_setg(errp, "Unknown key id flag");
>> +return 0;
>> +}
>> +
>> +alg = QCRYPTO_HASH_ALGO_SHA1;
>> +if ((flag &
>> qcrypto_to_gnutls_keyid_flags_map[QCRYPTO_KEYID_FLAGS_SHA512]) ||
>> +(flag &
>> qcrypto_to_gnutls_keyid_flags_map[QCRYPTO_KEYID_FLAGS_BEST_KNOWN])) {
>> +alg = QCRYPTO_HASH_ALGO_SHA512;
>> +} else if (flag &
>> qcrypto_to_gnutls_keyid_flags_map[QCRYPTO_KEYID_FLAGS_SHA256]) {
>> +alg = QCRYPTO_HASH_ALGO_SHA256;
>> +}
>> +
>> +return qcrypto_get_x509_hash_len(alg, errp);
>> +}
>
> This method looks fairly pointless to me. Why doesn't the caller just
> directly call qcrypto_get_x509_hash_len without thi
Re: [PATCH v2 02/25] hw/s390x/ipl: Create certificate store
On Thu, May 08, 2025 at 06:50:18PM -0400, Zhuoying Cai wrote:
> Create a certificate store for boot certificates used for secure IPL.
>
> Load certificates from the -boot-certificate option into the cert store.
>
> Currently, only x509 certificates in DER format and uses SHA-256 hashing
> algorithm are supported, as these are the types required for secure boot
> on s390.
>
> Signed-off-by: Zhuoying Cai
> ---
> crypto/meson.build | 5 +-
> crypto/x509-utils.c | 163
> hw/s390x/cert-store.c | 242
> hw/s390x/cert-store.h | 39 ++
> hw/s390x/ipl.c | 9 ++
> hw/s390x/ipl.h | 3 +
> hw/s390x/meson.build| 1 +
> include/crypto/x509-utils.h | 6 +
> include/hw/s390x/ipl/qipl.h | 3 +
> qapi/crypto.json| 80
Please split the crypto subsystem changes from the s390x subsystem
changes, as separate commits. Also be sure to CC myself (as crypto
maintainer) on patches that change the crypto subsystem.
> diff --git a/crypto/x509-utils.c b/crypto/x509-utils.c
> index 8bad00a51b..0b8cfc9022 100644
> --- a/crypto/x509-utils.c
> +++ b/crypto/x509-utils.c
> @@ -11,6 +11,12 @@
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> #include "crypto/x509-utils.h"
> +
> +/*
> + * Surround with GNUTLS marco to ensure the interfaces are
> + * still available when GNUTLS is not enabled.
This comment is redundant - we don't need to explain
what an #ifdef does.
> + */
> +#ifdef CONFIG_GNUTLS
> #include
> #include
> #include
> @@ -25,6 +31,94 @@ static const int
> qcrypto_to_gnutls_hash_alg_map[QCRYPTO_HASH_ALGO__MAX] = {
> [QCRYPTO_HASH_ALGO_RIPEMD160] = GNUTLS_DIG_RMD160,
> };
>
> +static const int qcrypto_to_gnutls_keyid_flags_map[QCRYPTO_KEYID_FLAGS__MAX]
> = {
> +[QCRYPTO_KEYID_FLAGS_SHA1] = GNUTLS_KEYID_USE_SHA1,
> +[QCRYPTO_KEYID_FLAGS_SHA256] = GNUTLS_KEYID_USE_SHA256,
> +[QCRYPTO_KEYID_FLAGS_SHA512] = GNUTLS_KEYID_USE_SHA512,
> +[QCRYPTO_KEYID_FLAGS_BEST_KNOWN] = GNUTLS_KEYI_DUSE_BEST_KNOWN,
> +};
> +
> +static const int qcrypto_to_gnutls_cert_fmt_map[QCRYPTO_CERT_FMT__MAX] = {
> +[QCRYPTO_CERT_FMT_DER] = GNUTLS_X509_FMT_DER,
> +[QCRYPTO_CERT_FMT_PEM] = GNUTLS_X509_FMT_PEM,
> +};
> +
> +int qcrypto_check_x509_cert_fmt(uint8_t *cert, size_t size,
> + QCryptoCertFmt fmt, Error **errp)
> +{
> +int rc;
> +int ret = 0;
> +gnutls_x509_crt_t crt;
> +gnutls_datum_t datum = {.data = cert, .size = size};
> +
> +if (fmt >= G_N_ELEMENTS(qcrypto_to_gnutls_cert_fmt_map)) {
> +error_setg(errp, "Unknown certificate format");
> +return ret;
> +}
> +
> +if (gnutls_x509_crt_init(&crt) < 0) {
> +error_setg(errp, "Failed to initialize certificate");
> +goto cleanup;
> +}
> +
> +rc = gnutls_x509_crt_import(crt, &datum,
> qcrypto_to_gnutls_cert_fmt_map[fmt]);
> +if (rc == GNUTLS_E_ASN1_TAG_ERROR) {
> +ret = 0;
> +goto cleanup;
> +}
> +
> +ret = 1;
> +
> +cleanup:
> +gnutls_x509_crt_deinit(crt);
> +return ret;
> +}
> +
> +static int qcrypto_get_x509_cert_fmt(uint8_t *cert, size_t size, Error
> **errp)
> +{
> +int fmt;
> +
> +if (qcrypto_check_x509_cert_fmt(cert, size, QCRYPTO_CERT_FMT_DER, errp))
> {
> +fmt = GNUTLS_X509_FMT_DER;
> +} else if (qcrypto_check_x509_cert_fmt(cert, size, QCRYPTO_CERT_FMT_PEM,
> errp)) {
> +fmt = GNUTLS_X509_FMT_PEM;
> +} else {
> +return -1;
> +}
> +
> +return fmt;
> +}
> +
> +int qcrypto_get_x509_hash_len(QCryptoHashAlgo alg, Error **errp)
> +{
> +if (alg >= G_N_ELEMENTS(qcrypto_to_gnutls_hash_alg_map)) {
> +error_setg(errp, "Unknown hash algorithm");
> +return 0;
> +}
> +
> +return gnutls_hash_get_len(qcrypto_to_gnutls_hash_alg_map[alg]);
> +}
> +
> +int qcrypto_get_x509_keyid_len(QCryptoKeyidFlags flag, Error **errp)
> +{
> +QCryptoHashAlgo alg;
> +
> +if (flag >= G_N_ELEMENTS(qcrypto_to_gnutls_keyid_flags_map)) {
> +error_setg(errp, "Unknown key id flag");
> +return 0;
> +}
> +
> +alg = QCRYPTO_HASH_ALGO_SHA1;
> +if ((flag &
> qcrypto_to_gnutls_keyid_flags_map[QCRYPTO_KEYID_FLAGS_SHA512]) ||
> +(flag &
> qcrypto_to_gnutls_keyid_flags_map[QCRYPTO_KEYID_FLAGS_BEST_KNOWN])) {
> +alg = QCRYPTO_HASH_ALGO_SHA512;
> +} else if (flag &
> qcrypto_to_gnutls_keyid_flags_map[QCRYPTO_KEYID_FLAGS_SHA256]) {
> +alg = QCRYPTO_HASH_ALGO_SHA256;
> +}
> +
> +return qcrypto_get_x509_hash_len(alg, errp);
> +}
This method looks fairly pointless to me. Why doesn't the caller just
directly call qcrypto_get_x509_hash_len without this indirection of
QCRYPTO_KEYID_FLAGS, especially given that you've just hardcoded to
use of QCRYPTO_KEYID_FLAGS_SHA256 in the caller ?
> @@ -74,3 +168,72 @@ int qcrypto_get_x509_cert_fingerprint(uint8_t *cert,
Re: [PATCH v2 02/25] hw/s390x/ipl: Create certificate store
On 09/05/2025 00.50, Zhuoying Cai wrote:
Create a certificate store for boot certificates used for secure IPL.
Load certificates from the -boot-certificate option into the cert store.
Nit: Remove the "-" before the "boot-certificate" here now, too.
Currently, only x509 certificates in DER format and uses SHA-256 hashing
algorithm are supported, as these are the types required for secure boot
on s390.
Signed-off-by: Zhuoying Cai
---
crypto/meson.build | 5 +-
crypto/x509-utils.c | 163
hw/s390x/cert-store.c | 242
hw/s390x/cert-store.h | 39 ++
hw/s390x/ipl.c | 9 ++
hw/s390x/ipl.h | 3 +
hw/s390x/meson.build| 1 +
include/crypto/x509-utils.h | 6 +
include/hw/s390x/ipl/qipl.h | 3 +
qapi/crypto.json| 80
10 files changed, 547 insertions(+), 4 deletions(-)
create mode 100644 hw/s390x/cert-store.c
create mode 100644 hw/s390x/cert-store.h
diff --git a/crypto/meson.build b/crypto/meson.build
index 735635de1f..0614bfa914 100644
--- a/crypto/meson.build
+++ b/crypto/meson.build
@@ -22,12 +22,9 @@ crypto_ss.add(files(
'tlscredsx509.c',
'tlssession.c',
'rsakey.c',
+ 'x509-utils.c',
))
-if gnutls.found()
- crypto_ss.add(files('x509-utils.c'))
-endif
Alternatively, you could put the "return -ENOTSUP;" functions into a
x509-utils-stub.c file instead. Just as an idea. Not sure what is nicer
here, though.
if nettle.found()
crypto_ss.add(nettle, files('hash-nettle.c', 'hmac-nettle.c',
'pbkdf-nettle.c'))
if hogweed.found()
diff --git a/crypto/x509-utils.c b/crypto/x509-utils.c
index 8bad00a51b..0b8cfc9022 100644
--- a/crypto/x509-utils.c
+++ b/crypto/x509-utils.c
@@ -11,6 +11,12 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "crypto/x509-utils.h"
+
+/*
+ * Surround with GNUTLS marco to ensure the interfaces are
+ * still available when GNUTLS is not enabled.
+ */
+#ifdef CONFIG_GNUTLS
#include
#include
#include
@@ -25,6 +31,94 @@ static const int
qcrypto_to_gnutls_hash_alg_map[QCRYPTO_HASH_ALGO__MAX] = {
[QCRYPTO_HASH_ALGO_RIPEMD160] = GNUTLS_DIG_RMD160,
};
+static const int qcrypto_to_gnutls_keyid_flags_map[QCRYPTO_KEYID_FLAGS__MAX] = {
+[QCRYPTO_KEYID_FLAGS_SHA1] = GNUTLS_KEYID_USE_SHA1,
+[QCRYPTO_KEYID_FLAGS_SHA256] = GNUTLS_KEYID_USE_SHA256,
+[QCRYPTO_KEYID_FLAGS_SHA512] = GNUTLS_KEYID_USE_SHA512,
+[QCRYPTO_KEYID_FLAGS_BEST_KNOWN] = GNUTLS_KEYID_USE_BEST_KNOWN,
+};
+
+static const int qcrypto_to_gnutls_cert_fmt_map[QCRYPTO_CERT_FMT__MAX] = {
+[QCRYPTO_CERT_FMT_DER] = GNUTLS_X509_FMT_DER,
+[QCRYPTO_CERT_FMT_PEM] = GNUTLS_X509_FMT_PEM,
+};
+
+int qcrypto_check_x509_cert_fmt(uint8_t *cert, size_t size,
+ QCryptoCertFmt fmt, Error **errp)
Indentation seems to be off by one?
+{
+int rc;
+int ret = 0;
+gnutls_x509_crt_t crt;
+gnutls_datum_t datum = {.data = cert, .size = size};
+
+if (fmt >= G_N_ELEMENTS(qcrypto_to_gnutls_cert_fmt_map)) {
+error_setg(errp, "Unknown certificate format");
+return ret;
> +}> +
+if (gnutls_x509_crt_init(&crt) < 0) {
+error_setg(errp, "Failed to initialize certificate");
+goto cleanup;
So this will do a gnutls_x509_crt_deinit() in case the init() failed ... is
that OK or should deinit() only be called after init() succeeded?
+}
+
+rc = gnutls_x509_crt_import(crt, &datum,
qcrypto_to_gnutls_cert_fmt_map[fmt]);
+if (rc == GNUTLS_E_ASN1_TAG_ERROR) {
+ret = 0;
+goto cleanup;
+}
+
+ret = 1;
+
+cleanup:
+gnutls_x509_crt_deinit(crt);
+return ret;
+}
The return code handling of this function is somewhat confusing. It looks
like it is meant to return a boolean value (1 for success, 0 for failure),
and that's also the way you use it in the function below, but the return
type is "int". Even worse the stub function at the end of the file does a "
return -ENOTSUP;". Although this does not seem to be a problem right now,
this might be very fragile for future changes (future code that's expecting
a 0 for failures and non-zero for succcess will fail with -ENOTSUP).
I'd suggest to rework this function so that it 0 for success and a negative
error code in case of errors to avoid the possibility of confusion.
+static int qcrypto_get_x509_cert_fmt(uint8_t *cert, size_t size, Error **errp)
+{
+int fmt;
If you initialize fmt with -1 here ...
+if (qcrypto_check_x509_cert_fmt(cert, size, QCRYPTO_CERT_FMT_DER, errp)) {
+fmt = GNUTLS_X509_FMT_DER;
+} else if (qcrypto_check_x509_cert_fmt(cert, size, QCRYPTO_CERT_FMT_PEM,
errp)) {
+fmt = GNUTLS_X509_FMT_PEM;
+} else {
+return -1;
... you can drop the final else branch here.
+}
+
+return fmt;
+}
+
+int qcrypto_get_x509_hash_len(QCryptoHashAlgo alg, Error **errp)
+
