API contract for init() expects re-initing

[nils-ohlmeier]

It surprised me to learn the internal self test calls init() several times and appears to expect that the underlying crypto library get re-initialized with the new key.

From other APIs I would expect the when calling an init function a second time it either:

• after checking if init was called at least once already return right away
• return an error on calling init more then once

Is this really the intended behavior, or should the self tests be changed (if possible)?

[pabuhler]

@nils-ohlmeier, in this context I take it you are talking about the srtp_cipher_type_t::init function and the two cases of "re-initialize cipher for decryption"?

Judging by the comments in the code this is the intended behavior and is somewhat similar to the EVP api of openssl. It is also "documented" in cipher.h

/*
 * a srtp_cipher_init_func_t [re-]initializes a cipher_t with a given key
 */

and is public through srtp_replace_cipher_type().

Changing the self test code to use two separate cipher objects should be easy enough but first we would need to document the API better.
If you think this should be changed then we can keep it open as an enhancement request for a future 3.0 release?

[nils-ohlmeier]

Yes I meant the srtp_cipher_type_t::init function. Since it caused a memory leak with the NSS implementation I'm not the only one who was surprised by this re-initing expectation. But I missed that it is mentioned in the comments.

And I did not expected an immediate change. I only raised it in case people where not aware of this behavior, as it seems the other existing implementations don't do dynamic allocations inside the init function.

If this is desired and documented behavior I'm fine with leaving it like it is.

[pabuhler]

"desired" is maybe overstating it, I am unaware of why it is like it is, but it is documented. We can leave it open as a potential enhancement, and revisit this with next major release.