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: local policy)
Message-ID: <4E64F9A3.6040304@velox.ch>
Date: Mon, 05 Sep 2011 18:32:35 +0200
From: Kaspar Brand <httpd-dev.2011@velox.ch>
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>
In-Reply-To: <4E629093.1090302@primary.net>
Content-Type: multipart/mixed;
 boundary="------------070603090409030106040303"

This is a multi-part message in MIME format.
--------------070603090409030106040303
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

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

--------------070603090409030106040303
Content-Type: text/plain;
 name="ssl_engine_init.c.diff"
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
 filename="ssl_engine_init.c.diff"

SW5kZXg6IHNzbF9lbmdpbmVfaW5pdC5jCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIHNzbF9lbmdpbmVf
aW5pdC5jCShyZXZpc2lvbiAxMTY1MzQ3KQorKysgc3NsX2VuZ2luZV9pbml0LmMJKHdvcmtp
bmcgY29weSkKQEAgLTExMTUsNyArMTExNSw5IEBAIHN0YXRpYyB2b2lkIHNzbF9pbml0X3By
b3h5X2NlcnRzKHNlcnZlcl9yZWMgKnMsCiAgICAgaW50IG4sIG5jZXJ0cyA9IDA7CiAgICAg
U1RBQ0tfT0YoWDUwOV9JTkZPKSAqc2s7CiAgICAgbW9kc3NsX3BrX3Byb3h5X3QgKnBrcCA9
IG1jdHgtPnBrcDsKLSAgICBTVEFDS19PRihYNTA5X0lORk8pICpjaGFpbjsKKyAgICBTVEFD
S19PRihYNTA5KSAqY2hhaW47CisgICAgWDUwOV9TVE9SRV9DVFggKnNjdHg7CisgICAgWDUw
OV9TVE9SRSAqc3RvcmUgPSBTU0xfQ1RYX2dldF9jZXJ0X3N0b3JlKG1jdHgtPnNzbF9jdHgp
OwogCiAgICAgU1NMX0NUWF9zZXRfY2xpZW50X2NlcnRfY2IobWN0eC0+c3NsX2N0eCwKICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICBzc2xfY2FsbGJhY2tfcHJveHlfY2VydCk7
CkBAIC0xMTYxLDI5ICsxMTYzLDQxIEBAIHN0YXRpYyB2b2lkIHNzbF9pbml0X3Byb3h5X2Nl
cnRzKHNlcnZlcl9yZWMgKnMsCiAgICAgICAgICAgICAgICAgIG5jZXJ0cyk7CiAgICAgcGtw
LT5jZXJ0cyA9IHNrOwogCi0gICAgaWYgKCFwa3AtPmNhX2NlcnRfZmlsZSkgeworICAgIGlm
ICghcGtwLT5jYV9jZXJ0X2ZpbGUgfHwgIXN0b3JlKSB7CiAgICAgICAgIHJldHVybjsKICAg
ICB9CiAKLSAgICAvKiBMb2FkIGFsbCBvZiB0aGUgQ0EgY2VydHMgYW5kIGNvbnN0cnVjdCBh
IGNoYWluICovCi0gICAgc2sgPSBza19YNTA5X0lORk9fbmV3X251bGwoKTsKKyAgICBzY3R4
ID0gWDUwOV9TVE9SRV9DVFhfbmV3KCk7CisgICAgaWYgKCFzY3R4KSB7CisgICAgICAgIGFw
X2xvZ19lcnJvcihBUExPR19NQVJLLCBBUExPR19FTUVSRywgMCwgcywKKyAgICAgICAgICAg
ICAgICAgICAgICJTU0wgcHJveHkgY2xpZW50IGNlcnQgaW5pdGlhbGl6YXRpb24gZmFpbGVk
Iik7CisgICAgICAgIHNzbF9sb2dfc3NsX2Vycm9yKFNTTExPR19NQVJLLCBBUExPR19FTUVS
Rywgcyk7CisgICAgICAgIHNzbF9kaWUoKTsKKyAgICB9CiAKLSAgICBTU0xfWDUwOV9JTkZP
X2xvYWRfZmlsZShwdGVtcCwgc2ssIHBrcC0+Y2FfY2VydF9maWxlKTsKLSAgICBwa3AtPmNh
X2NlcnRzID0gKFNUQUNLX09GKFg1MDlfSU5GTykgKiopIGFwcl9wY2FsbG9jKHAsIG5jZXJ0
cyAqIHNpemVvZihzaykpOwotCisgICAgWDUwOV9TVE9SRV9sb2FkX2xvY2F0aW9ucyhzdG9y
ZSwgcGtwLT5jYV9jZXJ0X2ZpbGUsIE5VTEwpOworICAgIAogICAgIGZvciAobiA9IDA7IG4g
PCBuY2VydHM7IG4rKykgewotICAgICAgICBpbnQgbGVuOworICAgICAgICBpbnQgaSwgbGVu
OwogICAgICAgICBYNTA5X0lORk8gKmluZiA9IHNrX1g1MDlfSU5GT192YWx1ZShwa3AtPmNl
cnRzLCBuKTsKLSAgICAgICAgY2hhaW4gPSBza19YNTA5X0lORk9fbmV3X251bGwoKTsKLSAg
ICAgICAgbGVuID0gU1NMX1g1MDlfSU5GT19jcmVhdGVfY2hhaW4oaW5mLT54NTA5LCBzaywg
Y2hhaW4pOworCisgICAgICAgIFg1MDlfU1RPUkVfQ1RYX2luaXQoc2N0eCwgc3RvcmUsIGlu
Zi0+eDUwOSwgTlVMTCk7CisgICAgICAgIFg1MDlfdmVyaWZ5X2NlcnQoc2N0eCk7CisgICAg
ICAgIEVSUl9jbGVhcl9lcnJvcigpOworICAgICAgIAorICAgICAgICBjaGFpbiA9IHNrX1g1
MDlfbmV3X251bGwoKTsKKyAgICAgICAgZm9yIChpID0gMDsgaSA8IHNrX1g1MDlfbnVtKHNj
dHgtPmNoYWluKTsgaSsrKSB7CisgICAgICAgICAgIHNrX1g1MDlfcHVzaChjaGFpbiwgc2tf
WDUwOV92YWx1ZShzY3R4LT5jaGFpbiwgaSkpOworICAgICAgICB9CiAgICAgICAgIHBrcC0+
Y2FfY2VydHNbbl0gPSBjaGFpbjsKIAogICAgICAgICBhcF9sb2dfZXJyb3IoQVBMT0dfTUFS
SywgQVBMT0dfREVCVUcsIDAsIHMsCiAgICAgICAgICAgICAgICAgICAgICAiY2xpZW50IGNl
cnRpZmljYXRlICVpIGhhcyBsb2FkZWQgJWkgIgotICAgICAgICAgICAgICAgICAgICAgImlu
dGVybWVkaWFyeSBzaWduZXJzICIsIG4sIGxlbik7CisgICAgICAgICAgICAgICAgICAgICAi
aW50ZXJtZWRpYXRlIENBcyIsIG4sIGkpOwogICAgIH0KIAogICAgIHNrX1g1MDlfSU5GT19m
cmVlKHNrKTsKKyAgICBYNTA5X1NUT1JFX0NUWF9mcmVlKHNjdHgpOwogfQogCiBzdGF0aWMg
dm9pZCBzc2xfaW5pdF9wcm94eV9jdHgoc2VydmVyX3JlYyAqcywK
--------------070603090409030106040303--