# HG changeset patch
# User Alessandro Ghedini <alessandro@cloudflare.com>
# Date 1453481195 0
# Fri Jan 22 16:46:35 2016 +0000
# Node ID a8c4f65236ad90138863d5295ca059a3d37da37e
# Parent 02abce4764b7c964e7ee6e10b1a7abf1fee95f79
OCSP stapling: refactor OCSP response verification

Move the OCSP response verification and the certificate issuer code to
stand-alone functions, so that it can be reused by other code.

Signed-off-by: Alessandro Ghedini <alessandro@cloudflare.com>

diff -r 02abce4764b7 -r a8c4f65236ad src/event/ngx_event_openssl_stapling.c
--- a/src/event/ngx_event_openssl_stapling.c Tue Jan 12 19:19:07 2016 +0300
+++ b/src/event/ngx_event_openssl_stapling.c Fri Jan 22 16:46:35 2016 +0000
@@ -85,10 +85,15 @@

static ngx_int_t ngx_ssl_stapling_file(ngx_conf_t *cf, ngx_ssl_t *ssl,
ngx_str_t *file);
+static ngx_int_t ngx_ssl_stapling_find_issuer(ngx_ssl_stapling_t *staple,
+ ngx_log_t *log, SSL_CTX *ssl_ctx, X509 *cert, STACK_OF(X509) *chain);
static ngx_int_t ngx_ssl_stapling_issuer(ngx_conf_t *cf, ngx_ssl_t *ssl);
static ngx_int_t ngx_ssl_stapling_responder(ngx_conf_t *cf, ngx_ssl_t *ssl,
ngx_str_t *responder);

+static ngx_int_t ngx_ssl_stapling_verify_response(ngx_log_t *log,
+ ngx_str_t *resp, ngx_ssl_stapling_t *staple, STACK_OF(X509) *chain,
+ ngx_int_t flags, time_t *valid);
static int ngx_ssl_certificate_status_callback(ngx_ssl_conn_t *ssl_conn,
void *data);
static void ngx_ssl_stapling_update(ngx_ssl_stapling_t *staple);
@@ -259,12 +264,87 @@


static ngx_int_t
+ngx_ssl_stapling_find_issuer(ngx_ssl_stapling_t *staple, ngx_log_t *log,
+ SSL_CTX *ssl_ctx, X509 *cert, STACK_OF(X509) *chain)
+{
+ int i, n, rc;
+ X509 *issuer;
+ X509_STORE *store;
+ X509_STORE_CTX *store_ctx;
+
+ n = sk_X509_num(chain);
+
+ ngx_log_debug1(NGX_LOG_DEBUG_EVENT, log, 0,
+ "SSL get issuer: %d extra certs", n);
+
+ for (i = 0; i < n; i++) {
+ issuer = sk_X509_value(chain, i);
+ if (X509_check_issued(issuer, cert) == X509_V_OK) {
+ CRYPTO_add(&issuer->references, 1, CRYPTO_LOCK_X509);
+
+ ngx_log_debug1(NGX_LOG_DEBUG_EVENT, log, 0,
+ "SSL get issuer: found %p in extra certs", issuer);
+
+ staple->cert = cert;
+ staple->issuer = issuer;
+
+ return NGX_OK;
+ }
+ }
+
+ store = SSL_CTX_get_cert_store(ssl_ctx);
+ if (store == NULL) {
+ ngx_ssl_error(NGX_LOG_EMERG, log, 0,
+ "SSL_CTX_get_cert_store() failed");
+ return NGX_ERROR;
+ }
+
+ store_ctx = X509_STORE_CTX_new();
+ if (store_ctx == NULL) {
+ ngx_ssl_error(NGX_LOG_EMERG, log, 0,
+ "X509_STORE_CTX_new() failed");
+ return NGX_ERROR;
+ }
+
+ if (X509_STORE_CTX_init(store_ctx, store, NULL, NULL) == 0) {
+ ngx_ssl_error(NGX_LOG_EMERG, log, 0,
+ "X509_STORE_CTX_init() failed");
+ X509_STORE_CTX_free(store_ctx);
+ return NGX_ERROR;
+ }
+
+ rc = X509_STORE_CTX_get1_issuer(&issuer, store_ctx, cert);
+
+ if (rc == -1) {
+ ngx_ssl_error(NGX_LOG_EMERG, log, 0,
+ "X509_STORE_CTX_get1_issuer() failed");
+ X509_STORE_CTX_free(store_ctx);
+ return NGX_ERROR;
+ }
+
+ if (rc == 0) {
+ ngx_log_error(NGX_LOG_WARN, log, 0,
+ "\"ssl_stapling\" ignored, issuer certificate not found");
+ X509_STORE_CTX_free(store_ctx);
+ return NGX_DECLINED;
+ }
+
+ X509_STORE_CTX_free(store_ctx);
+
+ ngx_log_debug1(NGX_LOG_DEBUG_EVENT, log, 0,
+ "SSL get issuer: found %p in cert store", issuer);
+
+ staple->cert = cert;
+ staple->issuer = issuer;
+
+ return NGX_OK;
+}
+
+
+static ngx_int_t
ngx_ssl_stapling_issuer(ngx_conf_t *cf, ngx_ssl_t *ssl)
{
- int i, n, rc;
- X509 *cert, *issuer;
- X509_STORE *store;
- X509_STORE_CTX *store_ctx;
+ X509 *cert;
STACK_OF(X509) *chain;
ngx_ssl_stapling_t *staple;

@@ -277,72 +357,8 @@
chain = ssl->ctx->extra_certs;
#endif

- n = sk_X509_num(chain);
-
- ngx_log_debug1(NGX_LOG_DEBUG_EVENT, ssl->log, 0,
- "SSL get issuer: %d extra certs", n);
-
- for (i = 0; i < n; i++) {
- issuer = sk_X509_value(chain, i);
- if (X509_check_issued(issuer, cert) == X509_V_OK) {
- CRYPTO_add(&issuer->references, 1, CRYPTO_LOCK_X509);
-
- ngx_log_debug1(NGX_LOG_DEBUG_EVENT, ssl->log, 0,
- "SSL get issuer: found %p in extra certs", issuer);
-
- staple->cert = cert;
- staple->issuer = issuer;
-
- return NGX_OK;
- }
- }
-
- store = SSL_CTX_get_cert_store(ssl->ctx);
- if (store == NULL) {
- ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
- "SSL_CTX_get_cert_store() failed");
- return NGX_ERROR;
- }
-
- store_ctx = X509_STORE_CTX_new();
- if (store_ctx == NULL) {
- ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
- "X509_STORE_CTX_new() failed");
- return NGX_ERROR;
- }
-
- if (X509_STORE_CTX_init(store_ctx, store, NULL, NULL) == 0) {
- ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
- "X509_STORE_CTX_init() failed");
- X509_STORE_CTX_free(store_ctx);
- return NGX_ERROR;
- }
-
- rc = X509_STORE_CTX_get1_issuer(&issuer, store_ctx, cert);
-
- if (rc == -1) {
- ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
- "X509_STORE_CTX_get1_issuer() failed");
- X509_STORE_CTX_free(store_ctx);
- return NGX_ERROR;
- }
-
- if (rc == 0) {
- ngx_log_error(NGX_LOG_WARN, ssl->log, 0,
- "\"ssl_stapling\" ignored, issuer certificate not found");
- X509_STORE_CTX_free(store_ctx);
- return NGX_DECLINED;
- }
-
- X509_STORE_CTX_free(store_ctx);
-
- ngx_log_debug1(NGX_LOG_DEBUG_EVENT, ssl->log, 0,
- "SSL get issuer: found %p in cert store", issuer);
-
- staple->cert = cert;
- staple->issuer = issuer;
-
- return NGX_OK;
+ return ngx_ssl_stapling_find_issuer(staple, ssl->log, ssl->ctx,
+ cert, chain);
}


@@ -529,8 +545,10 @@
}


-static void
-ngx_ssl_stapling_ocsp_handler(ngx_ssl_ocsp_ctx_t *ctx)
+static ngx_int_t
+ngx_ssl_stapling_verify_response(ngx_log_t *log, ngx_str_t *resp,
+ ngx_ssl_stapling_t *staple, STACK_OF(X509) *chain, ngx_int_t flags,
+ time_t *valid)
{
#if OPENSSL_VERSION_NUMBER >= 0x0090707fL
const
@@ -538,34 +556,24 @@
u_char *p;
int n;
size_t len;
- time_t now, valid;
- ngx_str_t response;
X509_STORE *store;
- STACK_OF(X509) *chain;
OCSP_CERTID *id;
OCSP_RESPONSE *ocsp;
OCSP_BASICRESP *basic;
- ngx_ssl_stapling_t *staple;
ASN1_GENERALIZEDTIME *thisupdate, *nextupdate;

- staple = ctx->data;
- now = ngx_time();
ocsp = NULL;
basic = NULL;
id = NULL;

- if (ctx->code != 200) {
- goto error;
- }
-
/* check the response */

- len = ctx->response->last - ctx->response->pos;
- p = ctx->response->pos;
+ len = resp->len;
+ p = resp->data;

ocsp = d2i_OCSP_RESPONSE(NULL, &p, len);
if (ocsp == NULL) {
- ngx_ssl_error(NGX_LOG_ERR, ctx->log, 0,
+ ngx_ssl_error(NGX_LOG_ERR, log, 0,
"d2i_OCSP_RESPONSE() failed");
goto error;
}
@@ -573,7 +581,7 @@
n = OCSP_response_status(ocsp);

if (n != OCSP_RESPONSE_STATUS_SUCCESSFUL) {
- ngx_log_error(NGX_LOG_ERR, ctx->log, 0,
+ ngx_log_error(NGX_LOG_ERR, log, 0,
"OCSP response not successful (%d: %s)",
n, OCSP_response_status_str(n));
goto error;
@@ -581,36 +589,27 @@

basic = OCSP_response_get1_basic(ocsp);
if (basic == NULL) {
- ngx_ssl_error(NGX_LOG_ERR, ctx->log, 0,
+ ngx_ssl_error(NGX_LOG_ERR, log, 0,
"OCSP_response_get1_basic() failed");
goto error;
}

store = SSL_CTX_get_cert_store(staple->ssl_ctx);
if (store == NULL) {
- ngx_ssl_error(NGX_LOG_CRIT, ctx->log, 0,
+ ngx_ssl_error(NGX_LOG_CRIT, log, 0,
"SSL_CTX_get_cert_store() failed");
goto error;
}

-#if OPENSSL_VERSION_NUMBER >= 0x10001000L
- SSL_CTX_get_extra_chain_certs(staple->ssl_ctx, &chain);
-#else
- chain = staple->ssl_ctx->extra_certs;
-#endif
-
- if (OCSP_basic_verify(basic, chain, store,
- staple->verify ? OCSP_TRUSTOTHER : OCSP_NOVERIFY)
- != 1)
- {
- ngx_ssl_error(NGX_LOG_ERR, ctx->log, 0,
+ if (OCSP_basic_verify(basic, chain, store, flags) != 1) {
+ ngx_ssl_error(NGX_LOG_ERR, log, 0,
"OCSP_basic_verify() failed");
goto error;
}

- id = OCSP_cert_to_id(NULL, ctx->cert, ctx->issuer);
+ id = OCSP_cert_to_id(NULL, staple->cert, staple->issuer);
if (id == NULL) {
- ngx_ssl_error(NGX_LOG_CRIT, ctx->log, 0,
+ ngx_ssl_error(NGX_LOG_CRIT, log, 0,
"OCSP_cert_to_id() failed");
goto error;
}
@@ -619,34 +618,34 @@
&thisupdate, &nextupdate)
!= 1)
{
- ngx_log_error(NGX_LOG_ERR, ctx->log, 0,
+ ngx_log_error(NGX_LOG_ERR, log, 0,
"certificate status not found in the OCSP response");
goto error;
}

if (n != V_OCSP_CERTSTATUS_GOOD) {
- ngx_log_error(NGX_LOG_ERR, ctx->log, 0,
+ ngx_log_error(NGX_LOG_ERR, log, 0,
"certificate status \"%s\" in the OCSP response",
OCSP_cert_status_str(n));
goto error;
}

if (OCSP_check_validity(thisupdate, nextupdate, 300, -1) != 1) {
- ngx_ssl_error(NGX_LOG_ERR, ctx->log, 0,
+ ngx_ssl_error(NGX_LOG_ERR, log, 0,
"OCSP_check_validity() failed");
goto error;
}

if (nextupdate) {
- valid = ngx_ssl_stapling_time(nextupdate);
- if (valid == (time_t) NGX_ERROR) {
- ngx_log_error(NGX_LOG_ERR, ctx->log, 0,
+ *valid = ngx_ssl_stapling_time(nextupdate);
+ if (*valid == (time_t) NGX_ERROR) {
+ ngx_log_error(NGX_LOG_ERR, log, 0,
"invalid nextUpdate time in certificate status");
goto error;
}

} else {
- valid = NGX_MAX_TIME_T_VALUE;
+ *valid = NGX_MAX_TIME_T_VALUE;
}

OCSP_CERTID_free(id);
@@ -657,9 +656,54 @@
basic = NULL;
ocsp = NULL;

+ return NGX_OK;
+
+error:
+
+ if (id) {
+ OCSP_CERTID_free(id);
+ }
+
+ if (basic) {
+ OCSP_BASICRESP_free(basic);
+ }
+
+ if (ocsp) {
+ OCSP_RESPONSE_free(ocsp);
+ }
+
+ return NGX_ERROR;
+}
+
+
+static void
+ngx_ssl_stapling_ocsp_handler(ngx_ssl_ocsp_ctx_t *ctx)
+{
+ time_t now, valid;
+ ngx_int_t rc, flags;
+ ngx_str_t response;
+ STACK_OF(X509) *chain;
+ ngx_ssl_stapling_t *staple;
+
+ now = ngx_time();
+
+ staple = ctx->data;
+
+ if (ctx->code != 200) {
+ goto error;
+ }
+
+#if OPENSSL_VERSION_NUMBER >= 0x10001000L
+ SSL_CTX_get_extra_chain_certs(staple->ssl_ctx, &chain);
+#else
+ chain = staple->ssl_ctx->extra_certs;
+#endif
+
+ flags = staple->verify ? OCSP_TRUSTOTHER : OCSP_NOVERIFY;
+
/* copy the response to memory not in ctx->pool */

- response.len = len;
+ response.len = ctx->response->last - ctx->response->pos;
response.data = ngx_alloc(response.len, ctx->log);

if (response.data == NULL) {
@@ -668,6 +712,14 @@

ngx_memcpy(response.data, ctx->response->pos, response.len);

+ /* check the response */
+
+ rc = ngx_ssl_stapling_verify_response(ctx->log, &response, staple, chain,
+ flags, &valid);
+ if (rc != NGX_OK) {
+ goto error;
+ }
+
ngx_log_debug2(NGX_LOG_DEBUG_EVENT, ctx->log, 0,
"ssl ocsp response, %s, %uz",
OCSP_cert_status_str(n), response.len);
@@ -695,16 +747,8 @@
staple->loading = 0;
staple->refresh = now + 300;

- if (id) {
- OCSP_CERTID_free(id);
- }
-
- if (basic) {
- OCSP_BASICRESP_free(basic);
- }
-
- if (ocsp) {
- OCSP_RESPONSE_free(ocsp);
+ if (response.data != NULL) {
+ ngx_free(response.data);
}

ngx_ssl_ocsp_done(ctx);

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel