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 -0000 1.233
+++ ssl/s3_srvr.c 19 Jun 2012 10:59:34 -0000
@@ -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 -0000 1.234
+++ ssl/ssl_lib.c 19 Jun 2012 10:59:34 -0000
@@ -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 -0000 1.141
+++ ssl/ssl_locl.h 19 Jun 2012 10:59:34 -0000
@@ -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.c 11 Jun 2012 09:23:55 -0000 1.123
+++ ssl/t1_lib.c 19 Jun 2012 10:59:34 -0000
@@ -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 = 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:
- s->tlsext_status_expected = 0;
- break;
- /* status request response should be sent */
- case SSL_TLSEXT_ERR_OK:
- if (s->tlsext_ocsp_resp)
- s->tlsext_status_expected = 1;
- else
- s->tlsext_status_expected = 0;
- break;
- /* something bad happened */
- case SSL_TLSEXT_ERR_ALERT_FATAL:
- ret = SSL_TLSEXT_ERR_ALERT_FATAL;
- al = SSL_AD_INTERNAL_ERROR;
- goto err;
- }
- }
- else
- s->tlsext_status_expected = 0;
-
#ifdef TLSEXT_TYPE_opaque_prf_input
{
/* This sort of belongs into ssl_prepare_serverhello_tlsext(),
* but we might be sending an alert in response to the client
hello,
- * so this has to happen here in
ssl_check_clienthello_tlsext(). */
+ * so this has to happen here in
ssl_check_clienthello_tlsext_early(). */
int r = 1;
@@ -2353,8 +2322,8 @@
}
}
-#endif
err:
+#endif
switch (ret)
{
case SSL_TLSEXT_ERR_ALERT_FATAL:
@@ -2372,6 +2341,59 @@
}
}
+int ssl_check_clienthello_tlsext_late(SSL *s)
+ {
+ int ret=SSL_TLSEXT_ERR_OK;
+ int al;
+
+ /* If status request then ask callback what to do.
+ * Note: this must be called after servername callbacks in case
+ * the certificate has changed, and must be called after the cipher
+ * has been chosen because this may influence which certificate is sent
+ */
+ 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:
+ s->tlsext_status_expected = 0;
+ break;
+ /* status request response should be sent */
+ case SSL_TLSEXT_ERR_OK:
+ if (s->tlsext_ocsp_resp)
+ s->tlsext_status_expected = 1;
+ else
+ s->tlsext_status_expected = 0;
+ break;
+ /* something bad happened */
+ case SSL_TLSEXT_ERR_ALERT_FATAL:
+ ret = SSL_TLSEXT_ERR_ALERT_FATAL;
+ al = SSL_AD_INTERNAL_ERROR;
+ goto err;
+ }
+ }
+ else
+ s->tlsext_status_expected = 0;
+
+ err:
+ switch (ret)
+ {
+ case SSL_TLSEXT_ERR_ALERT_FATAL:
+ ssl3_send_alert(s,SSL3_AL_FATAL,al);
+ return -1;
+
+ case SSL_TLSEXT_ERR_ALERT_WARNING:
+ ssl3_send_alert(s,SSL3_AL_WARNING,al);
+ return 1;
+
+ default:
+ return 1;
+ }
+ }
+
int ssl_check_serverhello_tlsext(SSL *s)
{
int ret=SSL_TLSEXT_ERR_NOACK;
