On 17/10/13 16:18, Maxim Dounin wrote:
<snip>
>> Issues I'm aware of with this patch:
>>
>> - It doesn't check that each of the certs has a different key type
>> (but perhaps it should). If you specify multiple certs with the
>> same algorithm, all but the last one will be ignored.
>
> Bad, but we could live with it if there will be no better way to
> do things.

We could call EVP_PKEY_type() for each private key, and throw an error
if there's >1 key of any particular type (EVP_PKEY_RSA, EVP_PKEY_DSA,
EVP_PKEY_EC).

But I think I'd prefer to "live with it". Future OpenSSL versions might
add new key types, or might make it possible for a server to offer
multiple certs of the same type (e.g. 2 ECC certs with different
curves). It would be nice if Nginx could support such enhancements
automatically.

>> - The certs and keys need to be specified in the correct order.
>> If you specify "my_rsa.crt my_ecc.crt" and "my_ecc.key my_rsa.key",
>> Nginx will start but it won't be able to complete any SSL
>> handshakes. This could be improved.
>
> This is certainly not something acceptable. There should be a
> better way to specify certs and keys.

Patch v2 (attached) checks that each cert and key are correctly paired.
If they're incorrectly paired, Nginx will throw an error and refuse to
start.

>> - It doesn't add the new feature to mail_ssl_module. Perhaps it should.
>
> This could wait.

Implemented in patch v2. Untested, but the changes are virtually
identical to the http_ssl_module changes.

>> - The changes I made to ngx_conf_set_str_array_slot() work for me,
>> but do they break anything?
>
> It doesn't look like changes we want. The
> ngx_conf_set_str_array_slot() function is intended to handle
> arrays like this:
>
> example_array_directive value1;
> example_array_directive value2;

Thanks for that tip. :-)

> I would rather see ssl_certificates to be used this way, something
> like:
>
> ssl_certificate rsa.crt;
> ssl_certificate_key rsa.key;
>
> ssl_certificate ecc.crt;
> ssl_certificate_key ecc.key;

I agree. Fixed in patch v2.

>> - An RSA cert and an ECC cert might well be issued by different
>> CAs. On Apache httpd, you have to use SSLCACertificatePath to
>> persuade OpenSSL to send different Intermediate certs for each one.
>> Nginx doesn't currently have an equivalent directive, and Maxim has
>> previously said it's unlikely to be added [1].
>> I haven't researched this properly yet, but I think it might be
>> possible to do "certificate path" in memory (i.e. without syscalls
>> and disk access on each certificate check) using the OpenSSL
>> X509_LOOKUP API.
>
> AFAIR, OpenSSL only able to store one certificate chain per
> SSL_CTX, which is the root cause of the problem.

I think you're right. I've not tried to address this yet.

>> - I expect Maxim will have other comments. :-)
>
> One thing which instantly comes in mind is SSL Stapling related
> issues.

Ah yes. The Nginx stapling code seems to assume one cert and therefore
one OCSP Response. So, I think it needs updating to handle multiple
certs and OCSP Responses and to call SSL_get_certificate(SSL*) to get
the cert that the server has selected to send to the client. I've not
tried to address this yet.

--
Rob Stradling
Senior Research & Development Scientist
COMODO - Creating Trust Online
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel