Skip to content

Commit

Permalink
Support setting per-connection OCSP staple
Browse files Browse the repository at this point in the history
Right now the only way to set an OCSP response is SSL_CTX_set_ocsp_response
however this assumes that all the SSLs generated from a SSL_CTX share the
same OCSP response, which is wrong.

This is similar to the OpenSSL "function" SSL_get_tlsext_status_ocsp_resp,
the main difference being that this doesn't take ownership of the OCSP buffer.

In order to avoid memory duplication in case SSL_CTX has its own response,
a CRYPTO_BUFFER is used for both SSL_CTX and SSL.

Change-Id: I3a0697f82b805ac42a22be9b6bb596aa0b530025
Reviewed-on: https://boringssl-review.googlesource.com/12660
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
  • Loading branch information
ghedo authored and CQ bot account: commit-bot@chromium.org committed Dec 8, 2016
1 parent 7c57286 commit 559f064
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 21 deletions.
13 changes: 11 additions & 2 deletions include/openssl/ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,13 @@ OPENSSL_EXPORT int SSL_CTX_set_ocsp_response(SSL_CTX *ctx,
const uint8_t *response,
size_t response_len);

/* SSL_set_ocsp_response sets the OCSP reponse that is sent to clients which
* request it. It returns one on success and zero on error. The caller retains
* ownership of |response|. */
OPENSSL_EXPORT int SSL_set_ocsp_response(SSL *ssl,
const uint8_t *response,
size_t response_len);

/* SSL_SIGN_* are signature algorithm values as defined in TLS 1.3. */
#define SSL_SIGN_RSA_PKCS1_SHA1 0x0201
#define SSL_SIGN_RSA_PKCS1_SHA256 0x0401
Expand Down Expand Up @@ -4009,8 +4016,7 @@ struct ssl_ctx_st {
size_t signed_cert_timestamp_list_length;

/* OCSP response to be sent to the client, if requested. */
uint8_t *ocsp_response;
size_t ocsp_response_length;
CRYPTO_BUFFER *ocsp_response;

/* keylog_callback, if not NULL, is the key logging callback. See
* |SSL_CTX_set_keylog_callback|. */
Expand Down Expand Up @@ -4224,6 +4230,9 @@ struct ssl_st {
/* session_timeout is the default lifetime in seconds of the session
* created in this connection. */
long session_timeout;

/* OCSP response to be sent to the client, if requested. */
CRYPTO_BUFFER *ocsp_response;
};


Expand Down
4 changes: 2 additions & 2 deletions ssl/handshake_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -1143,8 +1143,8 @@ static int ssl3_send_certificate_status(SSL_HANDSHAKE *hs) {
SSL3_MT_CERTIFICATE_STATUS) ||
!CBB_add_u8(&body, TLSEXT_STATUSTYPE_ocsp) ||
!CBB_add_u24_length_prefixed(&body, &ocsp_response) ||
!CBB_add_bytes(&ocsp_response, ssl->ctx->ocsp_response,
ssl->ctx->ocsp_response_length) ||
!CBB_add_bytes(&ocsp_response, CRYPTO_BUFFER_data(ssl->ocsp_response),
CRYPTO_BUFFER_len(ssl->ocsp_response)) ||
!ssl_complete_message(ssl, &cbb)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
CBB_cleanup(&cbb);
Expand Down
27 changes: 17 additions & 10 deletions ssl/ssl_lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ void SSL_CTX_free(SSL_CTX *ctx) {
OPENSSL_free(ctx->psk_identity_hint);
OPENSSL_free(ctx->supported_group_list);
OPENSSL_free(ctx->alpn_client_proto_list);
OPENSSL_free(ctx->ocsp_response);
CRYPTO_BUFFER_free(ctx->ocsp_response);
OPENSSL_free(ctx->signed_cert_timestamp_list);
EVP_PKEY_free(ctx->tlsext_channel_id_private);

Expand Down Expand Up @@ -484,6 +484,12 @@ SSL *SSL_new(SSL_CTX *ctx) {
ssl->session_timeout = ctx->session_timeout;
}

/* If the context has an OCSP response, use it. */
if (ctx->ocsp_response != NULL) {
CRYPTO_BUFFER_up_ref(ctx->ocsp_response);
ssl->ocsp_response = ctx->ocsp_response;
}

return ssl;

err:
Expand Down Expand Up @@ -525,6 +531,7 @@ void SSL_free(SSL *ssl) {
OPENSSL_free(ssl->psk_identity_hint);
sk_X509_NAME_pop_free(ssl->client_CA, X509_NAME_free);
sk_SRTP_PROTECTION_PROFILE_free(ssl->srtp_profiles);
CRYPTO_BUFFER_free(ssl->ocsp_response);

if (ssl->method != NULL) {
ssl->method->ssl_free(ssl);
Expand Down Expand Up @@ -1791,16 +1798,16 @@ int SSL_CTX_set_signed_cert_timestamp_list(SSL_CTX *ctx, const uint8_t *list,

int SSL_CTX_set_ocsp_response(SSL_CTX *ctx, const uint8_t *response,
size_t response_len) {
OPENSSL_free(ctx->ocsp_response);
ctx->ocsp_response_length = 0;

ctx->ocsp_response = BUF_memdup(response, response_len);
if (ctx->ocsp_response == NULL) {
return 0;
}
ctx->ocsp_response_length = response_len;
CRYPTO_BUFFER_free(ctx->ocsp_response);
ctx->ocsp_response = CRYPTO_BUFFER_new(response, response_len, NULL);
return ctx->ocsp_response != NULL;
}

return 1;
int SSL_set_ocsp_response(SSL *ssl, const uint8_t *response,
size_t response_len) {
CRYPTO_BUFFER_free(ssl->ocsp_response);
ssl->ocsp_response = CRYPTO_BUFFER_new(response, response_len, NULL);
return ssl->ocsp_response != NULL;
}

int SSL_set_tlsext_host_name(SSL *ssl, const char *name) {
Expand Down
2 changes: 1 addition & 1 deletion ssl/t1_lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -1212,7 +1212,7 @@ static int ext_ocsp_add_serverhello(SSL_HANDSHAKE *hs, CBB *out) {
SSL *const ssl = hs->ssl;
if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION ||
!hs->ocsp_stapling_requested ||
ssl->ctx->ocsp_response_length == 0 ||
ssl->ocsp_response == NULL ||
ssl->s3->session_reused ||
!ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
return 1;
Expand Down
5 changes: 2 additions & 3 deletions ssl/test/bssl_shim.cc
Original file line number Diff line number Diff line change
Expand Up @@ -398,9 +398,8 @@ static bool GetCertificate(SSL *ssl, bssl::UniquePtr<X509> *out_x509,
return false;
}
if (!config->ocsp_response.empty() &&
!SSL_CTX_set_ocsp_response(SSL_get_SSL_CTX(ssl),
(const uint8_t *)config->ocsp_response.data(),
config->ocsp_response.size())) {
!SSL_set_ocsp_response(ssl, (const uint8_t *)config->ocsp_response.data(),
config->ocsp_response.size())) {
return false;
}
return true;
Expand Down
6 changes: 3 additions & 3 deletions ssl/tls13_both.c
Original file line number Diff line number Diff line change
Expand Up @@ -466,14 +466,14 @@ int tls13_prepare_certificate(SSL_HANDSHAKE *hs) {
}

if (hs->ocsp_stapling_requested &&
ssl->ctx->ocsp_response_length != 0) {
ssl->ocsp_response != NULL) {
CBB contents, ocsp_response;
if (!CBB_add_u16(&extensions, TLSEXT_TYPE_status_request) ||
!CBB_add_u16_length_prefixed(&extensions, &contents) ||
!CBB_add_u8(&contents, TLSEXT_STATUSTYPE_ocsp) ||
!CBB_add_u24_length_prefixed(&contents, &ocsp_response) ||
!CBB_add_bytes(&ocsp_response, ssl->ctx->ocsp_response,
ssl->ctx->ocsp_response_length) ||
!CBB_add_bytes(&ocsp_response, CRYPTO_BUFFER_data(ssl->ocsp_response),
CRYPTO_BUFFER_len(ssl->ocsp_response)) ||
!CBB_flush(&extensions)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
goto err;
Expand Down

0 comments on commit 559f064

Please sign in to comment.