Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (athena.apache.org: domain of DRuggeri@primary.net
 designates 216.87.38.220 as permitted sender)
Message-ID: <4E74CB7A.3010201@primary.net>
Date: Sat, 17 Sep 2011 11:31:54 -0500
From: Daniel Ruggeri <DRuggeri@primary.net>
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64;
 rv:6.0.2) Gecko/20110902 Thunderbird/6.0.2
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/
 modules/ssl/
References: <20110823193508.9E41A2388A02@eris.apache.org>
 <4E61C3CE.4020500@velox.ch> <4E625521.3000905@opensslfoundation.com>
 <4E629093.1090302@primary.net> <4E64F9A3.6040304@velox.ch>
 <4E652A1B.8020106@primary.net> <4E65ADD6.50301@velox.ch>
 <4E710F48.8000804@primary.net> <4E747886.3090006@velox.ch>
 <4E747E5E.20900@opensslfoundation.com>
In-Reply-To: <4E747E5E.20900@opensslfoundation.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

On 9/17/2011 6:02 AM, Dr Stephen Henson wrote:
> Yes you need store the returned value and free it with X509_free().
>
> Note also that because you ignore return values of X509_verify_cert() you might
> have a situation where the chain is not complete and so deleting the last
> element will remove a non-root CA.

Both suggestions make sense - here is what was just committed to
trunk... I also added logging of verification failures at WARNING level.
Since I was in the file again anyhow, I added logging at DEBUG of what
gets loaded and the order so there is no ambiguity.

...
    for (n = 0; n < ncerts; n++) {
        int i, res;
        char cert_cn[256];

        X509_INFO *inf = sk_X509_INFO_value(pkp->certs, n);
        X509_NAME *name = X509_get_subject_name(inf->x509);
        X509_NAME_oneline(name, cert_cn, sizeof(cert_cn));
        X509_STORE_CTX_init(sctx, store, inf->x509, NULL);

        res=X509_verify_cert(sctx);
        chain = X509_STORE_CTX_get1_chain(sctx);

        if (res == 1) {
            /* Removing the client cert if verification is OK
             * could save a loop when choosing which cert to send
             * when more than one is available */
            /* XXX: This is not needed if we collapse the two
             * checks in ssl_engine_kernel in the future */
            X509_free(sk_X509_shift(chain));
        }
        else {
            int n = X509_STORE_CTX_get_error(sctx);
            ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s,
                         "SSL proxy client cert chain verification
failed for %s: %s",
                         cert_cn, X509_verify_cert_error_string(n));
        }
        ERR_clear_error();
        i = sk_X509_num(chain);
        pkp->ca_certs[n] = chain;
        if (i == 0 || (res != 1 && i == 1) ) {
            /* zero or only the client cert won't be very useful
             * due to verification failure */
            sk_X509_pop_free(chain, X509_free);
            i = 0;
            pkp->ca_certs[n] = NULL;
        }

        X509_STORE_CTX_cleanup(sctx);

        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
                     "loaded %i intermediate CA%s for cert %i (%s)",
                     i, i == 1 ? "" : "s", n, cert_cn);
        if (i > 0) {
            int j;
            for (j=0; j<i; j++) {
                char ca_cn[256];
                X509_NAME *ca_name =
X509_get_subject_name(sk_X509_value(chain, j));
                X509_NAME_oneline(ca_name, ca_cn, sizeof(ca_cn));
                ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "%i: %s", j,
ca_cn);
            }
        }
    }

    X509_STORE_CTX_free(sctx);
...

-- 
Daniel Ruggeri