Full_Name: Steve Langasek Version: 2.4.7 OS: Debian URL: http://people.ubuntu.com/~vorlon/gnutls-altname-nulterminated.patch Submission from: (NULL) (2001:4830:1244:0:219:d2ff:fe76:2acb) When built with GnuTLS, libldap fails to correctly verify DNS hostnames against the subjectAltName field of the provided certificate. The reason for this is that, while the "length" that gnutls returns for the CN is equal to the strlen(), the length returned by gnutls_x509_crt_get_subject_alt_name() includes a trailing NUL. I have verified that the referenced patch corrects this for the case of non-wildcard DNS subjectAltName values. I have not tested the code for the wildcarded case, though it seems likely that the same bug applies there and will need to be fixed.
steve.langasek@canonical.com wrote: > Full_Name: Steve Langasek > Version: 2.4.7 > OS: Debian > URL: http://people.ubuntu.com/~vorlon/gnutls-altname-nulterminated.patch > Submission from: (NULL) (2001:4830:1244:0:219:d2ff:fe76:2acb) > > > When built with GnuTLS, libldap fails to correctly verify DNS hostnames against > the subjectAltName field of the provided certificate. The reason for this is > that, while the "length" that gnutls returns for the CN is equal to the > strlen(), the length returned by gnutls_x509_crt_get_subject_alt_name() includes > a trailing NUL. > > I have verified that the referenced patch corrects this for the case of > non-wildcard DNS subjectAltName values. I have not tested the code for the > wildcarded case, though it seems likely that the same bug applies there and will > need to be fixed. I cannot duplicate this error with GnuTLS 1.7.8 or 1.7.9. The altname length that is returned just includes the non-NUL characters. Note that all of libldap's TLS functionality was tested and working with GnuTLS 1.7. What version are you using? It seems to me that if your version of GnuTLS is indeed behaving this way, then it's a GnuTLS bug, since in C, the length of a string never includes the trailing NUL. -- -- Howard Chu Chief Architect, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
changed notes changed state Open to Feedback
On Sat, Feb 09, 2008 at 11:04:18PM -0800, Howard Chu wrote: > I cannot duplicate this error with GnuTLS 1.7.8 or 1.7.9. The altname > length that is returned just includes the non-NUL characters. Note that > all of libldap's TLS functionality was tested and working with GnuTLS > 1.7. What version are you using? Reproduced with GnuTLS 2.0.4 and GnuTLS 2.2.1. > It seems to me that if your version of GnuTLS is indeed behaving this way, > then it's a GnuTLS bug, since in C, the length of a string never includes > the trailing NUL. It's true that the /length/ of a string doesn't include the trailing NUL, but it does have to be included in the storage /size/ of a C string, and it's debatable which is intended here. Given that one of the errors returned by gnutls_x509_crt_get_subject_alt_name() is GNUTLS_E_SHORT_MEMORY_BUFFER, it seems obvious to me that this should use semantics for storage size rather than string length, and the only question in my mind is whether the trailing NUL is included as part of the internal representation of the string. If this is a behavior change as you say, then I guess we need clarification from GnuTLS upstream about whether this is intentional.
Steve Langasek wrote: > On Sat, Feb 09, 2008 at 11:04:18PM -0800, Howard Chu wrote: >> I cannot duplicate this error with GnuTLS 1.7.8 or 1.7.9. The altname >> length that is returned just includes the non-NUL characters. Note that >> all of libldap's TLS functionality was tested and working with GnuTLS >> 1.7. What version are you using? > > Reproduced with GnuTLS 2.0.4 and GnuTLS 2.2.1. > >> It seems to me that if your version of GnuTLS is indeed behaving this way, >> then it's a GnuTLS bug, since in C, the length of a string never includes >> the trailing NUL. > > It's true that the /length/ of a string doesn't include the trailing NUL, > but it does have to be included in the storage /size/ of a C string, and > it's debatable which is intended here. Since this is an ASN.1 structure, one would ordinarily not expect any NUL termination in the first place. And since other GnuTLS library functions are returning the raw data size, excluding any trailing NUL, the behavior you're seeing here is pretty suspicious. > Given that one of the errors > returned by gnutls_x509_crt_get_subject_alt_name() is > GNUTLS_E_SHORT_MEMORY_BUFFER, it seems obvious to me that this should use > semantics for storage size rather than string length, and the only question > in my mind is whether the trailing NUL is included as part of the internal > representation of the string. > > If this is a behavior change as you say, then I guess we need clarification > from GnuTLS upstream about whether this is intentional. That sounds like the best step for now. Just to be sure, how was the certificate created? Have you verified that libldap with OpenSSL accepts the certificate correctly? So far it sounds just as likely to me that your subjectAltName actually includes a trailing NUL in its data. ASN.1 structures don't use NUL-terminated strings here, the DER form requires definite lengths to be encoded up front. -- -- Howard Chu Chief Architect, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
hyc@symas.com wrote: > Steve Langasek wrote: >> Given that one of the errors >> returned by gnutls_x509_crt_get_subject_alt_name() is >> GNUTLS_E_SHORT_MEMORY_BUFFER, it seems obvious to me that this should use >> semantics for storage size rather than string length, and the only question >> in my mind is whether the trailing NUL is included as part of the internal >> representation of the string. >> >> If this is a behavior change as you say, then I guess we need clarification >> from GnuTLS upstream about whether this is intentional. Yes. I've just tested with GnuTLS 2.2.1 and 2.3.0 and see the same result you're seeing. The change is here: http://git.savannah.gnu.org/gitweb/?p=gnutls.git;a=commitdiff;h=deaa3ac31c2e83c292562ab66c1817c7ebc27048 and it is clearly a bug, since subjectAltName's are not necessarily strings. (E.g., they can also be IP addresses, which are just 4 or 16 octets.) If you notice in the diff, they set *name_size = len + 1; and then later name[len] = 0; but this occurs *after* the check for SHORT_MEMORY_BUFFER. So in fact they can cause a write past the end of the supplied buffer. This patch should be reverted, it is clearly wrong. -- -- Howard Chu Chief Architect, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
On Sun, Feb 10, 2008 at 01:19:49AM -0800, Howard Chu wrote: >> It's true that the /length/ of a string doesn't include the trailing NUL, >> but it does have to be included in the storage /size/ of a C string, and >> it's debatable which is intended here. > Since this is an ASN.1 structure, one would ordinarily not expect any NUL > termination in the first place. And since other GnuTLS library functions > are returning the raw data size, excluding any trailing NUL, the behavior > you're seeing here is pretty suspicious. Well, granted; it is consistent in newer versions of GnuTLS, though, despite being inconsistent with older versions and with the other crt APIs. >> Given that one of the errors >> returned by gnutls_x509_crt_get_subject_alt_name() is >> GNUTLS_E_SHORT_MEMORY_BUFFER, it seems obvious to me that this should use >> semantics for storage size rather than string length, and the only question >> in my mind is whether the trailing NUL is included as part of the internal >> representation of the string. >> If this is a behavior change as you say, then I guess we need clarification >> from GnuTLS upstream about whether this is intentional. > That sounds like the best step for now. Just to be sure, how was the > certificate created? Using the openssl commandline tool, and specifying a subjectAltName=DNS:hostname entry in the [ v3_ca ] section of /etc/ssl/openssl.cnf. This was created strictly as a test certificate, in response to reports of problems precisely with subjectAltName validation (http://bugs.debian.org/462588#98). > Have you verified that libldap with OpenSSL accepts the certificate > correctly? Yes, I've tested with ldapsearch from OpenLDAP 2.3.30 linked against OpenSSL, which was able to validate the subjectAltName just fine. Cheers, -- Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. Ubuntu Developer http://www.debian.org/ slangasek@ubuntu.com vorlon@debian.org
changed notes
A fix has been committed in the GnuTLS code base. -------- Original Message -------- Subject: Re: (ITS#5361) cert verification failures with GnuTLS and DNS subjectAltName Date: Fri, 15 Feb 2008 23:11:32 +0200 From: Nikos Mavrogiannopoulos <nmav@gnutls.org> To: Howard Chu <hyc@symas.com> CC: Joe Orton <joe@manyfish.co.uk>, gnutls-devel@gnu.org References: <200802100917.m1A9HkSb015171@boole.openldap.org> <200802152216.25025.nmav@gnutls.org> <47B5F843.8080503@symas.com> On Friday 15 February 2008, Howard Chu wrote: > > Anyway, does the attached > > patch solve your problem? > > Not really. It still returns a size one byte larger than expected for the > strings. Even in languages where NUL-terminated strings are the norm, the > terminating byte is not included in the length. > > The point is, we expect this API to return exactly the data that was in the > certificate. If the caller wants to treat the data as a string, they can > NUL-terminate it themselves. The manpage only says that the data will be > returned, it does not say that it will be altered in any way. Actually you are right. The return value shouldn't be increased (this also happens in the other similar functions). I've corrected the patch and commited at: http://git.savannah.gnu.org/gitweb/?p=gnutls.git;a=commitdiff;h=4cc3c6b6ed00660e55559bab148021fc077da21f regards, Nikos -- -- Howard Chu Chief Architect, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
Howard Chu wrote: > A fix has been committed in the GnuTLS code base. For reference, the discussion on gnutls-devel was here http://lists.gnu.org/archive/html/gnutls-devel/2008-02/msg00005.html -- -- Howard Chu Chief Architect, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
changed state Feedback to Closed
Bug in GnuTLS 2.1.8-2.3.0