You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Kaspar Brand <ht...@velox.ch> on 2011/09/03 08:06:06 UTC

Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/ modules/ssl/

> Author: druggeri
> Date: Tue Aug 23 19:35:07 2011
> New Revision: 1160863
> 
> URL: http://svn.apache.org/viewvc?rev=1160863&view=rev
> Log:
> Add SSLProxyMachineCertificateChainFile directive and documentation for bug 50812

Sorry for being late with my comments...


> +        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
> +                     "client certificate %i has loaded %i "
> +                     "intermediary signers ", n, len);

Nit: could you replace "intermediary" by "intermediate" in all log
messages and comments? The former isn't really an X.509/PKIX term. (In
the above message, I suggest saying "intermediate CA certificates".)


> Modified: httpd/httpd/trunk/modules/ssl/ssl_util_ssl.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_util_ssl.c?rev=1160863&r1=1160862&r2=1160863&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_util_ssl.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_util_ssl.c Tue Aug 23 19:35:07 2011
> @@ -434,6 +434,45 @@ BOOL SSL_X509_INFO_load_path(apr_pool_t 
>      return ok;
>  }
>  
> +/*
> + * Construct a stack of X509_INFO containing only certificates
> + * that have signed the provided certificate or are an intermediary
> + * signer of the certificate
> +*/
> +int SSL_X509_INFO_create_chain(const X509 *x509,
> +                             STACK_OF(X509_INFO) *ca_certs,
> +                             STACK_OF(X509_INFO) *chain)
> +{
> +    int can_proceed=1;
> +    int len=0;
> +    int i;
> +    X509 *certificate = (X509 *)x509;
> +    X509_INFO *info;
> +    X509_NAME *cert_issuer_name, *ca_name, *ca_issuer_name;
> +
> +    while (can_proceed) {
> +        can_proceed = 0;
> +        cert_issuer_name = X509_get_issuer_name(certificate);
> +
> +        for (i = 0; i < sk_X509_INFO_num(ca_certs); i++) {
> +            info = sk_X509_INFO_value(ca_certs, i);
> +            ca_name = X509_get_subject_name(info->x509);
> +            ca_issuer_name = X509_get_issuer_name(info->x509);
> +
> +            if (X509_NAME_cmp(cert_issuer_name, ca_name) == 0) {
> +                /* Check for a self-signed cert (no issuer) */
> +                can_proceed=X509_NAME_cmp(ca_name, ca_issuer_name) == 0 ? 0 : 1;
> +                len++;
> +                certificate = info->x509;
> +                sk_X509_INFO_unshift(chain, info);
> +                break;
> +            }
> +        }
> +    }
> +
> +    return len;
> +}
> +

I think it's preferrable to let OpenSSL build the chain (instead of
doing it ourselves). There's no readily available function for this,
unfortunately, but could you try something along the lines in OpenSSL's
s3_both.c:ssl3_output_cert_chain()? See

  http://cvs.openssl.org/chngview?cn=18326

I.e., use X509_verify_cert(), ignore its result, but grab the chain from
the X509_STORE_CTX afterwards. (And when you're done, it's probably
wise to call ERR_clear_error, see http://cvs.openssl.org/chngview?cn=19472).

Kaspar

Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/modules/ssl/

Posted by Daniel Ruggeri <DR...@primary.net>.
On 9/3/2011 1:06 AM, Kaspar Brand wrote:
> Nit: could you replace "intermediary" by "intermediate" in all log
> messages and comments? The former isn't really an X.509/PKIX term. (In
> the above message, I suggest saying "intermediate CA certificates".)
No problem.

> I think it's preferrable to let OpenSSL build the chain (instead of
> doing it ourselves). There's no readily available function for this,
> unfortunately, but could you try something along the lines in OpenSSL's
> s3_both.c:ssl3_output_cert_chain()? See
>
>   http://cvs.openssl.org/chngview?cn=18326
>
> I.e., use X509_verify_cert(), ignore its result, but grab the chain from
> the X509_STORE_CTX afterwards. (And when you're done, it's probably
> wise to call ERR_clear_error, see http://cvs.openssl.org/chngview?cn=19472).
I searched for a function to do exactly this and came up empty. Thank
you very much for bringing this to my attention! I'll definitely update
the patch with this because the method I'm using is certainly a
sticks-and-stones approach.

-- 
--
Daniel Ruggeri

Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/ modules/ssl/

Posted by Daniel Ruggeri <DR...@primary.net>.
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


Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/ modules/ssl/

Posted by Dr Stephen Henson <sh...@opensslfoundation.com>.
On 17/09/2011 11:37, Kaspar Brand wrote:
> On 14.09.2011 22:32, Daniel Ruggeri wrote:
>> My usage tests pass muster with the approach we have discussed, so I
>> have updated trunk and the 2.2 backport proposal. At this point, I am
>> satisfied with this particular patch, though I won't lose sight of the
>> server-side issue. Since the patch should now be complete, I have given
>> my vote in the 2.2 STATUS file and would appreciate any further
>> review/votes.
> 
> Looks good, judging from my (admittedly not very comprehensive) testing.
> 
>> +        chain = X509_STORE_CTX_get1_chain(sctx);
>> +        sk_X509_shift(chain);
>> +        i=sk_X509_num(chain);
>>          pkp->ca_certs[n] = chain;
> 
> I think with the "sk_X509_shift(chain)" line, we're leaking an X509
> struct... maybe Steve can confirm (?). Adding a short comment as to why
> the first cert is dropped would be useful IMO, too.
> 

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.

Steve.
-- 
Dr Stephen Henson. OpenSSL Software Foundation, Inc.
1829 Mount Ephraim Road
Adamstown, MD 21710
+1 877-673-6775
shenson@opensslfoundation.com

Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/ modules/ssl/

Posted by Kaspar Brand <ht...@velox.ch>.
On 14.09.2011 22:32, Daniel Ruggeri wrote:
> My usage tests pass muster with the approach we have discussed, so I
> have updated trunk and the 2.2 backport proposal. At this point, I am
> satisfied with this particular patch, though I won't lose sight of the
> server-side issue. Since the patch should now be complete, I have given
> my vote in the 2.2 STATUS file and would appreciate any further
> review/votes.

Looks good, judging from my (admittedly not very comprehensive) testing.

> +        chain = X509_STORE_CTX_get1_chain(sctx);
> +        sk_X509_shift(chain);
> +        i=sk_X509_num(chain);
>          pkp->ca_certs[n] = chain;

I think with the "sk_X509_shift(chain)" line, we're leaking an X509
struct... maybe Steve can confirm (?). Adding a short comment as to why
the first cert is dropped would be useful IMO, too.

Second point, somewhat related: if we end up with an empty chain at this
point, then it seems preferrable to me to discard/free the
STACK_OF(X509) right on the spot (and not store it in pkp->ca_certs).

Kaspar

Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/ modules/ssl/

Posted by Daniel Ruggeri <DR...@primary.net>.
On 9/6/2011 12:21 AM, Kaspar Brand wrote:
> On 05.09.2011 21:59, Daniel Ruggeri wrote:
>> could be reused to build a chain for the server-side of mod_ssl (because
>> today, the chain certs get presented in whatever order they are in the
>> file resulting in unhappy java clients). With a little bit of
>> refactoring on the server side, this could be taken care of just as well.
> I agree, this is definitely desirable, and we should certainly do it for
> trunk. As suggested by Steve, we shouldn't simply ignore all
> X509_verify_cert errors in this case, too. Something like
> modssl_check_cert(), which returns a proper chain on success, would be
> my idea.
>
>> I've made a few adjustments and built/tested the snippet below. Works
>> well, though in my test cases I can't tell if the chain is being sent or
>> not (suggestions on how to verify?).
> If you have a proxied server which runs httpd/mod_ssl, then you can use
> the SSLOptions +ExportCertData, and look for the SSL_CLIENT_CERT_CHAIN_n
> environment vars.
>

My usage tests pass muster with the approach we have discussed, so I
have updated trunk and the 2.2 backport proposal. At this point, I am
satisfied with this particular patch, though I won't lose sight of the
server-side issue. Since the patch should now be complete, I have given
my vote in the 2.2 STATUS file and would appreciate any further
review/votes.

-- 
Daniel Ruggeri


Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/ modules/ssl/

Posted by Kaspar Brand <ht...@velox.ch>.
On 05.09.2011 21:59, Daniel Ruggeri wrote:
> could be reused to build a chain for the server-side of mod_ssl (because
> today, the chain certs get presented in whatever order they are in the
> file resulting in unhappy java clients). With a little bit of
> refactoring on the server side, this could be taken care of just as well.

I agree, this is definitely desirable, and we should certainly do it for
trunk. As suggested by Steve, we shouldn't simply ignore all
X509_verify_cert errors in this case, too. Something like
modssl_check_cert(), which returns a proper chain on success, would be
my idea.

> I've made a few adjustments and built/tested the snippet below. Works
> well, though in my test cases I can't tell if the chain is being sent or
> not (suggestions on how to verify?).

If you have a proxied server which runs httpd/mod_ssl, then you can use
the SSLOptions +ExportCertData, and look for the SSL_CLIENT_CERT_CHAIN_n
environment vars.

> A potential solution to this is to create a directive controlling
> whether a new NULL context is used when loading the store or the
> existing SSL context. In the documentation for both directives, we could
> inform the server admin the impact of either decision.

I'm somewhat reluctant towards adding even more config knobs, but if
it's unavoidable... too bad extra chain certs can't be set at the SSL*
level.

> FWIW, RFC 2246 (SSL 3.1/TLS 1.0), RFC 4346 (SSL 3.2/TLS 1.1) and RFC
> 5246 (SSL 3.3/TLS 1.2) place no requirements on sending a chain aside
> from making it clear that a chain can be sent. I would say for the
> largest range of compatibility, a chain should be sent, but it's not a
> requirement if it makes the server admin uncomfortable with the openssl
> trust side effect.

It might be a wording issue (there's no explicit MUST), but the
statement under the "Meaning of this message" in RFC 5246 makes it
relatively clear that you're expected to send a chain (you may omit the
root, yes, but not the intermediates).

If we're the client, then it's definitely in our interest to send the
chain, I think - otherwise you would have to ask the server admin to
explicitly add our intermediate CA cert(s) to his store.

Kaspar

Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/ modules/ssl/

Posted by Daniel Ruggeri <DR...@primary.net>.
On 9/5/2011 11:32 AM, Kaspar Brand wrote:
> Attached is an *untested* patch which hopefully gives you an idea of the
> approach I'm suggesting (you might still want to separate the chain
> building into a function of its own, I simply left t inline in
> ssl_init_proxy_certs for easier editing). Not sure if it works, but
> would appreciate if you could give it a try.

Yes, I like the suggestion. I added some constraints to what I was doing
by trying to design a function that would take X509_INFO so the function
could be reused to build a chain for the server-side of mod_ssl (because
today, the chain certs get presented in whatever order they are in the
file resulting in unhappy java clients). With a little bit of
refactoring on the server side, this could be taken care of just as well.

I've made a few adjustments and built/tested the snippet below. Works
well, though in my test cases I can't tell if the chain is being sent or
not (suggestions on how to verify?).


On 9/5/2011 11:52 AM, Dr Stephen Henson wrote:
> Potential gotcha is that you end up loading up client CAs in the trusted
> certificate store which isn't always what you want. For example if that context
> gets reused they'll be trusted server CA certificates later.

I would say that a case where a server admin doesn't wish to trust
issuers of their own certs is remote, but possible. I think an
appropriately worded blurb in the documentation would be important.
Also, since this functionality hasn't existed yet, I'm inclined to think
that even fewer folks would be impacted.
A potential solution to this is to create a directive controlling
whether a new NULL context is used when loading the store or the
existing SSL context. In the documentation for both directives, we could
inform the server admin the impact of either decision.

FWIW, RFC 2246 (SSL 3.1/TLS 1.0), RFC 4346 (SSL 3.2/TLS 1.1) and RFC
5246 (SSL 3.3/TLS 1.2) place no requirements on sending a chain aside
from making it clear that a chain can be sent. I would say for the
largest range of compatibility, a chain should be sent, but it's not a
requirement if it makes the server admin uncomfortable with the openssl
trust side effect.


I'll clean up the work and update trunk as well as the 2.2 backport
patch sometime later this week.


static void ssl_init_proxy_certs(server_rec *s,
                                 apr_pool_t *p,
                                 apr_pool_t *ptemp,
                                 modssl_ctx_t *mctx)
{
    int n, ncerts = 0;
    STACK_OF(X509_INFO) *sk;
    STACK_OF(X509) *chain;
    X509_STORE_CTX *sctx;
    X509_STORE *store = SSL_CTX_get_cert_store(mctx->ssl_ctx);
    modssl_pk_proxy_t *pkp = mctx->pkp;

/* ... */

    if (!pkp->ca_cert_file || !store) {
        return;
    }

    /* Load all of the CA certs and construct a chain */
    pkp->ca_certs = (STACK_OF(X509) **) apr_pcalloc(p, ncerts * sizeof(sk));
    sctx = X509_STORE_CTX_new();

    if (!sctx) {
        ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s,
                     "SSL proxy client cert initialization failed");
        ssl_log_ssl_error(APLOG_MARK, APLOG_ERR, s);
        ssl_die();
    }

    X509_STORE_load_locations(store, pkp->ca_cert_file, NULL);

    for (n = 0; n < ncerts; n++) {
        int i;
        X509_INFO *inf = sk_X509_INFO_value(pkp->certs, n);
        X509_STORE_CTX_init(sctx, store, inf->x509, NULL);
        X509_verify_cert(sctx);
        ERR_clear_error();

        chain = X509_STORE_CTX_get1_chain(sctx);
        sk_X509_shift(chain);
        i=sk_X509_num(chain);
        pkp->ca_certs[n] = chain;
        X509_STORE_CTX_cleanup(sctx);

        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
                     "client certificate %i has loaded %i "
                     "intermediate CA%s", n, i, i == 1 ? "" : "s");
    }

    X509_STORE_CTX_free(sctx);
}

-- 
--
Daniel Ruggeri

Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/ modules/ssl/

Posted by Dr Stephen Henson <sh...@opensslfoundation.com>.
On 05/09/2011 17:32, Kaspar Brand wrote:
> 
> SSL_CTX_add_extra_chain_cert() isn't an option for us, I assume, so the
> best we can probably do in ssl_init_proxy_certs()) is to load the certs
> specified via ProxyMachineCertificateChainFile with
> X509_STORE_load_locations and rely on OpenSSL's "auto chain building"
> afterwards.
> 

Potential gotcha is that you end up loading up client CAs in the trusted
certificate store which isn't always what you want. For example if that context
gets reused they'll be trusted server CA certificates later.

Also I think this:

+        chain = sk_X509_new_null();
+        for (i = 0; i < sk_X509_num(sctx->chain); i++) {
+           sk_X509_push(chain, sk_X509_value(sctx->chain, i));
+        }

along with:

+    X509_STORE_CTX_free(sctx);

will end up freeing up the certificates you just added to the chain. You can
replace that all with:

chain = X509_STORE_CTX_get1_chain(sctx);

which creates a STACK_OF(X509) and ups the reference count of the added
certificates so they stick around after you free the context.

Steve.
-- 
Dr Stephen Henson. OpenSSL Software Foundation, Inc.
1829 Mount Ephraim Road
Adamstown, MD 21710
+1 877-673-6775
shenson@opensslfoundation.com

Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/ modules/ssl/

Posted by Kaspar Brand <ht...@velox.ch>.
On 03.09.2011 22:39, Daniel Ruggeri wrote:
> updated method I'm using. Rather than storing the chain as
> STACK_OF(X509_INFO) I have switched to STACK_OF(X509) and am using the
> following function to build the chain.
> Comments are definitely appreciated as I don't have a very good frame of
> reference for using X509_verify_cert().

Switching to STACK_OF(X509) is the right thing, yes (you don't need
X509_INFO and its additional fields for the CA certs). After having
looked at ssl_init_proxy_certs() and ssl_callback_proxy_cert() a bit
more closely, I think some additional changes might be needed.

Thanks to your patch, ssl_callback_proxy_cert() will now pick the proper
cert even if the ca_list does not include the DN of the CA which signed
the cert, which is definitely an improvement. There's still one
drawback, though, IINM: for proxy connections, mod_ssl currently won't
include the intermediate CA certs of its client cert(s) in outgoing
connections.

As SSL_CTX_set_client_cert_cb(3) states:

       The client_cert_cb() cannot return a complete certificate
       chain, it can only return one client certificate. If the
       chain only has a length of 2, the root CA certificate may
       be omitted according to the TLS standard and thus a stan-
       dard conforming answer can be sent to the server. For a
       longer chain, the client must send the complete chain
       (with the option to leave out the root CA certificate).
       This can only be accomplished by either adding the inter-
       mediate CA certificates into the trusted certificate
       store for the SSL_CTX object (resulting in having to add
       CA certificates that otherwise maybe would not be
       trusted), or by adding the chain certificates using the
       SSL_CTX_add_extra_chain_cert(3) function, which is only
       available for the SSL_CTX object as a whole and that
       therefore probably can only apply for one client certifi-
       cate, making the concept of the callback function (to
       allow the choice from several certificates) questionable.

SSL_CTX_add_extra_chain_cert() isn't an option for us, I assume, so the
best we can probably do in ssl_init_proxy_certs()) is to load the certs
specified via ProxyMachineCertificateChainFile with
X509_STORE_load_locations and rely on OpenSSL's "auto chain building"
afterwards.

Populating the mctx->pkp->ca_certs stacks is still required, however,
since these are needed in the callback to figure out the correct client
cert.

Attached is an *untested* patch which hopefully gives you an idea of the
approach I'm suggesting (you might still want to separate the chain
building into a function of its own, I simply left t inline in
ssl_init_proxy_certs for easier editing). Not sure if it works, but
would appreciate if you could give it a try.

Kaspar

Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/ modules/ssl/

Posted by Daniel Ruggeri <DR...@primary.net>.
On 9/3/2011 11:26 AM, Dr Stephen Henson wrote:
> Some errors (signature error, expired certificates) should arguably logged or
> even treated as fatal errors. This would be because the cause is a badly
> configured server and it is better to get the user to fix their configuration
> than send a certificate chain that is invalid.
>
> In other cases you may hit problems because sometimes a certificate "chain"
> which doesn't quite fit the PKIX definition is used. An example would be a proxy
> certificate chain (for some value of "proxy", not necessarily standard)
> where some certificates in the chain are not CA certificates in the normal
> definition (basic constraints CA=TRUE). That kind of "chain" cannot directly be
> built up using X509_verify_cert().

Thank you for the note. I was hoping you and Kaspar would comment on the
updated method I'm using. Rather than storing the chain as
STACK_OF(X509_INFO) I have switched to STACK_OF(X509) and am using the
following function to build the chain.
Comments are definitely appreciated as I don't have a very good frame of
reference for using X509_verify_cert().

int SSL_X509_create_chain(const X509 *x509,
                          STACK_OF(X509_INFO) *ca_certs,
                          STACK_OF(X509) *chain)
{
    int i;
    X509_STORE_CTX *ctx;
    X509 *cert = (X509 *)x509;
    X509_INFO *ca_cert;
    STACK_OF(X509) *verified_stack;
    STACK_OF(X509) *tmp_stack=sk_X509_new_null();

    /* construct a temporary X509 chain from the X509_INFO chain */
    for(i = 0; i < sk_X509_INFO_num(ca_certs); i++) {
        ca_cert=sk_X509_INFO_value(ca_certs, i);
        sk_X509_push(tmp_stack, ca_cert->x509);
        }

    ctx = X509_STORE_CTX_new();
    if (ctx == NULL){
        sk_X509_pop_free(tmp_stack, X509_free);
        return -1;
        }
    if (!X509_STORE_CTX_init(ctx, NULL, cert, NULL)) {
        sk_X509_pop_free(tmp_stack, X509_free);
        return -1;
        }
    X509_STORE_CTX_trusted_stack(ctx, tmp_stack);

    X509_verify_cert(ctx);
    /* Ignore verification errors */
    ERR_clear_error();

    verified_stack=X509_STORE_CTX_get1_chain(ctx);

    for(i = sk_X509_num(tmp_stack) - 1; i >= 0; i--) {
        sk_X509_push(chain, sk_X509_value(tmp_stack, i));
    }

    X509_STORE_CTX_free(ctx);
    return sk_X509_num(chain);

}

-- 
--
Daniel Ruggeri

Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/ modules/ssl/

Posted by Dr Stephen Henson <sh...@opensslfoundation.com>.
On 03/09/2011 07:06, Kaspar Brand wrote:
> 
>> Modified: httpd/httpd/trunk/modules/ssl/ssl_util_ssl.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_util_ssl.c?rev=1160863&r1=1160862&r2=1160863&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/ssl/ssl_util_ssl.c (original)
>> +++ httpd/httpd/trunk/modules/ssl/ssl_util_ssl.c Tue Aug 23 19:35:07 2011
>> @@ -434,6 +434,45 @@ BOOL SSL_X509_INFO_load_path(apr_pool_t 
>>      return ok;
>>  }
>>  
>> +/*
>> + * Construct a stack of X509_INFO containing only certificates
>> + * that have signed the provided certificate or are an intermediary
>> + * signer of the certificate
>> +*/
>> +int SSL_X509_INFO_create_chain(const X509 *x509,
>> +                             STACK_OF(X509_INFO) *ca_certs,
>> +                             STACK_OF(X509_INFO) *chain)
>> +{
>> +    int can_proceed=1;
>> +    int len=0;
>> +    int i;
>> +    X509 *certificate = (X509 *)x509;
>> +    X509_INFO *info;
>> +    X509_NAME *cert_issuer_name, *ca_name, *ca_issuer_name;
>> +
>> +    while (can_proceed) {
>> +        can_proceed = 0;
>> +        cert_issuer_name = X509_get_issuer_name(certificate);
>> +
>> +        for (i = 0; i < sk_X509_INFO_num(ca_certs); i++) {
>> +            info = sk_X509_INFO_value(ca_certs, i);
>> +            ca_name = X509_get_subject_name(info->x509);
>> +            ca_issuer_name = X509_get_issuer_name(info->x509);
>> +
>> +            if (X509_NAME_cmp(cert_issuer_name, ca_name) == 0) {
>> +                /* Check for a self-signed cert (no issuer) */
>> +                can_proceed=X509_NAME_cmp(ca_name, ca_issuer_name) == 0 ? 0 : 1;
>> +                len++;
>> +                certificate = info->x509;
>> +                sk_X509_INFO_unshift(chain, info);
>> +                break;
>> +            }
>> +        }
>> +    }
>> +
>> +    return len;
>> +}
>> +
> 
> I think it's preferrable to let OpenSSL build the chain (instead of
> doing it ourselves). There's no readily available function for this,
> unfortunately, but could you try something along the lines in OpenSSL's
> s3_both.c:ssl3_output_cert_chain()? See
> 
>   http://cvs.openssl.org/chngview?cn=18326
> 
> I.e., use X509_verify_cert(), ignore its result, but grab the chain from
> the X509_STORE_CTX afterwards. (And when you're done, it's probably
> wise to call ERR_clear_error, see http://cvs.openssl.org/chngview?cn=19472).
> 

Sorry for the duplicate comment missed this.

There are a few side effects with using X509_verify_cert().

Some errors (signature error, expired certificates) should arguably logged or
even treated as fatal errors. This would be because the cause is a badly
configured server and it is better to get the user to fix their configuration
than send a certificate chain that is invalid.

In other cases you may hit problems because sometimes a certificate "chain"
which doesn't quite fit the PKIX definition is used. An example would be a proxy
certificate chain (for some value of "proxy", not necessarily standard)
where some certificates in the chain are not CA certificates in the normal
definition (basic constraints CA=TRUE). That kind of "chain" cannot directly be
built up using X509_verify_cert().

Steve.
-- 
Dr Stephen Henson. OpenSSL Software Foundation, Inc.
1829 Mount Ephraim Road
Adamstown, MD 21710
+1 877-673-6775
shenson@opensslfoundation.com