[gnutls-dev] thread safety issue in gnutls_certificate_verify_peers2()

[Rupert Kittinger]

Hi everybody,

I have found an unexpected race condition in using gnutls in an 
multithreaded application.

It is a multithreaded tcp server, where each thread handles its own 
session. It uses x509 certificates and requires client certification.
All threads share the same certificate_credentials structure. I would 
have expected that the credentials structure is read-only and does not 
need explicit locking, but unfortunately...

Under heavy load, the server crashes reproducably in 
gnutls_certificate_verify_peers2(). After some debugging, I found the 
following:

asn1_der_coding() seems to modify its first argument (ASN1_TYPE element) 
when encoding seuences or sets. e.g.


    case TYPE_SEQUENCE_OF: case TYPE_SET_OF:
      if(move!=UP){
        _asn1_ltostr(counter,temp);
        tlen = strlen(temp);
        
        if (tlen > 0)
           _asn1_set_value(p,temp,tlen+1);
        p=p->down;
        ...

where p seems to point inside the asn1 structure. The program crashes 
when _asn1_set_value() frees the node while another thread is reading 
the same structure and tries to dereference a null pointer.

What is the reason for writing inside the structure? I admit I did not 
tr yto understand the der encoding code in detail, it looks quite 
intimidating :-)

Protecting the call to gnutls_certificate_verify_peers2() is an 
acceptable workaround, but I would suggest fixing this issue inside 
libtasn1. At the very least, there should be a hint inside the 
documentation that credentials should not be shared between threads.
The window of opportunity for this race condition is very short, and I 
have not yet been able to reproduce it on a single-cpu machine :-(

cheers,
Rupert

PS: thanks for all your efforts!


--
Rupert Kittinger <address@hidden>
Krenngasse 32
A-8010 Graz
Austria