Re: [PULL 10/10] crypto: Introduce x509 utils

[Dorjoy Chowdhury <dorjoychy111@gmail.com>]

On Tue, Mar 18, 2025 at 10:44 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 9 Sept 2024 at 15:21, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > From: Dorjoy Chowdhury <dorjoychy111@gmail.com>
> >
> > An utility function for getting fingerprint from X.509 certificate
> > has been introduced. Implementation only provided using gnutls.
>
> Hi; recent changes in the codebase mean that one of Coverity's
> "maybe this needs an error check" heuristics is now triggering
> for this code (CID 1593155):
>
> > +int qcrypto_get_x509_cert_fingerprint(uint8_t *cert, size_t size,
> > +                                      QCryptoHashAlgorithm alg,
> > +                                      uint8_t *result,
> > +                                      size_t *resultlen,
> > +                                      Error **errp)
> > +{
> > +    int ret = -1;
> > +    int hlen;
> > +    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);
>
> gnutls_x509_crt_init() can fail and return a negative value
> on error -- should we be checking for and handling this
> error case ?
>

Yes, I think so. It should be probably something like below:

if (gnutls_x509_crt_init(&crt) < 0) {
    error_setg(errp, "Failed to initialize certificate");
    return -1;
}


Regards,
Dorjoy