[libvirt PATCH v2 0/2] qemu_migration_cookie: Properly fetch cert DN
Jiri Denemark (2): qemu_migration_cookie: Rename ret in qemuDomainExtractTLSSubject qemu_migration_cookie: Properly fetch cert DN src/qemu/qemu_migration_cookie.c | 34 ++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 13 deletions(-) -- 2.35.0
We use 'ret' for storing values to be returned from a function. Return values from called functions that are not supposed to be returned further are usually called 'rv' (or 'rc'). Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- Notes: Version 2: - no change src/qemu/qemu_migration_cookie.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index ba05a5a07f..76a01781d6 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -184,7 +184,7 @@ qemuDomainExtractTLSSubject(const char *certdir) g_autofree char *pemdata = NULL; gnutls_datum_t pemdatum; gnutls_x509_crt_t cert; - int ret; + int rc; size_t subjectlen; certfile = g_strdup_printf("%s/server-cert.pem", certdir); @@ -195,22 +195,22 @@ qemuDomainExtractTLSSubject(const char *certdir) return NULL; } - ret = gnutls_x509_crt_init(&cert); - if (ret < 0) { + rc = gnutls_x509_crt_init(&cert); + if (rc < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot initialize cert object: %s"), - gnutls_strerror(ret)); + gnutls_strerror(rc)); return NULL; } pemdatum.data = (unsigned char *)pemdata; pemdatum.size = strlen(pemdata); - ret = gnutls_x509_crt_import(cert, &pemdatum, GNUTLS_X509_FMT_PEM); - if (ret < 0) { + rc = gnutls_x509_crt_import(cert, &pemdatum, GNUTLS_X509_FMT_PEM); + if (rc < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot load cert data from %s: %s"), - certfile, gnutls_strerror(ret)); + certfile, gnutls_strerror(rc)); return NULL; } -- 2.35.0
If 1024 was not enough to fit the DN, gnutls_x509_crt_get_dn would store the required size in subjectlen. And since we're not checking the return value of this function, we would happily overwrite some random memory. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: Version 2: - do not pass NULL to the first gnutls_x509_crt_get_dn call src/qemu/qemu_migration_cookie.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 76a01781d6..5d14e271c1 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -180,12 +180,12 @@ static char * qemuDomainExtractTLSSubject(const char *certdir) { g_autofree char *certfile = NULL; - char *subject = NULL; + g_autofree char *subject = NULL; g_autofree char *pemdata = NULL; gnutls_datum_t pemdatum; gnutls_x509_crt_t cert; int rc; - size_t subjectlen; + size_t subjectlen = 256; certfile = g_strdup_printf("%s/server-cert.pem", certdir); @@ -214,13 +214,21 @@ qemuDomainExtractTLSSubject(const char *certdir) return NULL; } - subjectlen = 1024; subject = g_new0(char, subjectlen + 1); - - gnutls_x509_crt_get_dn(cert, subject, &subjectlen); + rc = gnutls_x509_crt_get_dn(cert, subject, &subjectlen); + if (rc == GNUTLS_E_SHORT_MEMORY_BUFFER) { + subject = g_realloc(subject, subjectlen + 1); + rc = gnutls_x509_crt_get_dn(cert, subject, &subjectlen); + } + if (rc != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot get cert distinguished name: %s"), + gnutls_strerror(rc)); + return NULL; + } subject[subjectlen] = '\0'; - return subject; + return g_steal_pointer(&subject); } -- 2.35.0
On 2/11/22 13:29, Jiri Denemark wrote:
If 1024 was not enough to fit the DN, gnutls_x509_crt_get_dn would store the required size in subjectlen. And since we're not checking the return value of this function, we would happily overwrite some random memory.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
Notes: Version 2: - do not pass NULL to the first gnutls_x509_crt_get_dn call
src/qemu/qemu_migration_cookie.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Jiri Denemark -
Michal Prívozník