Add set_ticket_key_callback (SSL_CTX_set_tlsext_ticket_key_cb)

[toidiu]

Add a wrapper for the SSL_CTX_set_tlsext_ticket_key_cb, which allows consumers to configure the EVP_CIPHER_CTX and HMAC_CTX used for encrypting/decrypting session tickets.

See https://docs.openssl.org/1.0.2/man3/SSL_CTX_set_tlsext_ticket_key_cb/ for more details.

[kornelski]

key_name is uninitialized, so it can't be &mut [u8; 16]

[kornelski]

I've changed the code to use MaybeUninit instead of raw pointers where possible.

[toidiu]

Confirming my understanding: panic on assert seems appropriate because otherwise we have UB.

[kornelski]

References must never be null, so they can't be made from a null pointer.

This assert isn't strictly necessary as long as the function is always given non-null pointers.

I thought it's better to be extra careful about this, I don't feel strongly about this choice. It could have been a Result (assuming null is a real possibility), or downgraded to debug_assert if we trust that we'll never get a null pointer in these functions.

[kornelski]

I don't understand this safety requirement. It's not clear what "initialized" here refers to, since this is an init function doing initialization.

[toidiu]

I was conflating documentation between this and session resumption. Removed and removed the unsafe.

[kornelski]

again not clear what "initialized" is

[toidiu]

I was conflating documentation between this and session resumption. Removed and removed the unsafe.

[kornelski]

Only length of the key needs to be checked, right? The iv is an array of max len.

Is there a cheap way to check key length against the Cipher to make it a safe function?

In raw_ticket_key the key is assumed to be 16 bytes long. Why is one place fixed, and another is flexible?

[toidiu]

Addressed in 2d5be20

Only length of the key needs to be checked, right? The iv is an array of max len.
Is there a cheap way to check key length against the Cipher to make it a safe function?

Yea good point. We can do a key.len() != cipher.key_len() and return early. But not sure if ErrorStack::get() is correct error to return here.

In raw_ticket_key the key is assumed to be 16 bytes long. Why is one place fixed, and another is flexible?

raw_ticket_key is dealing with key_name, which is guaranteed to be 16 bytes (docs).

Here, the application is calling EVP_EncryptInit_ex with the correct key and iv, the sizes of which depend on the chosen cipher.

I also expanded the tests to use the iv and key_name values from the callback.

[kornelski]

It's nicer with safe wrapper types. I've changed to use ForeignTypeRef to avoid ManuallyDrop.

Generally looks good.

[toidiu]

It's nicer with safe wrapper types.

Yea, much nicer not to use the ffi functions, I was sold when I got rid of the ret == 1 check.

I've changed to use ForeignTypeRef to avoid ManuallyDrop.

Sweet. I actually had to debug a panic in tests before adding the ManuallyDrop so this seems like the correct choice.

[kornelski]

ErrorStack::get() is for cases when FFI reports an error. I'll fix it in a separate PR.