Hello Remi,
On 2/24/2015 4:25 PM, Remi Gacogne wrote:
On 02/24/2015 03:17 PM, Nenad Merdanovic wrote:
This patchset adds support to configure TLS ticket keys used for
encryption and decryption of TLS tickets.
Hi Nenad,
I find your patch very interesting and I have some questions about it.
Is there a reason why it requires the number of active ticket keys to be
exactly TLS_TICKETS_NO? I understand you don't want to retain a large
number of active keys if you just keep appending keys to the file, but
perhaps you could allow a smaller number of keys, provided that there is
at least one?
TLS_TICKETS_NO is a build time option, so you can set it to whatever you
want. The idea which I discussed with Willy is to build an interface to
be able to update the keys via the socket so we don't even have to
reload in the future.
Don't you think that the documentation should mention that the file
containing the TLS ticket keys should never be written to disk, but kept
in RAM (using tmpfs for example) and prevented to be swapped to disk, as
otherwise it would destroy any hope of forward secrecy?
Sure, thanks for the idea.
I believe you may want to use EVP_MAX_IV_LENGTH instead of 16 as the
second parameter to RAND_pseudo_bytes(), as it may not be enough in the
future for larger ciphers.
Yup, will change this.
I know nobody is doing it, but I believe the return value of
RAND_pseudo_bytes() should be checked and -1 should be returned by
ssl_tlsext_ticket_key_cb() if it is != 1. Same thing for
EVP_EncryptInit_ex() and EVP_DecryptInit_ex().
Yeah, I guess it makes sense to do it for RAND_pseudo_bytes() although
it shouldn't fail. Even the reference implementation doesn't do it for
EVP_*, but I guess there is no harm and could save us in the future.
Thank you!
Regards,
Nenad
