| Message ID | CAKoLUM5EeuaP7Oa0U0jHW_JHE8_GHCuCBnFyORK-T+8U-dZc1A@mail.gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, 2015-03-03 at 19:49 -0600, Rohit Agrawal wrote: > Hi all, > > wpa_supplicant allows one to specify the sha256 hash of a certificate, > which is currently used (in src/crypto/tls_openssl.c lines 1506-1540) > to do the following: > * if a leaf certificate is valid and the certificate is pinned, don't > check the full chain (lines 1506-1507) > * if the leaf certificate is valid and the provided certificate > doesn't match the pinned one, reject it (lines 1518-1541). > > I would like to propose that the behavior be modified slightly to add > the following: > * if openssl reports that the leaf certificate is _invalid_ but it > matches the pinned certificate, accept it > > My use case is connecting to a RADIUS server I do not have control > over with certificate chain problems, but due to other out-of-band > reasons I trust the leaf certificate. Currently, even if I pin the > certificate, wpa_supplicant rejects it because openssl reports that > the certificate is invalid, and wpa_supplicant then ignores the fact > that I specified a pinned cert. > > If this is acceptable, the following patch implements the behavior change: > > From a568b8dacf2ba3c534a278217cb22719bda73c41 Mon Sep 17 00:00:00 2001 > From: Rohit Agrawal <rohit.agrawal.mn@gmail.com> > Date: Tue, 3 Mar 2015 19:44:52 -0600 > Subject: [PATCH] Pinned certificates are always accepted > > If OpenSSL reports that a presented leaf certificate is invalid > but it has been explicitly pinned, accept it anyway. > > Signed-off-by: Rohit Agrawal <rohit.agrawal.mn@gmail.com> > --- > src/crypto/tls_openssl.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/src/crypto/tls_openssl.c b/src/crypto/tls_openssl.c > index 46c4a46..f4b7620 100644 > --- a/src/crypto/tls_openssl.c > +++ b/src/crypto/tls_openssl.c > @@ -1516,7 +1516,9 @@ static int tls_verify_cb(int preverify_ok, > X509_STORE_CTX *x509_ctx) > err_str = X509_verify_cert_error_string(err); > > #ifdef CONFIG_SHA256 > - if (preverify_ok && depth == 0 && conn->server_cert_only) { > + // don't require preverify_ok so we can explicity allow otherwise > + // invalid pinned certs Use "C" style comments, not C++ style ones... > + if (depth == 0 && conn->server_cert_only) { I think your mailer mangled the whitespace in this patch. If it's got a "preformatted" option, try that when pasting in the patch itself? Dan > struct wpabuf *cert; > cert = get_x509_cert(err_cert); > if (!cert) { > @@ -1534,6 +1536,10 @@ static int tls_verify_cb(int preverify_ok, > X509_STORE_CTX *x509_ctx) > err_str = "Server certificate mismatch"; > err = X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN; > preverify_ok = 0; > + } else { > + // certificate matches pinned certificate, allow > + // regardless of other problems > + preverify_ok = 1; > } > wpabuf_free(cert); > }
diff --git a/src/crypto/tls_openssl.c b/src/crypto/tls_openssl.c index 46c4a46..f4b7620 100644 --- a/src/crypto/tls_openssl.c +++ b/src/crypto/tls_openssl.c @@ -1516,7 +1516,9 @@ static int tls_verify_cb(int preverify_ok, X509_STORE_CTX *x509_ctx) err_str = X509_verify_cert_error_string(err); #ifdef CONFIG_SHA256 - if (preverify_ok && depth == 0 && conn->server_cert_only) { + // don't require preverify_ok so we can explicity allow otherwise + // invalid pinned certs + if (depth == 0 && conn->server_cert_only) { struct wpabuf *cert; cert = get_x509_cert(err_cert);