Crypto-Plugin: ref counting for library initialization (WIP) #419
Conversation
…reaks test case testscr_check_kdb_internal_check
self->getName() returns a new temporary std::string, and passing its internal buffer as return value of __tostring means using free'd memory. fixes ElektraInitiative#413
We're passing enums and enums are limited to the size of an int (C99 6.7.2.2) Fixes ElektraInitiative#414
|
Thank you! I think the abstraction can be improved: the ref counting schema is needed for openssl, but for others there might be a better solution (atomic refcounting already as part of a crypto-API). So why not put the refcounting to the gcrypt and openssl provider of |
|
Btw. a way to avoid init/cleanup might be needed to. (e.g. If application already configured openssl) |
petermax2
changed the title
|
gcrypt does not seem to have any ref counting built into its public API. They might be using it internally (I read some old discussions about it in developer forums). Libssl uses ref counting internally but they don't seem to make it public either. Libcrypto does not use any ref counting at all but provides a lock mechanism through its API. [1, 2] However this is the part of the API that we try to initialize here. ;) [1] The Libgcrypt Reference Manual |
Crypto-Plugin: ref counting for library initialization (WIP)
|
Ok, then thank you for the PR! Some plugin options and maybe also as metadata in kdbOpen()/kdbClose() to enable/disable tearup/down should be added later, too. (and the check if gcrypt has been initialized) I merged it, because it was a nicely coupled unit, please create a new PR then. |
|
Thank you! OK, I noted the metadata check. Checking if gcrypt has already been set up is not supported by the API. In general the manual suggests to leave all of the initialization stuff to the application developer. So OpenSSL will make some things easier for us. |
|
An additional remark to the internal ref counting in gcrypt/libssl: If they do it correctly, it should not matter if you reopen/reclose it as long as the number of open and close matches? What happens if the crypto libs have multiple init/teardown? Did you test it? For plugin configuration we should,however, agree to a naming convention in #418 first. |
|
For now it does not matter if the setup routines have been called multiple times. I tested it for both Gcrypt and libcrypto. That may change in the future. The gcrypt folks are planning on dropping the initialization alltogether in the future but they didn't make it clear when that will be. |
|
That sounds good! Did you also test the case with complete shutdown and restart? That caused problems in python. So the ref counting introduced in this PR is actually not needed? |
|
gcrypt does not have to shutdown, I'm not sure about OpenSSL yet. The one thing I did not test yet is how these libraries behave when multiple threads are running and the library gets re-initialized in between operations. OpenSSL shouldn't have any problems as they internally just update a function pointer. The thing with ref counting is: I want to comply to the guidelines and manuals of the crypto APIs as much as possible. And they all suggest to initializing the libraries as early as possible and call these routines only once. So maybe in a future update something breaks if we call the setup routines multiple times. Also if libgcrypt starts in FIPS-enabled mode the library does additional setup, that is supposed to run only once in the lifetime of the application. The same is true for libcrypto. I think the self tests in FIPS-mode may throw errors if executed multiple times. It all depends on versions and compile/startup options and we can not be careful enough. |
|
Ok, thank you! So better we have the ref-counting in place! |
I prepared the library initialization using a simple ref counter (protected by a mutex) as preparation for the planned OpenSSL support.