Commit d5bf579e authored by Joe Orton's avatar Joe Orton
Browse files

Merge r1841218 from trunk:

* modules/ssl/ssl_engine_kernel.c (ssl_check_post_client_verify):
  Retrieve and set sslconn->client_cert here for both "modern" and
  classic access control.
  (ssl_hook_Access_classic, ssl_hook_Access_modern, ssl_hook_Access):
  Restore SSLRequire and FakeBasicAuth checks to ssl_hook_Access so tests
  are still applied for TLSv1.3.



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/tlsv1.3-for-2.4.x@1841219 13f79535-47bb-0310-9956-ffa450edef68
parent bbedd8b8
Loading
Loading
Loading
Loading
+103 −95
Original line number Diff line number Diff line
@@ -431,8 +431,22 @@ static void ssl_configure_env(request_rec *r, SSLConnRec *sslconn)
}

static int ssl_check_post_client_verify(request_rec *r, SSLSrvConfigRec *sc, 
                                        SSLDirConfigRec *dc, SSL *ssl)
                                        SSLDirConfigRec *dc, SSLConnRec *sslconn,
                                        SSL *ssl)
{
    X509 *cert;
    
    /*
     * Remember the peer certificate's DN
     */
    if ((cert = SSL_get_peer_certificate(ssl))) {
        if (sslconn->client_cert) {
            X509_free(sslconn->client_cert);
        }
        sslconn->client_cert = cert;
        sslconn->client_dn = NULL;
    }
    
    /*
     * Finally check for acceptable renegotiation results
     */
@@ -450,17 +464,13 @@ static int ssl_check_post_client_verify(request_rec *r, SSLSrvConfigRec *sc,
        }

        if (do_verify) {
            X509 *peercert;

            if ((peercert = SSL_get_peer_certificate(ssl)) == NULL) {
            if (cert == NULL) {
                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02263)
                              "Re-negotiation handshake failed: "
                              "Client certificate missing");

                return HTTP_FORBIDDEN;
            }

            X509_free(peercert);
        }
    }
    return OK;
@@ -476,17 +486,13 @@ static int ssl_hook_Access_classic(request_rec *r, SSLSrvConfigRec *sc, SSLDirCo
    server_rec *handshakeserver = sslconn ? sslconn->server : NULL;
    SSLSrvConfigRec *hssc       = handshakeserver? mySrvConfig(handshakeserver) : NULL;
    SSL_CTX *ctx = NULL;
    apr_array_header_t *requires;
    ssl_require_t *ssl_requires;
    int ok, i, rc;
    BOOL renegotiate = FALSE, renegotiate_quick = FALSE;
    X509 *cert;
    X509 *peercert;
    X509_STORE *cert_store = NULL;
    X509_STORE_CTX *cert_store_ctx;
    STACK_OF(SSL_CIPHER) *cipher_list_old = NULL, *cipher_list = NULL;
    const SSL_CIPHER *cipher = NULL;
    int depth, verify_old, verify, n;
    int depth, verify_old, verify, n, rc;
    const char *ncipher_suite;

#ifdef HAVE_SRP
@@ -866,6 +872,7 @@ static int ssl_hook_Access_classic(request_rec *r, SSLSrvConfigRec *sc, SSLDirCo

        if (renegotiate_quick) {
            STACK_OF(X509) *cert_stack;
            X509 *cert;

            /* perform just a manual re-verification of the peer */
            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02258)
@@ -1017,21 +1024,10 @@ static int ssl_hook_Access_classic(request_rec *r, SSLSrvConfigRec *sc, SSLDirCo
            sslconn->server = r->server;
        }

        /*
         * Remember the peer certificate's DN
         */
        if ((cert = SSL_get_peer_certificate(ssl))) {
            if (sslconn->client_cert) {
                X509_free(sslconn->client_cert);
            }
            sslconn->client_cert = cert;
            sslconn->client_dn = NULL;
        }

        /*
         * Finally check for acceptable renegotiation results
         */
        if (OK != (rc = ssl_check_post_client_verify(r, sc, dc, ssl))) {
        if (OK != (rc = ssl_check_post_client_verify(r, sc, dc, sslconn, ssl))) {
            return rc;
        }

@@ -1055,74 +1051,6 @@ static int ssl_hook_Access_classic(request_rec *r, SSLSrvConfigRec *sc, SSLDirCo
        }
    }

    /* If we're trying to have the user name set from a client
     * certificate then we need to set it here. This should be safe as
     * the user name probably isn't important from an auth checking point
     * of view as the certificate supplied acts in that capacity.
     * However, if FakeAuth is being used then this isn't the case so
     * we need to postpone setting the username until later.
     */
    if ((dc->nOptions & SSL_OPT_FAKEBASICAUTH) == 0 && dc->szUserName) {
        char *val = ssl_var_lookup(r->pool, r->server, r->connection,
                                   r, (char *)dc->szUserName);
        if (val && val[0])
            r->user = val;
        else
            ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(02227)
                          "Failed to set r->user to '%s'", dc->szUserName);
    }

    /*
     * Check SSLRequire boolean expressions
     */
    requires = dc->aRequirement;
    ssl_requires = (ssl_require_t *)requires->elts;

    for (i = 0; i < requires->nelts; i++) {
        ssl_require_t *req = &ssl_requires[i];
        const char *errstring;
        ok = ap_expr_exec(r, req->mpExpr, &errstring);

        if (ok < 0) {
            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02265)
                          "access to %s failed, reason: Failed to execute "
                          "SSL requirement expression: %s",
                          r->filename, errstring);

            /* remember forbidden access for strict require option */
            apr_table_setn(r->notes, "ssl-access-forbidden", "1");

            return HTTP_FORBIDDEN;
        }

        if (ok != 1) {
            ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02266)
                          "Access to %s denied for %s "
                          "(requirement expression not fulfilled)",
                          r->filename, r->useragent_ip);

            ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02228)
                          "Failed expression: %s", req->cpExpr);

            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02229)
                          "access to %s failed, reason: %s",
                          r->filename,
                          "SSL requirement expression not fulfilled");

            /* remember forbidden access for strict require option */
            apr_table_setn(r->notes, "ssl-access-forbidden", "1");

            return HTTP_FORBIDDEN;
        }
    }

    /*
     * Else access is granted from our point of view (except vendor
     * handlers override). But we have to return DECLINED here instead
     * of OK, because mod_auth and other modules still might want to
     * deny access.
     */

    return DECLINED;
}

@@ -1246,7 +1174,7 @@ static int ssl_hook_Access_modern(request_rec *r, SSLSrvConfigRec *sc, SSLDirCon
            /*
             * Finally check for acceptable renegotiation results
             */
            if (OK != (rc = ssl_check_post_client_verify(r, sc, dc, ssl))) {
            if (OK != (rc = ssl_check_post_client_verify(r, sc, dc, sslconn, ssl))) {
                return rc;
            }
        }
@@ -1262,6 +1190,9 @@ int ssl_hook_Access(request_rec *r)
    SSLSrvConfigRec *sc         = mySrvConfig(r->server);
    SSLConnRec *sslconn         = myConnConfig(r->connection);
    SSL *ssl                    = sslconn ? sslconn->ssl : NULL;
    apr_array_header_t *requires;
    ssl_require_t *ssl_requires;
    int ok, i, ret;

    /* On a slave connection, we do not expect to have an SSLConnRec, but
     * our master connection might have one. */
@@ -1317,10 +1248,87 @@ int ssl_hook_Access(request_rec *r)
    /* TLSv1.3+ is less complicated here. Branch off into a new codeline
     * and avoid messing with the past. */
    if (SSL_version(ssl) >= TLS1_3_VERSION) {
        return ssl_hook_Access_modern(r, sc, dc, sslconn, ssl);
        ret = ssl_hook_Access_modern(r, sc, dc, sslconn, ssl);
    }
    else
#endif
    return ssl_hook_Access_classic(r, sc, dc, sslconn, ssl);
    {
        ret = ssl_hook_Access_classic(r, sc, dc, sslconn, ssl);
    }

    if (ret != DECLINED) {
        return ret;
    }

    /* If we're trying to have the user name set from a client
     * certificate then we need to set it here. This should be safe as
     * the user name probably isn't important from an auth checking point
     * of view as the certificate supplied acts in that capacity.
     * However, if FakeAuth is being used then this isn't the case so
     * we need to postpone setting the username until later.
     */
    if ((dc->nOptions & SSL_OPT_FAKEBASICAUTH) == 0 && dc->szUserName) {
        char *val = ssl_var_lookup(r->pool, r->server, r->connection,
                                   r, (char *)dc->szUserName);
        if (val && val[0])
            r->user = val;
        else
            ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(02227)
                          "Failed to set r->user to '%s'", dc->szUserName);
    }

    /*
     * Check SSLRequire boolean expressions
     */
    requires = dc->aRequirement;
    ssl_requires = (ssl_require_t *)requires->elts;

    for (i = 0; i < requires->nelts; i++) {
        ssl_require_t *req = &ssl_requires[i];
        const char *errstring;
        ok = ap_expr_exec(r, req->mpExpr, &errstring);

        if (ok < 0) {
            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02265)
                          "access to %s failed, reason: Failed to execute "
                          "SSL requirement expression: %s",
                          r->filename, errstring);

            /* remember forbidden access for strict require option */
            apr_table_setn(r->notes, "ssl-access-forbidden", "1");

            return HTTP_FORBIDDEN;
        }

        if (ok != 1) {
            ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02266)
                          "Access to %s denied for %s "
                          "(requirement expression not fulfilled)",
                          r->filename, r->useragent_ip);

            ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02228)
                          "Failed expression: %s", req->cpExpr);

            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02229)
                          "access to %s failed, reason: %s",
                          r->filename,
                          "SSL requirement expression not fulfilled");

            /* remember forbidden access for strict require option */
            apr_table_setn(r->notes, "ssl-access-forbidden", "1");

            return HTTP_FORBIDDEN;
        }
    }

    /*
     * Else access is granted from our point of view (except vendor
     * handlers override). But we have to return DECLINED here instead
     * of OK, because mod_auth and other modules still might want to
     * deny access.
     */

    return DECLINED;
}

/*