httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r829619 - in /httpd/httpd/trunk: ./ modules/ssl/
Date Sun, 25 Oct 2009 23:44:17 GMT


On 10/25/2009 06:21 PM, jorton@apache.org wrote:
> Author: jorton
> Date: Sun Oct 25 17:21:10 2009
> New Revision: 829619
> 
> URL: http://svn.apache.org/viewvc?rev=829619&view=rev
> Log:
> Add support for OCSP "stapling":
> 
> * modules/ssl/ssl_util_stapling.c: New file.
> 
> * modules/ssl/config.m4, modules/ssl/mod_ssl.dsp: Build it.
> 
> * modules/ssl/ssl_toolkit_compat.h: Define HAVE_OCSP_STAPLING if
>   OpenSSL is of suitable version (>= 0.9.8g) and capability (TLS
>   extension support enabled).
> 
> * modules/ssl/mod_ssl.c: Add config directives.
> 
> * modules/ssl/ssl_private.h: Add prototypes for new functions.
>   (SSLModConfigRec): Add fields for stapling socache instance and
>   associated mutex.
>   (modssl_ctx_t): Add config fields for stapling.
> 
> * modules/ssl/ssl_engine_init.c (ssl_init_Module, ssl_init_Child):
>   Call the stapling initialization functions.
> 
> * modules/ssl/ssl_engine_config.c: Add config hooks.
> 
> * modules/ssl/ssl_scache.c: Create, initialize and destroy the socache
>   instance for OCSP responses.
> 
> Submitted by: Dr Stephen Henson <shenson oss-institute.org>

Will there be documentation patches for the new directives?

> 
> Added:
>     httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/ssl/config.m4
>     httpd/httpd/trunk/modules/ssl/mod_ssl.c
>     httpd/httpd/trunk/modules/ssl/mod_ssl.dsp
>     httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
>     httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
>     httpd/httpd/trunk/modules/ssl/ssl_private.h
>     httpd/httpd/trunk/modules/ssl/ssl_scache.c
>     httpd/httpd/trunk/modules/ssl/ssl_toolkit_compat.h
> 

> Added: httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c?rev=829619&view=auto
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c (added)
> +++ httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c Sun Oct 25 17:21:10 2009

> +/*
> + * OCSP response caching code. The response is preceded by a flag value
> + * which indicates whether the response was invalid when it was stored.
> + * the purpose of this flag is to avoid repeated queries to a server
> + * which has given an invalid response while allowing a response which
> + * has subsequently become invalid to be retried immediately.
> + *
> + * The key for the cache is the hash of the certificate the response
> + * is for.
> + */
> +static BOOL stapling_cache_response(server_rec *s, modssl_ctx_t *mctx,
> +                                    OCSP_RESPONSE *rsp, certinfo *cinf,
> +                                    BOOL ok, apr_pool_t *pool)
> +{
> +    SSLModConfigRec *mc = myModConfig(s);
> +    unsigned char resp_der[MAX_STAPLING_DER];
> +    unsigned char *p;
> +    int resp_derlen;
> +    BOOL rv;
> +    time_t timeout;
> +
> +    resp_derlen = i2d_OCSP_RESPONSE(rsp, NULL) + 1;
> +
> +    if (resp_derlen <= 0) {
> +        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
> +                     "OCSP stapling response encode error??");
> +        return FALSE;
> +    }
> +
> +    if (resp_derlen > sizeof resp_der) {
> +        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
> +                     "OCSP stapling response too big (%u bytes)", resp_derlen);
> +        return FALSE;
> +    }
> +
> +
> +    p = resp_der;
> +
> +    if (ok == TRUE) {
> +        *p++ = 1;
> +        timeout = mctx->stapling_cache_timeout;
> +    } else {
> +        *p++ = 0;
> +        timeout = mctx->stapling_errcache_timeout;
> +    }
> +
> +    timeout += time(NULL);

Shouldn't we use apr_time_now here?

> +
> +    i2d_OCSP_RESPONSE(rsp, &p);

Is this correct? p is already char *. So &p would be char **.
If p gets changed by i2d_OCSP_RESPONSE our flag set above would get lost ???

> +
> +    rv = mc->stapling_cache->store(mc->stapling_cache_context, s,
> +                                   cinf->idx, sizeof(cinf->idx),
> +                                   timeout, resp_der, resp_derlen, pool);
> +    if (rv != APR_SUCCESS) {
> +        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
> +                     "stapling_cache_response: OCSP response session store error!");
> +        return FALSE;
> +    }
> +    
> +    return TRUE;
> +}
> +
> +static BOOL stapling_get_cached_response(server_rec *s, OCSP_RESPONSE **prsp,
> +                                         BOOL *pok, certinfo *cinf,
> +                                         apr_pool_t *pool)
> +{
> +    SSLModConfigRec *mc = myModConfig(s);
> +    apr_status_t rv;
> +    OCSP_RESPONSE *rsp;
> +    unsigned char resp_der[MAX_STAPLING_DER];
> +    const unsigned char *p;
> +    unsigned int resp_derlen = MAX_STAPLING_DER;
> +
> +    rv = mc->stapling_cache->retrieve(mc->stapling_cache_context, s,
> +                                      cinf->idx, sizeof(cinf->idx),
> +                                      resp_der, &resp_derlen, pool);
> +    if (rv != APR_SUCCESS) {
> +        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
> +                     "stapling_get_cached_response: cache miss");
> +        return TRUE;
> +    }
> +    if (resp_derlen <= 1) {
> +        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
> +                     "stapling_get_cached_response: response length invalid??");
> +        return TRUE;
> +    }
> +    p = resp_der;
> +    if (pok) {
> +        if (*p)
> +            *pok = TRUE;
> +        else
> +            *pok = FALSE;
> +    }
> +    p++;
> +    resp_derlen--;
> +    rsp = d2i_OCSP_RESPONSE(NULL, &p, resp_derlen);

Why &p and not p?

> +    if (!rsp) {
> +        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
> +                     "stapling_get_cached_response: response parse error??");
> +        return TRUE;
> +    }
> +    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
> +                 "stapling_get_cached_response: cache hit");
> +
> +    *prsp = rsp;
> +    
> +    return TRUE;
> +}
> +

> +static BOOL stapling_renew_response(server_rec *s, modssl_ctx_t *mctx, SSL *ssl,
> +                                    certinfo *cinf, OCSP_RESPONSE **prsp,
> +                                    apr_pool_t *pool)
> +{
> +    conn_rec *conn      = (conn_rec *)SSL_get_app_data(ssl);
> +    apr_pool_t *vpool;
> +    OCSP_REQUEST *req = NULL;
> +    OCSP_CERTID *id = NULL;
> +    STACK_OF(X509_EXTENSION) *exts;
> +    int i;
> +    BOOL ok = FALSE;
> +    BOOL rv = TRUE;
> +    const char *ocspuri;
> +    apr_uri_t uri;
> +
> +    *prsp = NULL;
> +    /* Build up OCSP query from server certificate info */
> +    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
> +                 "stapling_renew_response: querying responder");
> +
> +    req = OCSP_REQUEST_new();
> +    if (!req)
> +        goto err;
> +    id = OCSP_CERTID_dup(cinf->cid);
> +    if (!id)
> +        goto err;
> +    if (!OCSP_request_add0_id(req, id))
> +        goto err;
> +    id = NULL;
> +    /* Add any extensions to the request */
> +    SSL_get_tlsext_status_exts(ssl, &exts);
> +    for (i = 0; i < sk_X509_EXTENSION_num(exts); i++) {
> +        X509_EXTENSION *ext = sk_X509_EXTENSION_value(exts, i);
> +        if (!OCSP_REQUEST_add_ext(req, ext, -1))
> +            goto err;
> +    }
> +
> +    if (mctx->stapling_force_url)
> +        ocspuri = mctx->stapling_force_url;
> +    else
> +        ocspuri = cinf->uri;
> +    
> +    /* Create a temporary pool to constrain memory use */
> +    apr_pool_create(&vpool, conn->pool);
> +
> +    ok = apr_uri_parse(vpool, ocspuri, &uri);
> +    if (ok != APR_SUCCESS) {
> +        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
> +                     "stapling_renew_response: Error parsing uri %s",
> +                      ocspuri);
> +	rv = FALSE;
> +	goto done;

I guess we are missing some indention here.

> +    } else if (strcmp(uri.scheme, "http")) {
> +        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
> +                     "stapling_renew_response: Unsupported uri %s", ocspuri);
> +	rv = FALSE;
> +	goto done;

I guess we are missing some indention here.

> +    }
> +
> +    *prsp = modssl_dispatch_ocsp_request(&uri, mctx->stapling_responder_timeout,
> +                                         req, conn, vpool);
> + 
> +    apr_pool_destroy(vpool);
> +
> +    if (!*prsp) {
> +        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
> +                     "stapling_renew_response: responder error");
> +        if (mctx->stapling_fake_trylater) { 
> +            *prsp = OCSP_response_create(OCSP_RESPONSE_STATUS_TRYLATER, NULL);
> +        } 
> +        else {
> +            goto done;
> +        }
> +    } else {
> +        int response_status = OCSP_response_status(*prsp);
> +
> +        if (response_status == OCSP_RESPONSE_STATUS_SUCCESSFUL) {
> +            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
> +                        "stapling_renew_response: query response received");
> +            stapling_check_response(s, mctx, cinf, *prsp, &ok);
> +            if (ok == FALSE) {
> +                ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
> +                             "stapling_renew_response: error in retreived response!");
> +            }
> +        } else {
> +            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
> +                         "stapling_renew_response: responder error %s",
> +                         OCSP_response_status_str(response_status));
> +        }
> +    }
> +    if (stapling_cache_response(s, mctx, *prsp, cinf, ok, pool) == FALSE) {
> +        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
> +                     "stapling_renew_response: error caching response!");
> +    }
> +
> +done:
> +    if (id)
> +        OCSP_CERTID_free(id);
> +    if (req)
> +        OCSP_REQUEST_free(req);
> +    return rv;
> +err:
> +    rv = FALSE;
> +    goto done;

Looks a little bit like spaghetti code :-).

> +}
> +

> +/* Certificate Status callback. This is called when a client includes a
> + * certificate status request extension.
> + *
> + * Check for cached responses in session cache. If valid send back to
> + * client.  If absent or no longer valid query responder and update
> + * cache. */
> +static int stapling_cb(SSL *ssl, void *arg)
> +{
> +    conn_rec *conn      = (conn_rec *)SSL_get_app_data(ssl);
> +    server_rec *s       = conn->base_server;

Shouldn't we use mySrvFromConn(conn) here instead of conn->base_server?

> +
> +    SSLSrvConfigRec *sc = mySrvConfig(s);
> +    SSLConnRec *sslconn = myConnConfig(conn);
> +    modssl_ctx_t *mctx  = myCtxConfig(sslconn, sc);
> +    certinfo *cinf = NULL;
> +    OCSP_RESPONSE *rsp = NULL;
> +    int rv;
> +    BOOL ok;
> +
> +    if (sc->server->stapling_enabled != TRUE) {
> +        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
> +                     "stapling_cb: OCSP Stapling disabled");
> +        return SSL_TLSEXT_ERR_NOACK;
> +    }
> +
> +    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
> +                 "stapling_cb: OCSP Stapling callback called");
> +
> +    cinf = stapling_get_cert_info(s, mctx, ssl);
> +    if (cinf == NULL) {
> +        return SSL_TLSEXT_ERR_NOACK;
> +    }
> +
> +    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
> +                 "stapling_cb: retrieved cached certificate data");
> +
> +    /* Check to see if we already have a response for this certificate */
> +    stapling_mutex_on(s);
> +
> +    rv = stapling_get_cached_response(s, &rsp, &ok, cinf, conn->pool);
> +    if (rv == FALSE) {
> +        stapling_mutex_off(s);
> +        return SSL_TLSEXT_ERR_ALERT_FATAL;
> +    }
> +
> +    if (rsp) {
> +        /* see if response is acceptable */
> +        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
> +                     "stapling_cb: retrieved cached response");
> +        rv = stapling_check_response(s, mctx, cinf, rsp, NULL);
> +        if (rv == SSL_TLSEXT_ERR_ALERT_FATAL) {
> +            OCSP_RESPONSE_free(rsp);
> +            stapling_mutex_off(s);
> +            return SSL_TLSEXT_ERR_ALERT_FATAL;
> +        } 
> +        else if (rv == SSL_TLSEXT_ERR_NOACK) {
> +            /* Error in response. If this error was not present when it was
> +             * stored (i.e. response no longer valid) then it can be
> +             * renewed straight away.
> +             *
> +             * If the error *was* present at the time it was stored then we
> +             * don't renew the response straight away we just wait for the
> +             * cached response to expire.
> +             */
> +            if (ok) {
> +                OCSP_RESPONSE_free(rsp);
> +                rsp = NULL;
> +            }
> +            else if (!mctx->stapling_return_errors) {
> +                OCSP_RESPONSE_free(rsp);
> +                stapling_mutex_off(s);
> +                return SSL_TLSEXT_ERR_NOACK;
> +            }
> +        }
> +    }
> +
> +    if (rsp == NULL) {
> +        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
> +                     "stapling_cb: renewing cached response");
> +        rv = stapling_renew_response(s, mctx, ssl, cinf, &rsp, conn->pool);
> +
> +        if (rv == FALSE) {
> +            stapling_mutex_off(s);
> +            ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
> +                         "stapling_cb: fatal error");
> +            return SSL_TLSEXT_ERR_ALERT_FATAL;
> +        }
> +    }
> +    stapling_mutex_off(s);
> +
> +    if (rsp) {
> +        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
> +                     "stapling_cb: setting response");
> +        if (!stapling_set_response(ssl, rsp))
> +            return SSL_TLSEXT_ERR_ALERT_FATAL;
> +        return SSL_TLSEXT_ERR_OK;
> +    }
> +    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
> +                 "stapling_cb: no response available");
> +
> +    return SSL_TLSEXT_ERR_NOACK;
> +
> +}
> +


Regards

RĂ¼diger

Mime
View raw message