Bug 1699062

Description of problem:

When gnutls is in LIB_STATE_ERROR, it means that further memory allocations will fail.
virNetTLSContextNew function in src/rpc/virnettlscontext.c file doesn't take into account that case and attempt to free already freed ctxt->x509cred structure, which unavoidably leads to segfault.

It was discovered after analysis of frequent dynflowd service segfaults in Foreman. On the foreman's side it has been reported and described here: https://projects.theforeman.org/issues/26574

Version-Release number of selected component (if applicable):

Bug was introduced in version 0.9.3 and is present in all subsequent versions
It may be attributed to this commit from year 2010:

https://github.com/libvirt/libvirt/commit/30fd0bbbfc76469dee5f636d3f8972c81ff43f85

How reproducible:

In foreman 1.21 apply errata on systems that are running as QEMU-KVM/LibVirt guests.

Steps to Reproduce:
1. Log in to foreman
2. Navigate to Hosts/Content Hosts
3. Select from the list any host with installable updates and select "Manage Errata" from "Select Action" drop down menu
4. Select and install some updates

Actual results:

Lets examine it through tour with gdb session. dynflow_executor can't read entropy from /dev/urandom which causes gnutls to switch into LIB_STATE_ERROR state:

(gdb) handle SIGPIPE nostop noprint pass
Signal        Stop      Print   Pass to program Description
SIGPIPE       No        No      Yes             Broken pipe
(gdb) break rpc/virnettlscontext.c:710 if ret != 0
No symbol "ret" in current context.
(gdb) break rpc/virnettlscontext.c:760
Breakpoint 1 at 0x7f884e656380: file rpc/virnettlscontext.c, line 760.
(gdb) break gnutls_cert.c:230 if ret != 0
Breakpoint 2 at 0x7f884c697d3a: file gnutls_cert.c, line 230.
(gdb) watch _gnutls_lib_mode
Hardware watchpoint 3: _gnutls_lib_mode
(gdb) continue
Continuing.
...
[Switching to Thread 0x7f882bdfd700 (LWP 15300)]
Hardware watchpoint 3: _gnutls_lib_mode

Old value = 3
New value = 4
rnd_func (_ctx=<optimized out>, length=<optimized out>, data=<optimized out>) at pk.c:69
69      }
(gdb) info symbol rnd_func
rnd_func in section .text of /lib64/libgnutls.so.28
(gdb) list
64                      _gnutls_switch_lib_state(LIB_STATE_ERROR);
65      #else
66                      abort();
67      #endif
68              }
69      }
70
71      static void
72      ecc_scalar_zclear (struct ecc_scalar *s)
73      {
(gdb) bt full                                                                                                                                                                                                                                                        [1/9284]
#0  rnd_func (_ctx=<optimized out>, length=<optimized out>, data=<optimized out>) at pk.c:69
No locals.
#1  0x00007f884af6a55a in nettle_mpz_random_size () from /lib64/libhogweed.so.2
No symbol table info available.
#2  0x00007f884af6a5e4 in nettle_mpz_random () from /lib64/libhogweed.so.2
No symbol table info available.
#3  0x00007f884af6cf09 in _nettle_rsa_blind () from /lib64/libhogweed.so.2
No symbol table info available.
#4  0x00007f884af6bc91 in nettle_rsa_pkcs1_sign_tr () from /lib64/libhogweed.so.2
No symbol table info available.
#5  0x00007f884c740849 in _wrap_nettle_pk_sign (algo=<optimized out>, signature=0x7f882bdf1eb0, vdata=0x7f882bdf1e40, pk_params=0x7f8820f911a0) at pk.c:651
        priv = {size = 512, d = {{_mp_alloc = 65, _mp_size = 64, _mp_d = 0x7f8820fa1fd0}}, p = {{_mp_alloc = 33, _mp_size = 32, _mp_d = 0x7f8820f92670}}, q = {{_mp_alloc = 33, _mp_size = 32, _mp_d = 0x7f8820fa21e0}}, a = {{_mp_alloc = 32, _mp_size = 32,
              _mp_d = 0x7f8820fa2400}}, b = {{_mp_alloc = 32, _mp_size = 32, _mp_d = 0x7f8820fa2510}}, c = {{_mp_alloc = 32, _mp_size = 32, _mp_d = 0x7f8820fa22f0}}}
        pub = {size = 512, n = {{_mp_alloc = 65, _mp_size = 64, _mp_d = 0x7f8820fa1dc0}}, e = {{_mp_alloc = 1, _mp_size = 1, _mp_d = 0x7f8820f93180}}}
        s = {{_mp_alloc = 64, _mp_size = 64, _mp_d = 0x7f8820fa5b30}}
        ret = <optimized out>
        hash_len = 0
        me = <optimized out>
#6  0x00007f884c6a6dd7 in gnutls_privkey_sign_data (signer=0x7f8820f984c0, hash=hash@entry=GNUTLS_DIG_SHA256, flags=flags@entry=0, data=data@entry=0x7f882bdf1ea0, signature=signature@entry=0x7f882bdf1eb0) at gnutls_privkey.c:796
        ret = 0
        digest = {data = 0x7f8820fa1b60 "010\r\006\t`\206H\001e\003\004\002\001\005", size = 51}
        me = <optimized out>
#7  0x00007f884c698655 in _gnutls_check_key_cert_match (res=res@entry=0x7f8820f8afd0) at gnutls_cert.c:947
        test = {data = 0x7f884c74cd90 "test text", size = 9}
        sig = {data = 0x0, size = 0}
        pk = 1
        pk2 = <optimized out>
        ret = <optimized out>
        __func__ = "_gnutls_check_key_cert_match"
#8  0x00007f884c6a4944 in gnutls_certificate_set_x509_key_file2 (res=0x7f8820f8afd0, certfile=<optimized out>, certfile@entry=0x7f8820f9c970 "/etc/pki/libvirt/clientcert.pem", keyfile=keyfile@entry=0x7f8820f9c8d0 "/etc/pki/libvirt/private/clientkey.pem",
    type=type@entry=GNUTLS_X509_FMT_PEM, pass=pass@entry=0x0, flags=flags@entry=0) at gnutls_x509.c:1410
        ret = <optimized out>
#9  0x00007f884c6a4ebb in gnutls_certificate_set_x509_key_file (res=<optimized out>, certfile=certfile@entry=0x7f8820f9c970 "/etc/pki/libvirt/clientcert.pem", keyfile=keyfile@entry=0x7f8820f9c8d0 "/etc/pki/libvirt/private/clientkey.pem",
    type=type@entry=GNUTLS_X509_FMT_PEM) at gnutls_x509.c:1350
No locals.
#10 0x00007f884e656a92 in virNetTLSContextLoadCredentials (ctxt=0x7f8820f8ae10, key=0x7f8820f9c8d0 "/etc/pki/libvirt/private/clientkey.pem", cert=0x7f8820f9c970 "/etc/pki/libvirt/clientcert.pem", cacrl=0x7f8820f9bfe0 "/etc/pki/CA/cacrl.pem",
    cacert=0x7f8820f9c750 "/etc/pki/CA/cacert.pem", isServer=false) at rpc/virnettlscontext.c:664
        rv = <optimized out>
        ret = -1
        err = <optimized out>
#11 virNetTLSContextNew (cacert=0x7f8820f9c750 "/etc/pki/CA/cacert.pem", cacrl=0x7f8820f9bfe0 "/etc/pki/CA/cacrl.pem", cert=0x7f8820f9c970 "/etc/pki/libvirt/clientcert.pem", key=0x7f8820f9c8d0 "/etc/pki/libvirt/private/clientkey.pem",
    x509dnWhitelist=x509dnWhitelist@entry=0x0, priority=priority@entry=0x0, sanityCheckCert=sanityCheckCert@entry=true, requireValidCert=requireValidCert@entry=true, isServer=isServer@entry=false) at rpc/virnettlscontext.c:721
        ctxt = 0x7f8820f8ae10
        err = <optimized out>
        __FUNCTION__ = "virNetTLSContextNew"
        __func__ = "virNetTLSContextNew"
...

Further code execution will leads to gnutls failed memory allocation with GNUTLS_E_LIB_IN_ERROR_STATE error.
As a consequence of that structure is freed with with gnutls_free at line 232 and GNUTLS_E_MEMORY_ERROR
is being returned to the caller

(gdb) cont
Continuing
....
[Switching to Thread 0x7f8848260700 (LWP 15440)]

Breakpoint 2, gnutls_certificate_allocate_credentials (res=0x7f882c6aab88) at gnutls_cert.c:230
230             if (ret < 0) {
(gdb) info symbol gnutls_certificate_allocate_credentials
gnutls_certificate_allocate_credentials in section .text of /lib64/libgnutls.so.28
(gdb) print ret
$3 = -402
(gdb) list
225
226             if (*res == NULL)
227                     return GNUTLS_E_MEMORY_ERROR;
228
229             ret = gnutls_x509_trust_list_init(&(*res)->tlist, 0);
230             if (ret < 0) {
231                     gnutls_assert();
232                     gnutls_free(*res);
233                     return GNUTLS_E_MEMORY_ERROR;
234             }
(gdb) list verify-high.c:77
72      gnutls_x509_trust_list_init(gnutls_x509_trust_list_t * list,
73                                  unsigned int size)
74      {
75              gnutls_x509_trust_list_t tmp;
76
77              FAIL_IF_LIB_ERROR;
78
79              tmp =
80                  gnutls_calloc(1, sizeof(struct gnutls_x509_trust_list_st));
(gdb) bt
#0  gnutls_certificate_allocate_credentials (res=0x7f882c6aab88) at gnutls_cert.c:230
#1  0x00007f884e656134 in virNetTLSContextNew (cacert=0x7f882c6aa6f0 "/etc/pki/CA/cacert.pem", cacrl=0x7f882c6a7620 "/etc/pki/CA/cacrl.pem", cert=0x7f882c6aad90 "/etc/pki/libvirt/clientcert.pem", key=0x7f882c6aacf0 "/etc/pki/libvirt/private/clientkey.pem",
    x509dnWhitelist=x509dnWhitelist@entry=0x0, priority=priority@entry=0x0, sanityCheckCert=sanityCheckCert@entry=true, requireValidCert=requireValidCert@entry=true, isServer=isServer@entry=false) at rpc/virnettlscontext.c:709
#2  0x00007f884e6570d2 in virNetTLSContextNewPath (pkipath=<optimized out>, tryUserPkiPath=<optimized out>, x509dnWhitelist=x509dnWhitelist@entry=0x0, priority=priority@entry=0x0, sanityCheckCert=sanityCheckCert@entry=true,
    requireValidCert=requireValidCert@entry=true, isServer=isServer@entry=false) at rpc/virnettlscontext.c:908
#3  0x00007f884e657f61 in virNetTLSContextNewClientPath (pkipath=<optimized out>, tryUserPkiPath=<optimized out>, priority=priority@entry=0x0, sanityCheckCert=sanityCheckCert@entry=true, requireValidCert=requireValidCert@entry=true) at rpc/virnettlscontext.c:937
#4  0x00007f884e7068cb in doRemoteOpen (conn=conn@entry=0x7f882c6aa9a0, priv=0x7f882c6aac50, driver_str=0x7f882c6aae20 "qemu", transport_str=<optimized out>, auth=auth@entry=0x0, conf=conf@entry=0x0, flags=0) at remote/remote_driver.c:968
#5  0x00007f884e708436 in remoteConnectOpen (conn=0x7f882c6aa9a0, auth=0x0, conf=0x0, flags=<optimized out>) at remote/remote_driver.c:1385
#6  0x00007f884e718207 in virConnectOpenInternal (name=<optimized out>, name@entry=0x7f882c6aa4d0 "qemu+tls://blade4s15.hv.internal/system", auth=auth@entry=0x0, flags=flags@entry=0) at libvirt.c:1040
#7  0x00007f884e7199a9 in virConnectOpen (name=0x7f882c6aa4d0 "qemu+tls://blade4s15.hv.internal/system") at libvirt.c:1118

All preconditions are set, so further execution leads to segmentation fault

(gdb) cont
Continuing.

Breakpoint 1, virNetTLSContextNew (cacert=0x7f882c6aa6f0 "/etc/pki/CA/cacert.pem", cacrl=0x7f882c6a7620 "/etc/pki/CA/cacrl.pem", cert=0x7f882c6aad90 "/etc/pki/libvirt/clientcert.pem", key=0x7f882c6aacf0 "/etc/pki/libvirt/private/clientkey.pem",
    x509dnWhitelist=x509dnWhitelist@entry=0x0, priority=priority@entry=0x0, sanityCheckCert=sanityCheckCert@entry=true, requireValidCert=requireValidCert@entry=true, isServer=isServer@entry=false) at rpc/virnettlscontext.c:760
760         if (isServer)
(gdb) print ctxt
$4 = (virNetTLSContextPtr) 0x7f882c6aab50
(gdb) print *ctxt
$5 = {parent = {parent = {u = {dummy_align1 = 7700611093, dummy_align2 = 0x1cafe0015, s = {magic = 3405643797, refs = 1}}, klass = 0x7f8820f9c9a0}, lock = {lock = {__data = {__lock = 0, __count = 0, __owner = 0, __nusers = 0, __kind = 512, __spins = 0, __elision = 0,
          __list = {__prev = 0x0, __next = 0x0}}, __size = '\000' <repeats 17 times>, "\002", '\000' <repeats 21 times>, __align = 0}}}, x509cred = 0x7f882c6aaf20, dhParams = 0x0, isServer = false, requireValidCert = false, x509dnWhitelist = 0x0, priority = 0x0}
(gdb) print *ctxt->x509cred
$6 = {dh_params = 0x0, params_func = 0x39353a3132203830, certs = 0x2b3030362e34333a, ncerts = 808464432, pkey = 0x727265203a303434, keyring = 0x726976203a20726f, tlist = 0x6f43534c5474654e, verify_flags = 2019914862, verify_depth = 2003127924,
  verify_bits = 858863418, get_cert_callback = 0x6f7420656c62616e, client_get_cert_callback = 0x7461636f6c6c6120, server_get_cert_callback = 0x6320393035782065, get_cert_callback2 = 0x65646572, verify_callback = 0x141, pin = {cb = 0x7f882c000078,
    data = 0x7f882c000078}, pin_tmp = ":34.600+0000: 15440: error : vir", ocsp_func = 0x6f43534c5474654e, ocsp_func_ptr = 0x77654e747865746e, ocsp_response_file = 0x55203a203331373a <Address 0x55203a203331373a out of bounds>}
(gdb) print *ctxt->x509cred.tlist
Cannot access memory at address 0x6f43534c5474654e
(gdb) cont
Continuing.

Program received signal SIGSEGV, Segmentation fault.
gnutls_x509_trust_list_deinit (list=0x6f43534c5474654e, all=1) at verify-high.c:122
122             for (j = 0; j < list->blacklisted_size; j++) {

Expected results:

No segfaults :)

Additional info:

Lets take a closer look on virNetTLSContextNew function:

 686 static virNetTLSContextPtr virNetTLSContextNew(const char *cacert,
 687                                                const char *cacrl,
 688                                                const char *cert,
 689                                                const char *key,
 690                                                const char *const*x509dnWhitelist,
 691                                                const char *priority,
 692                                                bool sanityCheckCert,
 693                                                bool requireValidCert,
 694                                                bool isServer)
 695 {
 696     virNetTLSContextPtr ctxt;
 697     int err;
 698
 699     if (virNetTLSContextInitialize() < 0)
 700         return NULL;
 701
 702     if (!(ctxt = virObjectLockableNew(virNetTLSContextClass)))
 703         return NULL;
 704
 705     if (VIR_STRDUP(ctxt->priority, priority) < 0)
 706         goto error;
 707
 708     err = gnutls_certificate_allocate_credentials(&ctxt->x509cred);
 709     if (err) {
 710         virReportError(VIR_ERR_SYSTEM_ERROR,
 711                        _("Unable to allocate x509 credentials: %s"),
 712                        gnutls_strerror(err));
 713         goto error;
 714     }
 
 ....
 
 758  error:
 759     if (isServer)
 760         gnutls_dh_params_deinit(ctxt->dhParams);
 761     gnutls_certificate_free_credentials(ctxt->x509cred);
 762     VIR_FREE(ctxt);
 763     return NULL;
 764 } 

Here we can see that segfault would occur in case when allocation fails in lines 705 or 708.
Additionally ctxt->priority it't freed if error occures at a later stage of the code,
which can leads to memory leak.

Proposed patch:

diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 72e9ed9..8f6ec8f 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -703,14 +703,14 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert,
         return NULL;

     if (VIR_STRDUP(ctxt->priority, priority) < 0)
-        goto error;
+        goto ctxt_init_error;

     err = gnutls_certificate_allocate_credentials(&ctxt->x509cred);
     if (err) {
         virReportError(VIR_ERR_SYSTEM_ERROR,
                        _("Unable to allocate x509 credentials: %s"),
                        gnutls_strerror(err));
-        goto error;
+        goto ctxt_init_error;
     }

     if (sanityCheckCert &&
@@ -759,6 +759,8 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert,
     if (isServer)
         gnutls_dh_params_deinit(ctxt->dhParams);
     gnutls_certificate_free_credentials(ctxt->x509cred);
+ ctxt_init_error:
+    if (ctxt->priority) VIR_FREE(ctxt->priority);
     VIR_FREE(ctxt);
     return NULL;
 }