On 25.9.11 18:54, Daniel Ruggeri wrote: > On 9/23/2011 10:07 AM, Kaspar Brand wrote: > Alternatively, we could adjust the callback and init functions to always > build a chain (even if SSLProxyMachineCertificateChainFile is not set) > and check "by chain" by doing the X509_NAME_cmp for each item in the > STACK_OF(X509) in pkp->ca_certs rather than checking the issuer of each > item in pkp->certs. If the new directive is not set, everything would > *essentially* function the same way. To me, they are two ways to do the > same thing, though with the current approach, the verification messages > in startup will not show up unless using the new directive.
I'm fine with leaving it as is, i.e. having the EE certs in pkp->certs and the issuer certs in pkp->ca_certs - storing the EE cert twice would appear rather clumsy to me. I therefore suggest dropping the XXX comment, and perhaps add a few words to the definition of the "certs" and "ca_certs" members of the modssl_pk_proxy_t struct in ssl_private.h. >>> pkp->ca_certs[n] = NULL; >> Strictly speaking, the last assignment isn't necessary, since your >> calloc'ing ca_certs before. > > Setting to NULL will be caught by the update Rüdiger put in for 1162103 > and will skip all of the new logic in the callback function. IMO, I feel > this way is just a bit cleaner and easier to follow. I can be swayed if > you feel strongly about it, though. What I meant is that calloc() already sets "all bits zero", by definition. However, as I learned in the meantime, there are indeed machines where the memory representation of NULL is non-null (http://c-faq.com/null/machexamp.html), so I have been convinced. > I'll wait for feedback before committing and updating 2.2 STATUS. Go ahead, I'll add my (nonbinding) vote afterwards :-) Just one (hopefully last) thing I overlooked before: the "ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s);" line before the ssl_die() call apparently got lost somewhere on its way to the tree - could you re-add that? It makes sure we also capture the OpenSSL error in the log, before aborting. Kaspar
