The _gnutls_x509_verify_certificate fix
simon at josefsson.org
Tue Nov 11 16:46:27 CET 2008
Simon Josefsson <simon at josefsson.org> writes:
> Simon Josefsson <simon at josefsson.org> writes:
>> Tomas Mraz <tmraz at redhat.com> writes:
>>> On Mon, 2008-11-10 at 21:04 +0100, Nikos Mavrogiannopoulos wrote:
>>>> On Mon, Nov 10, 2008 at 2:47 PM, Tomas Mraz <tmraz at redhat.com> wrote:
>>>> > Hello,
>>>> > given the recent fix in the _gnutls_x509_verify_certificate I have been
>>>> > looking at the function. I see there are currently some limitations in
>>>> > it. For example it now doesn't allow verification of explicitely trusted
>>>> > self-signed site certificate. Is there some other method how this could
>>>> > be achieved?
>>>> You can achieve it by associating an address of a website with the
>>>> keyid of the given
>>>> certificate. This is more generic of trusting a self-signed
>>>> certificate. You can trust any
>>>> certificate first presented when accessing a website that way (ssh security).
>>> But the patch should be modified anyway because in case the server
>>> presents just a self-signed site certificate there will be a dereference
>>> of the certificate_list[-1].
>>> It is also questionable whether the function should not also check for
>>> clist_size of 0 before calling _gnutls_verify_certificate2().
>> Indeed. This may explain:
> On reading more code, probably not, since all calls to
> _gnutls_x509_verify_certificate goes through gnutls_x509_crt_list_verify
> which does check for clist_size==0. For readability, the two functions
> should probably be merged into one.
I was mistaken, the problem is when clist_size==1, then the function
decrements it and then references clist_size-1 which is bad.
So the patch in v2.6.1 and in the advisory causes crashes when the chain
contains just one self-signed certificate. Ouch. Thinking more about
this, I suspect the entire optimization to remove the final certificate
is pointless. So this patch (against v2.6.1) should also work:
diff --git a/lib/x509/verify.c b/lib/x509/verify.c
index 8fa90dc..92ef722 100644
@@ -374,17 +374,6 @@ _gnutls_x509_verify_certificate (const gnutls_x509_crt_t * certificate_list,
int i = 0, ret;
unsigned int status = 0, output;
- /* Check if the last certificate in the path is self signed.
- * In that case ignore it (a certificate is trusted only if it
- * leads to a trusted party by us, not the server's).
- if (gnutls_x509_crt_check_issuer (certificate_list[clist_size - 1],
- certificate_list[clist_size - 1]) > 0
- && clist_size > 0)
/* Verify the last certificate in the certificate path
* against the trusted CA certificate list.
The self-test needs to be modified slightly to accommodate this, because
now the validation returns without the GNUTLS_CERT_SIGNER_NOT_FOUND
flag, which is arguable more correct (?) since the problem is actually
because of failing verification.
Nikos, any thoughts?
Btw, I've updated cve-2008-4989.c in git, now it tests both the chain
that caused incorrect validation and a self-signed chain that causes a
More information about the Gnutls-devel