Re: svn commit: r1550060 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c

[Jan Kaluza <jk...@redhat.com>]

On 12/13/2013 09:54 AM, Ruediger Pluem wrote:
>
>
> jkaluza@apache.org wrote:
>> Author: jkaluza
>> Date: Wed Dec 11 07:16:28 2013
>> New Revision: 1550060
>>
>> URL: http://svn.apache.org/r1550060
>> Log:
>> mod_ssl: Add -t -DDUMP_CA_CERTS option which dumps the filenames of all
>> configured SSL CA certificates to stdout the same way as DUMP_CERTS does.
>>
>> Modified:
>>      httpd/httpd/trunk/CHANGES
>>      httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
>>
>
>> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_config.c?rev=1550060&r1=1550059&r2=1550060&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/ssl/ssl_engine_config.c (original)
>> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_config.c Wed Dec 11 07:16:28 2013
>> @@ -1870,30 +1870,95 @@ const char *ssl_cmd_SSLSRPUnknownUserSee
>>
>>   #endif /* HAVE_SRP */
>>
>> -void ssl_hook_ConfigTest(apr_pool_t *pconf, server_rec *s)
>> +static void dump_ca_cert_file(apr_file_t *out, const char *file) {
>> +    X509 *rc;
>> +    BIO *bioS;
>> +
>> +    if ((bioS=BIO_new_file(file, "r")) == NULL) {
>> +        return;
>> +    }
>> +
>> +    /* ca_cert_file is loaded using SSL_load_client_CA_file(). This method
>> +     * loads only file of PEM formatted certificates, so we have to load
>> +     * only PEM here too, to stay consistent.
>> +     */
>> +    rc = PEM_read_bio_X509 (bioS, NULL, NULL, NULL);
>> +    BIO_free(bioS);
>> +    if (rc) {
>> +        apr_file_printf(out, "  %s\n", file);
>
> Hm, do I miss the point or why don't we print out anything from the cert we just loaded and just the filename?
> Do we just want to know if the cert file loads?

At first sorry for late response, I was away during Christmas time. The 
idea was to stay consistent with what DUMP_CERTS does, so DUMP_CA_CERTS 
prints only filename of the cert so it can be later passed to certwatch.

I'm trying to load the certificate to find out if it's valid. I think 
this has to be done, because you can use SSLCACertificatePath to set 
path to directory containing CA certificates and to print really only 
valid certificates from this directory, we have to actually try to load 
them and ignore those which can't be loaded.

This is not done for DUMP_CERTS, because there is no directory 
equivalent of SSLCertificateFile (which is logical).

If we presume that directory specified by SSLCACertificatePath contains 
only valid certificates and no other files, we could remove that 
validity check and just print filenames of all files in that directory.

> Regards
>
> Rüdiger
>

Regards,
Jan Kaluza

Adding -DDUMP_CA_CERTS for mod_ssl (Re: svn commit: r1550060 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c)

[Kaspar Brand <ht...@velox.ch>]

On 06.01.2014 08:46, Jan Kaluža wrote:
> On 01/05/2014 11:10 AM, Kaspar Brand wrote:
> I think I have no problem with changing the code to work as you 
> describe. I've only thought the way it works now is better, because 
> otherwise httpd could dump some files which it does not consider later. 
> This could be confusing as well.
> 
>>> If we presume that directory specified by SSLCACertificatePath contains
>>> only valid certificates and no other files, we could remove that
>>> validity check and just print filenames of all files in that directory.
>>
>> The files in that directory are accessed via their OpenSSL subject name
>> hashes, i.e. OpenSSL will look for file names like 5e5a5bcb.0 etc. It's
>> therefore rather misleading if -DDUMP_CA_CERTS dumps all files in a
>> directory, only based on whether their contents include at least one PEM
>> block with BEGIN/END CERTIFICATE.
> 
> Hm, I think when I was testing that in December it definitely worked 
> with different filenames too. I will give it a try again, but if it's 
> like that, my current code is wrong and needs change definitely.

For SSLCACertificatePath, it's a twofold thing: a) what is happening at
startup and b) what is happening at request time.

For a), it's the code in ssl_engine_init.c which is pertinent:
ssl_init_FindCAList() collects a list of all CA subject DNs, with the
help of OpenSSL's SSL_load_client_CA_file(). mod_ssl will iterate over
all files in the directory itself, so the file name does not matter in
this case. (Note, however, that SSL_load_client_CA_file will only read
in the subject DN of a certificate, nothing more.)

For b), it's OpenSSL's which decides at request time what CA certificate
file to load - see the paragraph about CApath in the
SSL_CTX_load_verify_locations(3) man page. If OpenSSL can't load the
file via its subject name hash, verification will fail - irrespective of
whether the CA certificate has expired or not. As an additional caveat,
note that the subject name hashing code changed from 0.9.8
to 1.0.0 (see [1]), so this may further complicate things.

> The problem I'm trying to solve is that there are admins with websites 
> which requires the users accessing the website to have client 
> certificates signed by certain CAs. There was an incident where a CA 
> re-issued their signing cert because the old one was expiring. The admin 
> didn't know this, so when the old certificate expired, all users whose 
> certificates were signed by that CA could no longer get in because they 
> had not updated the cert for that CA.
> 
> If httpd had a "-DDUMP_CA_CERTS" option, they could use certwatch or 
> similar tool to be warned before CA cert expires.

I see, but I think you can't really solve it by having mod_ssl enumerate
files which contain a BEGIN/END CERTIFICATE block. What I would
recommend instead is that mod_ssl only dumps the directory names, and
that you add some glue to certwatch (or certwatch.cron, more precisely)
to iterate over the the {hash}.{n} files. (As an aside, I think that
certwatch will currently only process the first certificate in a file
configured with SSLCACertificateFile, unless I'm completely misreading
pemutil.c:EMUTIL_PEM_read_X509.)

Kaspar

[1]
https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=c2c99e2860566044b23a5b3fded6f70b7436b9ad

Re: svn commit: r1550060 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c

[Jan Kaluža <jk...@redhat.com>]

On 01/05/2014 11:10 AM, Kaspar Brand wrote:
> On 02.01.2014 07:49, Jan Kaluza wrote:
>> At first sorry for late response, I was away during Christmas time. The
>> idea was to stay consistent with what DUMP_CERTS does, so DUMP_CA_CERTS
>> prints only filename of the cert so it can be later passed to certwatch.
>>
>> I'm trying to load the certificate to find out if it's valid. I think
>> this has to be done, because you can use SSLCACertificatePath to set
>> path to directory containing CA certificates and to print really only
>> valid certificates from this directory, we have to actually try to load
>> them and ignore those which can't be loaded.
>
> Checking for file contents shouldn't happen at this place, I think. As
> the comments in the sources say, ssl_hook_ConfigTest is supposed to
> "Dump the filenames of all configured ... certificates to stdout".
> Suppressing a file name if the file doesn't include a PEM block with a
> certificate makes the output rather confusing, IMO.

I think I have no problem with changing the code to work as you 
describe. I've only thought the way it works now is better, because 
otherwise httpd could dump some files which it does not consider later. 
This could be confusing as well.

>> If we presume that directory specified by SSLCACertificatePath contains
>> only valid certificates and no other files, we could remove that
>> validity check and just print filenames of all files in that directory.
>
> The files in that directory are accessed via their OpenSSL subject name
> hashes, i.e. OpenSSL will look for file names like 5e5a5bcb.0 etc. It's
> therefore rather misleading if -DDUMP_CA_CERTS dumps all files in a
> directory, only based on whether their contents include at least one PEM
> block with BEGIN/END CERTIFICATE.

Hm, I think when I was testing that in December it definitely worked 
with different filenames too. I will give it a try again, but if it's 
like that, my current code is wrong and needs change definitely.

> Maybe it would help if you could come up with a short description of
> what problem you're trying to solve (apparently it's driven by RFEs for
> certwatch, from what I understand, but it's not clear to me what exactly
> you're trying to achieve).

The problem I'm trying to solve is that there are admins with websites 
which requires the users accessing the website to have client 
certificates signed by certain CAs. There was an incident where a CA 
re-issued their signing cert because the old one was expiring. The admin 
didn't know this, so when the old certificate expired, all users whose 
certificates were signed by that CA could no longer get in because they 
had not updated the cert for that CA.

If httpd had a "-DDUMP_CA_CERTS" option, they could use certwatch or 
similar tool to be warned before CA cert expires.

> Kaspar
>

Regards,
Jan Kaluza

Re: svn commit: r1550060 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c

[Kaspar Brand <ht...@velox.ch>]

On 02.01.2014 07:49, Jan Kaluza wrote:
> At first sorry for late response, I was away during Christmas time. The 
> idea was to stay consistent with what DUMP_CERTS does, so DUMP_CA_CERTS 
> prints only filename of the cert so it can be later passed to certwatch.
> 
> I'm trying to load the certificate to find out if it's valid. I think 
> this has to be done, because you can use SSLCACertificatePath to set 
> path to directory containing CA certificates and to print really only 
> valid certificates from this directory, we have to actually try to load 
> them and ignore those which can't be loaded.

Checking for file contents shouldn't happen at this place, I think. As
the comments in the sources say, ssl_hook_ConfigTest is supposed to
"Dump the filenames of all configured ... certificates to stdout".
Suppressing a file name if the file doesn't include a PEM block with a
certificate makes the output rather confusing, IMO.

> If we presume that directory specified by SSLCACertificatePath contains 
> only valid certificates and no other files, we could remove that 
> validity check and just print filenames of all files in that directory.

The files in that directory are accessed via their OpenSSL subject name
hashes, i.e. OpenSSL will look for file names like 5e5a5bcb.0 etc. It's
therefore rather misleading if -DDUMP_CA_CERTS dumps all files in a
directory, only based on whether their contents include at least one PEM
block with BEGIN/END CERTIFICATE.

Maybe it would help if you could come up with a short description of
what problem you're trying to solve (apparently it's driven by RFEs for
certwatch, from what I understand, but it's not clear to me what exactly
you're trying to achieve).

Kaspar