X.509 certificate verification in GNU TLS Library

[Guido Trentalancia]

Hello,

I have tested the current GNU TLS Library against the issue reported at
http://article.gmane.org/gmane.comp.encryption.gpg.gnutls.devel/3517 and
I believe the function _gnutls_x509_verify_certificate() in
lib/x509/verify.c needs to be modified according to the attached patch
in order for the certificate verification to work properly.

In fact, at the moment (version 2.8.4 and at least since the problem was
originally reported against branch 2.4.x as GNUTLS-SA-2009-3), the
certificate verification function returns the status after each check,
which implies that not all checks in _gnutls_x509_verify_certificate()
are necessarily performed. I believe the correct behaviour is that all
checks need to be performed (and stored in the variable "status" using
logical OR) and that the result in the variable "status" need to be
returned only then.

After the attached patch is applied, the function returns only at the
end, after all the checks have been performed (and the result contained
in the variable "status" is the logical OR of the results of each check
performed).

What I get is that only using this patch, the behaviour is consistent
with the expected results, as they have been outlined in the article
mentioned above.

Could somebody please double-check and eventually confirm ? Thanks.

[gnutls-2.8.4-cert-verification-return-status.patch]

---

--- gnutls-2.8.4-buggy/lib/x509/verify.c 2009-09-27 20:55:53.000000000 +0200
+++ gnutls-2.8.4/lib/x509/verify.c 2009-09-27 20:42:00.000000000 +0200
@@ -474,7 +474,6 @@ _gnutls_x509_verify_certificate (const g
       gnutls_assert ();
       status |= output;
       status |= GNUTLS_CERT_INVALID;
-      return status;
     }
 
   /* Check for revoked certificates in the chain
@@ -488,7 +487,6 @@ _gnutls_x509_verify_certificate (const g
  { /* revoked */
   status |= GNUTLS_CERT_REVOKED;
   status |= GNUTLS_CERT_INVALID;
-  return status;
  }
     }
 #endif
@@ -506,7 +504,6 @@ _gnutls_x509_verify_certificate (const g
     {
       status |= GNUTLS_CERT_NOT_ACTIVATED;
       status |= GNUTLS_CERT_INVALID;
-      return status;
     }
 
   t = gnutls_x509_crt_get_expiration_time (certificate_list[i]);
@@ -514,7 +511,6 @@ _gnutls_x509_verify_certificate (const g
     {
       status |= GNUTLS_CERT_EXPIRED;
       status |= GNUTLS_CERT_INVALID;
-      return status;
     }
  }
     }
@@ -535,13 +531,10 @@ _gnutls_x509_verify_certificate (const g
    _gnutls_verify_certificate2 (certificate_list[i - 1],
  &certificate_list[i], 1, flags,
  NULL)) == 0)
- {
-  status |= GNUTLS_CERT_INVALID;
-  return status;
- }
+        status |= GNUTLS_CERT_INVALID;
     }
 
-  return 0;
+  return status;
 }
 
 

---

_______________________________________________
Gnutls-devel mailing list
Gnutls-devel@...
http://lists.gnu.org/mailman/listinfo/gnutls-devel

[Simon Josefsson-2]

Guido Trentalancia <guido@...> writes:

> Hello,
>
> I have tested the current GNU TLS Library against the issue reported at
> http://article.gmane.org/gmane.comp.encryption.gpg.gnutls.devel/3517 and
> I believe the function _gnutls_x509_verify_certificate() in
> lib/x509/verify.c needs to be modified according to the attached patch
> in order for the certificate verification to work properly.
>
> In fact, at the moment (version 2.8.4 and at least since the problem was
> originally reported against branch 2.4.x as GNUTLS-SA-2009-3), the
> certificate verification function returns the status after each check,
> which implies that not all checks in _gnutls_x509_verify_certificate()
> are necessarily performed. I believe the correct behaviour is that all
> checks need to be performed (and stored in the variable "status" using
> logical OR) and that the result in the variable "status" need to be
> returned only then.
>
> After the attached patch is applied, the function returns only at the
> end, after all the checks have been performed (and the result contained
> in the variable "status" is the logical OR of the results of each check
> performed).
>
> What I get is that only using this patch, the behaviour is consistent
> with the expected results, as they have been outlined in the article
> mentioned above.
>
> Could somebody please double-check and eventually confirm ? Thanks.

Some test vectors would help to reinforce and explain your point, do you
have a test X.509 chain that validates incorrectly that you could post?

/Simon


_______________________________________________
Gnutls-devel mailing list
Gnutls-devel@...
http://lists.gnu.org/mailman/listinfo/gnutls-devel