[openssl.org #2836] [PATCH] Staple the correct OCSP Response when multiple certs are configured
Closing item as resolved. SteveH committed patches across all branches. Tim __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [openssl.org #2836] [PATCH] Staple the correct OCSP Response when multiple certs are configured
On 21/09/12 15:38, Rob Stradling via RT wrote: > On 21/09/12 15:12, Rob Stradling via RT wrote: >> On 21/09/12 15:04, Stephen Henson via RT wrote: > >>> Easiest solution is to also backport ssl_get_server_send_pkey see: >>> >>> http://cvs.openssl.org/chngview?cn=22840 >> >> I didn't think of that. Thanks! >> >> I'll prepare patches to backport 22840 to 1.0.0 and 0.9.8 (unless you or >> Ben get there first). > > http://cvs.openssl.org/patchset?cn=22840 applies cleanly (i.e. no failed > hunks) on top of my patches for 1.0.0 and 0.9.8. Steve, Ben committed my patch to the 1.0.0 branch yesterday: http://cvs.openssl.org/chngview?cn=22875 So, please would you now apply http://cvs.openssl.org/patchset?cn=22840 to the 1.0.0 branch? Thanks. -- Rob Stradling Senior Research & Development Scientist COMODO - Creating Trust Online __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [openssl.org #2836] [PATCH] Staple the correct OCSP Response when multiple certs are configured
On 21/09/12 15:12, Rob Stradling via RT wrote: On 21/09/12 15:04, Stephen Henson via RT wrote: Easiest solution is to also backport ssl_get_server_send_pkey see: http://cvs.openssl.org/chngview?cn=22840 I didn't think of that. Thanks! I'll prepare patches to backport 22840 to 1.0.0 and 0.9.8 (unless you or Ben get there first). http://cvs.openssl.org/patchset?cn=22840 applies cleanly (i.e. no failed hunks) on top of my patches for 1.0.0 and 0.9.8. -- Rob Stradling Senior Research & Development Scientist COMODO - Creating Trust Online __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [openssl.org #2836] [PATCH] Staple the correct OCSP Response when multiple certs are configured
On 21/09/12 15:04, Stephen Henson via RT wrote:
[rob.stradl...@comodo.com - Fri Sep 21 15:55:39 2012]:
Hi Steve.
I saw your update (to 1.0.2 and HEAD), and I did start looking at
backporting it into my 1.0.1/1.0.0/0.9.8 patches.
ssl_get_server_send_pkey() is not available in 1.0.1 and earlier, so the
t1_lib.c patch would have to be something like...
+ X509 *x;
+ x = ssl_get_server_send_cert)s);
+ /* If no certificate can't return certificate status */
+ if (x == NULL)
+ {
+ s->tlsext_status_expected = 0;
+ return 1;
+ }
+ /* Set current certificate to one we will use so
+* SSL_get_certificate et al can pick it up.
+*/
+ s->cert->key->x509 = x;
Is it OK to update s->cert->key->x509 like this?
No because you could end up with all sorts of bad things happening (keys
and certificates not matching, certificate types not matching and memory
leaks).
That's what I thought.
Easiest solution is to also backport ssl_get_server_send_pkey see:
http://cvs.openssl.org/chngview?cn=22840
I didn't think of that. Thanks!
I'll prepare patches to backport 22840 to 1.0.0 and 0.9.8 (unless you or
Ben get there first).
--
Rob Stradling
Senior Research & Development Scientist
COMODO - Creating Trust Online
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majord...@openssl.org
Re: [openssl.org #2836] [PATCH] Staple the correct OCSP Response when multiple certs are configured
Hi Steve.
I saw your update (to 1.0.2 and HEAD), and I did start looking at
backporting it into my 1.0.1/1.0.0/0.9.8 patches.
ssl_get_server_send_pkey() is not available in 1.0.1 and earlier, so the
t1_lib.c patch would have to be something like...
+ X509 *x;
+ x = ssl_get_server_send_cert)s);
+ /* If no certificate can't return certificate status */
+ if (x == NULL)
+ {
+ s->tlsext_status_expected = 0;
+ return 1;
+ }
+ /* Set current certificate to one we will use so
+* SSL_get_certificate et al can pick it up.
+*/
+ s->cert->key->x509 = x;
Is it OK to update s->cert->key->x509 like this?
On 21/09/12 14:34, Stephen Henson via RT wrote:
[rob.stradl...@comodo.com - Fri Sep 21 15:02:54 2012]:
Attached are patches for 1.0.0 and 0.9.8.
Note, I updated the original change to retain compatibility with
existing behaviour as far as possible. See:
http://cvs.openssl.org/chngview?cn=22808
Steve.
--
Rob Stradling
Senior Research & Development Scientist
COMODO - Creating Trust Online
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majord...@openssl.org
Re: [openssl.org #2836] [PATCH] Staple the correct OCSP Response when multiple certs are configured
On 21/09/12 15:12, Rob Stradling via RT wrote: > On 21/09/12 15:04, Stephen Henson via RT wrote: >> Easiest solution is to also backport ssl_get_server_send_pkey see: >> >> http://cvs.openssl.org/chngview?cn=22840 > > I didn't think of that. Thanks! > > I'll prepare patches to backport 22840 to 1.0.0 and 0.9.8 (unless you or > Ben get there first). http://cvs.openssl.org/patchset?cn=22840 applies cleanly (i.e. no failed hunks) on top of my patches for 1.0.0 and 0.9.8. -- Rob Stradling Senior Research & Development Scientist COMODO - Creating Trust Online __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [openssl.org #2836] [PATCH] Staple the correct OCSP Response when multiple certs are configured
On 21/09/12 15:04, Stephen Henson via RT wrote:
>> [rob.stradl...@comodo.com - Fri Sep 21 15:55:39 2012]:
>>
>> Hi Steve.
>>
>> I saw your update (to 1.0.2 and HEAD), and I did start looking at
>> backporting it into my 1.0.1/1.0.0/0.9.8 patches.
>>
>> ssl_get_server_send_pkey() is not available in 1.0.1 and earlier, so the
>> t1_lib.c patch would have to be something like...
>>
>> +X509 *x;
>> +x = ssl_get_server_send_cert)s);
>> +/* If no certificate can't return certificate status */
>> +if (x == NULL)
>> +{
>> +s->tlsext_status_expected = 0;
>> +return 1;
>> +}
>> +/* Set current certificate to one we will use so
>> + * SSL_get_certificate et al can pick it up.
>> + */
>> +s->cert->key->x509 = x;
>>
>> Is it OK to update s->cert->key->x509 like this?
>>
>
> No because you could end up with all sorts of bad things happening (keys
> and certificates not matching, certificate types not matching and memory
> leaks).
That's what I thought.
> Easiest solution is to also backport ssl_get_server_send_pkey see:
>
> http://cvs.openssl.org/chngview?cn=22840
I didn't think of that. Thanks!
I'll prepare patches to backport 22840 to 1.0.0 and 0.9.8 (unless you or
Ben get there first).
--
Rob Stradling
Senior Research & Development Scientist
COMODO - Creating Trust Online
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majord...@openssl.org
[openssl.org #2836] [PATCH] Staple the correct OCSP Response when multiple certs are configured
> [rob.stradl...@comodo.com - Fri Sep 21 15:55:39 2012]:
>
> Hi Steve.
>
> I saw your update (to 1.0.2 and HEAD), and I did start looking at
> backporting it into my 1.0.1/1.0.0/0.9.8 patches.
>
> ssl_get_server_send_pkey() is not available in 1.0.1 and earlier, so the
> t1_lib.c patch would have to be something like...
>
> + X509 *x;
> + x = ssl_get_server_send_cert)s);
> + /* If no certificate can't return certificate status */
> + if (x == NULL)
> + {
> + s->tlsext_status_expected = 0;
> + return 1;
> + }
> + /* Set current certificate to one we will use so
> + * SSL_get_certificate et al can pick it up.
> + */
> + s->cert->key->x509 = x;
>
> Is it OK to update s->cert->key->x509 like this?
>
No because you could end up with all sorts of bad things happening (keys
and certificates not matching, certificate types not matching and memory
leaks). Easiest solution is to also backport ssl_get_server_send_pkey see:
http://cvs.openssl.org/chngview?cn=22840
Steve.
--
Dr Stephen N. Henson. OpenSSL project core developer.
Commercial tech support now available see: http://www.openssl.org
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majord...@openssl.org
Re: [openssl.org #2836] [PATCH] Staple the correct OCSP Response when multiple certs are configured
Hi Steve.
I saw your update (to 1.0.2 and HEAD), and I did start looking at
backporting it into my 1.0.1/1.0.0/0.9.8 patches.
ssl_get_server_send_pkey() is not available in 1.0.1 and earlier, so the
t1_lib.c patch would have to be something like...
+ X509 *x;
+ x = ssl_get_server_send_cert)s);
+ /* If no certificate can't return certificate status */
+ if (x == NULL)
+ {
+ s->tlsext_status_expected = 0;
+ return 1;
+ }
+ /* Set current certificate to one we will use so
+* SSL_get_certificate et al can pick it up.
+*/
+ s->cert->key->x509 = x;
Is it OK to update s->cert->key->x509 like this?
On 21/09/12 14:34, Stephen Henson via RT wrote:
>> [rob.stradl...@comodo.com - Fri Sep 21 15:02:54 2012]:
>>
>> Attached are patches for 1.0.0 and 0.9.8.
>>
>>
>
> Note, I updated the original change to retain compatibility with
> existing behaviour as far as possible. See:
>
> http://cvs.openssl.org/chngview?cn=22808
>
> Steve.
>
--
Rob Stradling
Senior Research & Development Scientist
COMODO - Creating Trust Online
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majord...@openssl.org
[openssl.org #2836] [PATCH] Staple the correct OCSP Response when multiple certs are configured
> [rob.stradl...@comodo.com - Fri Sep 21 15:02:54 2012]: > > Attached are patches for 1.0.0 and 0.9.8. > > Note, I updated the original change to retain compatibility with existing behaviour as far as possible. See: http://cvs.openssl.org/chngview?cn=22808 Steve. -- Dr Stephen N. Henson. OpenSSL project core developer. Commercial tech support now available see: http://www.openssl.org __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [openssl.org #2836] [PATCH] Staple the correct OCSP Response when multiple certs are configured
Attached are patches for 1.0.0 and 0.9.8.
--
Rob Stradling
Senior Research & Development Scientist
COMODO - Creating Trust Online
Office Tel: +44.(0)1274.730505
Office Fax: +44.(0)1274.730909
www.comodo.com
COMODO CA Limited, Registered in England No. 04058690
Registered Office:
3rd Floor, 26 Office Village, Exchange Quay,
Trafford Road, Salford, Manchester M5 3EQ
This e-mail and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they are
addressed. If you have received this email in error please notify the
sender by replying to the e-mail containing this attachment. Replies to
this email may be monitored by COMODO for operational or business
reasons. Whilst every endeavour is taken to ensure that e-mails are free
from viruses, no liability can be accepted and the recipient is
requested to use their own virus checking software.
Index: ssl/s3_srvr.c
===
RCS file: /v/openssl/cvs/openssl/ssl/s3_srvr.c,v
retrieving revision 1.126.2.42
diff -u -r1.126.2.42 s3_srvr.c
--- ssl/s3_srvr.c 16 Feb 2012 15:21:17 - 1.126.2.42
+++ ssl/s3_srvr.c 20 Sep 2012 14:40:41 -
@@ -1005,7 +1005,7 @@
goto f_err;
}
}
- if (ssl_check_clienthello_tlsext(s) <= 0) {
+ if (ssl_check_clienthello_tlsext_early(s) <= 0) {
SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,SSL_R_CLIENTHELLO_TLSEXT);
goto err;
}
@@ -1131,6 +1131,16 @@
* s->tmp.new_cipher- the new cipher to use.
*/
+ /* Handles TLS extensions that we couldn't check earlier */
+ if (s->version >= SSL3_VERSION)
+ {
+ if (ssl_check_clienthello_tlsext_late(s) <= 0)
+ {
+
SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,SSL_R_CLIENTHELLO_TLSEXT);
+ goto err;
+ }
+ }
+
if (ret < 0) ret=1;
if (0)
{
Index: ssl/ssl_lib.c
===
RCS file: /v/openssl/cvs/openssl/ssl/ssl_lib.c,v
retrieving revision 1.133.2.31
diff -u -r1.133.2.31 ssl_lib.c
--- ssl/ssl_lib.c 5 Jan 2012 10:21:49 - 1.133.2.31
+++ ssl/ssl_lib.c 20 Sep 2012 14:40:41 -
@@ -1943,7 +1943,7 @@
}
/* THIS NEEDS CLEANING UP */
-X509 *ssl_get_server_send_cert(SSL *s)
+X509 *ssl_get_server_send_cert(const SSL *s)
{
unsigned long alg,kalg;
CERT *c;
@@ -2420,7 +2420,9 @@
/* Fix this function so that it takes an optional type parameter */
X509 *SSL_get_certificate(const SSL *s)
{
- if (s->cert != NULL)
+ if (s->server)
+ return(ssl_get_server_send_cert(s));
+ else if (s->cert != NULL)
return(s->cert->key->x509);
else
return(NULL);
Index: ssl/ssl_locl.h
===
RCS file: /v/openssl/cvs/openssl/ssl/ssl_locl.h,v
retrieving revision 1.63.2.22
diff -u -r1.63.2.22 ssl_locl.h
--- ssl/ssl_locl.h 9 Mar 2012 15:51:56 - 1.63.2.22
+++ ssl/ssl_locl.h 20 Sep 2012 14:40:41 -
@@ -740,7 +740,7 @@
int ssl_undefined_function(SSL *s);
int ssl_undefined_void_function(void);
int ssl_undefined_const_function(const SSL *s);
-X509 *ssl_get_server_send_cert(SSL *);
+X509 *ssl_get_server_send_cert(const SSL *);
EVP_PKEY *ssl_get_sign_pkey(SSL *,SSL_CIPHER *);
int ssl_cert_type(X509 *x,EVP_PKEY *pkey);
void ssl_set_cert_masks(CERT *c, SSL_CIPHER *cipher);
@@ -979,7 +979,8 @@
int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **data, unsigned char
*d, int n, int *al);
int ssl_prepare_clienthello_tlsext(SSL *s);
int ssl_prepare_serverhello_tlsext(SSL *s);
-int ssl_check_clienthello_tlsext(SSL *s);
+int ssl_check_clienthello_tlsext_early(SSL *s);
+int ssl_check_clienthello_tlsext_late(SSL *s);
int ssl_check_serverhello_tlsext(SSL *s);
#ifdef OPENSSL_NO_SHA256
Index: ssl/t1_lib.c
===
RCS file: /v/openssl/cvs/openssl/ssl/t1_lib.c,v
retrieving revision 1.13.2.30
diff -u -r1.13.2.30 t1_lib.c
--- ssl/t1_lib.c4 Jan 2012 14:25:10 - 1.13.2.30
+++ ssl/t1_lib.c20 Sep 2012 14:40:41 -
@@ -745,7 +745,7 @@
return 1;
}
-int ssl_check_clienthello_tlsext(SSL *s)
+int ssl_check_clienthello_tlsext_early(SSL *s)
{
int ret=SSL_TLSEXT_ERR_NOACK;
int al = SSL_AD_UNRECOGNIZED_NAME;
@@ -755,11 +755,34 @@
else if (s->initial_ctx != NULL &&
s->initial_ctx->tlsext_servername_callback != 0)
ret = s->initial_ctx->tlsext_servername_callback(s, &al,
s->initial_ctx->tlsext_servername_arg);
+ switch (ret)
+ {
+
Re: [openssl.org #2836] [PATCH] Staple the correct OCSP Response when multiple certs are configured
On 07/09/12 11:51, Rob Stradling wrote:
> Attached is an updated patch for CVS HEAD, plus a patch for the 1.0.2
> branch.
>
> Are you still accepting patches for 1.0.1?
Attached is a patch for 1.0.1.
--
Rob Stradling
Senior Research & Development Scientist
COMODO - Creating Trust Online
Index: ssl/s3_srvr.c
===
RCS file: /v/openssl/cvs/openssl/ssl/s3_srvr.c,v
retrieving revision 1.171.2.21.2.26
diff -u -r1.171.2.21.2.26 s3_srvr.c
--- ssl/s3_srvr.c 8 Jun 2012 09:18:46 - 1.171.2.21.2.26
+++ ssl/s3_srvr.c 12 Sep 2012 15:45:12 -
@@ -1183,7 +1183,7 @@
goto f_err;
}
}
- if (ssl_check_clienthello_tlsext(s) <= 0) {
+ if (ssl_check_clienthello_tlsext_early(s) <= 0) {
SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,SSL_R_CLIENTHELLO_TLSEXT);
goto err;
}
@@ -1405,6 +1405,16 @@
* s->tmp.new_cipher- the new cipher to use.
*/
+ /* Handles TLS extensions that we couldn't check earlier */
+ if (s->version >= SSL3_VERSION)
+ {
+ if (ssl_check_clienthello_tlsext_late(s) <= 0)
+ {
+
SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,SSL_R_CLIENTHELLO_TLSEXT);
+ goto err;
+ }
+ }
+
if (ret < 0) ret=1;
if (0)
{
Index: ssl/ssl_lib.c
===
RCS file: /v/openssl/cvs/openssl/ssl/ssl_lib.c,v
retrieving revision 1.176.2.19.2.25
diff -u -r1.176.2.19.2.25 ssl_lib.c
--- ssl/ssl_lib.c 8 Jun 2012 09:18:46 - 1.176.2.19.2.25
+++ ssl/ssl_lib.c 12 Sep 2012 15:45:12 -
@@ -2287,7 +2287,7 @@
#endif
/* THIS NEEDS CLEANING UP */
-X509 *ssl_get_server_send_cert(SSL *s)
+X509 *ssl_get_server_send_cert(const SSL *s)
{
unsigned long alg_k,alg_a;
CERT *c;
@@ -2780,7 +2780,9 @@
/* Fix this function so that it takes an optional type parameter */
X509 *SSL_get_certificate(const SSL *s)
{
- if (s->cert != NULL)
+ if (s->server)
+ return(ssl_get_server_send_cert(s));
+ else if (s->cert != NULL)
return(s->cert->key->x509);
else
return(NULL);
Index: ssl/ssl_locl.h
===
RCS file: /v/openssl/cvs/openssl/ssl/ssl_locl.h,v
retrieving revision 1.100.2.10.2.17
diff -u -r1.100.2.10.2.17 ssl_locl.h
--- ssl/ssl_locl.h 9 Mar 2012 15:52:20 - 1.100.2.10.2.17
+++ ssl/ssl_locl.h 12 Sep 2012 15:45:12 -
@@ -830,7 +830,7 @@
int ssl_undefined_function(SSL *s);
int ssl_undefined_void_function(void);
int ssl_undefined_const_function(const SSL *s);
-X509 *ssl_get_server_send_cert(SSL *);
+X509 *ssl_get_server_send_cert(const SSL *);
EVP_PKEY *ssl_get_sign_pkey(SSL *s,const SSL_CIPHER *c, const EVP_MD **pmd);
int ssl_cert_type(X509 *x,EVP_PKEY *pkey);
void ssl_set_cert_masks(CERT *c, const SSL_CIPHER *cipher);
@@ -1088,7 +1088,8 @@
int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **data, unsigned char
*d, int n, int *al);
int ssl_prepare_clienthello_tlsext(SSL *s);
int ssl_prepare_serverhello_tlsext(SSL *s);
-int ssl_check_clienthello_tlsext(SSL *s);
+int ssl_check_clienthello_tlsext_early(SSL *s);
+int ssl_check_clienthello_tlsext_late(SSL *s);
int ssl_check_serverhello_tlsext(SSL *s);
#ifndef OPENSSL_NO_HEARTBEATS
Index: ssl/t1_lib.c
===
RCS file: /v/openssl/cvs/openssl/ssl/t1_lib.c,v
retrieving revision 1.64.2.14.2.33
diff -u -r1.64.2.14.2.33 t1_lib.c
--- ssl/t1_lib.c27 Jun 2012 14:11:40 - 1.64.2.14.2.33
+++ ssl/t1_lib.c12 Sep 2012 15:45:12 -
@@ -1763,7 +1763,7 @@
return 1;
}
-int ssl_check_clienthello_tlsext(SSL *s)
+int ssl_check_clienthello_tlsext_early(SSL *s)
{
int ret=SSL_TLSEXT_ERR_NOACK;
int al = SSL_AD_UNRECOGNIZED_NAME;
@@ -1782,42 +1782,12 @@
else if (s->initial_ctx != NULL &&
s->initial_ctx->tlsext_servername_callback != 0)
ret = s->initial_ctx->tlsext_servername_callback(s, &al,
s->initial_ctx->tlsext_servername_arg);
- /* If status request then ask callback what to do.
-* Note: this must be called after servername callbacks in case
-* the certificate has changed.
-*/
- if ((s->tlsext_status_type != -1) && s->ctx && s->ctx->tlsext_status_cb)
- {
- int r;
- r = s->ctx->tlsext_status_cb(s, s->ctx->tlsext_status_arg);
- switch (r)
- {
- /* We don't want to send a status request response */
- case SSL_TLSEXT_ERR_NOACK:
-
Re: [openssl.org #2836] [PATCH] Staple the correct OCSP Response when multiple certs are configured
Attached is an updated patch for CVS HEAD, plus a patch for the 1.0.2
branch.
Are you still accepting patches for 1.0.1?
Any chance of reviewing these patches soon?
Thanks.
On 19/06/12 21:15, Rob Stradling via RT wrote:
> The OCSP Stapling Callback function (s->ctx->tlsext_status_cb) is called
> during the parsing of the ClientHello message, before the server has
> decided which cipher to use. However, since the choice of cipher can
> influence which server certificate is sent, this means that the wrong
> OCSP Response may be sent in cases where multiple server certificates
> are configured.
>
> The attached patch against CVS HEAD makes the following changes:
> - Moves the s->ctx->tlsext_status_cb() call to just after the cipher
> has been chosen. This involves splitting ssl_check_clienthello_tlsext()
> into two functions: "early" and "late".
> - Updates SSL_get_certificate() so that it returns the server
> certificate that actually gets sent. (This is the function that Apache
> httpd's OCSP Stapling code calls in order to determine which OCSP
> Response to send).
>
> I've tested this patch successfully with an installation of httpd 2.4.2
> that has both an RSA cert and an ECC cert configured.
>
> If this patch is OK, I'd like to backport it to the OpenSSL 1.0.x branch
> as well.
>
--
Rob Stradling
Senior Research & Development Scientist
COMODO - Creating Trust Online
Index: ssl/s3_srvr.c
===
RCS file: /v/openssl/cvs/openssl/ssl/s3_srvr.c,v
retrieving revision 1.239
diff -u -r1.239 s3_srvr.c
--- ssl/s3_srvr.c 15 Aug 2012 15:15:05 - 1.239
+++ ssl/s3_srvr.c 7 Sep 2012 10:00:12 -
@@ -1432,6 +1432,16 @@
* s->tmp.new_cipher- the new cipher to use.
*/
+ /* Handles TLS extensions that we couldn't check earlier */
+ if (s->version >= SSL3_VERSION)
+ {
+ if (ssl_check_clienthello_tlsext_late(s) <= 0)
+ {
+
SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,SSL_R_CLIENTHELLO_TLSEXT);
+ goto err;
+ }
+ }
+
if (ret < 0) ret=1;
if (0)
{
Index: ssl/ssl_lib.c
===
RCS file: /v/openssl/cvs/openssl/ssl/ssl_lib.c,v
retrieving revision 1.242
diff -u -r1.242 ssl_lib.c
--- ssl/ssl_lib.c 31 Aug 2012 11:18:54 - 1.242
+++ ssl/ssl_lib.c 7 Sep 2012 10:00:12 -
@@ -2336,7 +2336,7 @@
#endif
-static int ssl_get_server_cert_index(SSL *s)
+static int ssl_get_server_cert_index(const SSL *s)
{
int idx;
idx = ssl_cipher_get_cert_index(s->s3->tmp.new_cipher);
@@ -2347,7 +2347,7 @@
return idx;
}
-CERT_PKEY *ssl_get_server_send_pkey(SSL *s)
+CERT_PKEY *ssl_get_server_send_pkey(const SSL *s)
{
CERT *c;
int i;
@@ -2833,6 +2833,14 @@
/* Fix this function so that it takes an optional type parameter */
X509 *SSL_get_certificate(const SSL *s)
{
+ if (s->server)
+ {
+ CERT_PKEY *certpkey;
+ certpkey = ssl_get_server_send_pkey(s);
+ if (certpkey && certpkey->x509)
+ return certpkey->x509;
+ }
+
if (s->cert != NULL)
return(s->cert->key->x509);
else
Index: ssl/ssl_locl.h
===
RCS file: /v/openssl/cvs/openssl/ssl/ssl_locl.h,v
retrieving revision 1.155
diff -u -r1.155 ssl_locl.h
--- ssl/ssl_locl.h 31 Aug 2012 11:18:54 - 1.155
+++ ssl/ssl_locl.h 7 Sep 2012 10:00:12 -
@@ -934,7 +934,7 @@
int ssl_undefined_function(SSL *s);
int ssl_undefined_void_function(void);
int ssl_undefined_const_function(const SSL *s);
-CERT_PKEY *ssl_get_server_send_pkey(SSL *);
+CERT_PKEY *ssl_get_server_send_pkey(const SSL *s);
unsigned char *ssl_get_authz_data(SSL *s, size_t *authz_length);
EVP_PKEY *ssl_get_sign_pkey(SSL *s,const SSL_CIPHER *c, const EVP_MD **pmd);
int ssl_cert_type(X509 *x,EVP_PKEY *pkey);
@@ -1201,6 +1201,7 @@
unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned
char *limit);
unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned
char *limit);
int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **data, unsigned char
*d, int n);
+int ssl_check_clienthello_tlsext_late(SSL *s);
int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **data, unsigned char
*d, int n);
int ssl_prepare_clienthello_tlsext(SSL *s);
int ssl_prepare_serverhello_tlsext(SSL *s);
Index: ssl/t1_lib.c
===
RCS file: /v/openssl/cvs/openssl/ssl/t1_lib.c,v
retrieving revision 1.139
diff -u -r1.139 t1_lib.c
--- ssl/t1_lib.c29 Aug 2012 13:18:34 - 1.139
+++ ssl/t1_lib.c7 Sep 2012 10:0
[openssl.org #2836] [PATCH] Staple the correct OCSP Response when multiple certs are configured
The OCSP Stapling Callback function (s->ctx->tlsext_status_cb) is called
during the parsing of the ClientHello message, before the server has
decided which cipher to use. However, since the choice of cipher can
influence which server certificate is sent, this means that the wrong
OCSP Response may be sent in cases where multiple server certificates
are configured.
The attached patch against CVS HEAD makes the following changes:
- Moves the s->ctx->tlsext_status_cb() call to just after the cipher
has been chosen. This involves splitting ssl_check_clienthello_tlsext()
into two functions: "early" and "late".
- Updates SSL_get_certificate() so that it returns the server
certificate that actually gets sent. (This is the function that Apache
httpd's OCSP Stapling code calls in order to determine which OCSP
Response to send).
I've tested this patch successfully with an installation of httpd 2.4.2
that has both an RSA cert and an ECC cert configured.
If this patch is OK, I'd like to backport it to the OpenSSL 1.0.x branch
as well.
--
Rob Stradling
Senior Research & Development Scientist
COMODO - Creating Trust Online
Index: ssl/s3_srvr.c
===
RCS file: /v/openssl/cvs/openssl/ssl/s3_srvr.c,v
retrieving revision 1.233
diff -u -r1.233 s3_srvr.c
--- ssl/s3_srvr.c 6 Jun 2012 12:52:19 - 1.233
+++ ssl/s3_srvr.c 19 Jun 2012 10:59:34 -
@@ -1424,6 +1424,16 @@
* s->tmp.new_cipher- the new cipher to use.
*/
+ /* Handles TLS extensions that we couldn't check earlier */
+ if (s->version >= SSL3_VERSION)
+ {
+ if (!ssl_check_clienthello_tlsext_late(s))
+ {
+ SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,SSL_R_PARSE_TLSEXT);
+ goto err;
+ }
+ }
+
if (ret < 0) ret=1;
if (0)
{
Index: ssl/ssl_lib.c
===
RCS file: /v/openssl/cvs/openssl/ssl/ssl_lib.c,v
retrieving revision 1.234
diff -u -r1.234 ssl_lib.c
--- ssl/ssl_lib.c 18 Jun 2012 12:56:59 - 1.234
+++ ssl/ssl_lib.c 19 Jun 2012 10:59:34 -
@@ -2846,6 +2846,14 @@
/* Fix this function so that it takes an optional type parameter */
X509 *SSL_get_certificate(const SSL *s)
{
+ if (s->server)
+ {
+ CERT_PKEY *certpkey;
+ certpkey = ssl_get_server_send_pkey((SSL *)s);
+ if (certpkey && certpkey->x509)
+ return certpkey->x509;
+ }
+
if (s->cert != NULL)
return(s->cert->key->x509);
else
Index: ssl/ssl_locl.h
===
RCS file: /v/openssl/cvs/openssl/ssl/ssl_locl.h,v
retrieving revision 1.141
diff -u -r1.141 ssl_locl.h
--- ssl/ssl_locl.h 18 Jun 2012 12:56:59 - 1.141
+++ ssl/ssl_locl.h 19 Jun 2012 10:59:34 -
@@ -1132,6 +1132,7 @@
unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned
char *limit);
unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned
char *limit);
int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **data, unsigned char
*d, int n);
+int ssl_check_clienthello_tlsext_late(SSL *s);
int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **data, unsigned char
*d, int n);
int ssl_prepare_clienthello_tlsext(SSL *s);
int ssl_prepare_serverhello_tlsext(SSL *s);
Index: ssl/t1_lib.c
===
RCS file: /v/openssl/cvs/openssl/ssl/t1_lib.c,v
retrieving revision 1.123
diff -u -r1.123 t1_lib.c
--- ssl/t1_lib.c11 Jun 2012 09:23:55 - 1.123
+++ ssl/t1_lib.c19 Jun 2012 10:59:34 -
@@ -123,7 +123,7 @@
static int tls_decrypt_ticket(SSL *s, const unsigned char *tick, int ticklen,
const unsigned char *sess_id, int sesslen,
SSL_SESSION **psess);
-static int ssl_check_clienthello_tlsext(SSL *s);
+static int ssl_check_clienthello_tlsext_early(SSL *s);
int ssl_check_serverhello_tlsext(SSL *s);
#endif
@@ -1846,7 +1846,7 @@
return 0;
}
- if (ssl_check_clienthello_tlsext(s) <= 0)
+ if (ssl_check_clienthello_tlsext_early(s) <= 0)
{
SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_TLSEXT,SSL_R_CLIENTHELLO_TLSEXT);
return 0;
@@ -2247,7 +2247,7 @@
return 1;
}
-static int ssl_check_clienthello_tlsext(SSL *s)
+static int ssl_check_clienthello_tlsext_early(SSL *s)
{
int ret=SSL_TLSEXT_ERR_NOACK;
int al = SSL_AD_UNRECOGNIZED_NAME;
@@ -2266,42 +2266,11 @@
else if (s->initial_ctx != NULL &&
s->initial_ctx->tlsext_servername_callback != 0)
ret =
