Re: What is the REALLY proper way to use an ENGINE?
On Tue, Jan 04, 2011, Andrey Kulikov wrote: Thanks for a explanations. Let's consider following main, using ccgost engine: main(){ OPENSSL_config(NULL); ENGINE *e = ENGINE_by_id(gost); ENGINE_init(e); ENGINE_free(e); ENGINE_set_default(e, ENGINE_METHOD_ALL); OpenSSL_add_all_algorithms(); // emulating ENGINE_load_private_key() EVP_PKEY *pkey = EVP_PKEY_new(); pkey-ameth = ENGINE_get_pkey_asn1_meth(e, NID_id_GostR3410_2001); bla-bla-bla... //end emulating EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new(pkey, e); } Result ctx is always NULL. It happens because ENGINE_get_pkey_asn1_meth() returns pointer to corrupted internall ccgost structure ameth_GostR3410_2001. It IS initialized, then being freed. And contains garbage. Specifically pkey-ameth-pkey_id contains some random value instead of 811 (NID_id_GostR3410_2001). Is this code contain some error or invalid engine API usage? I can't see how that would happen: ENGINE_by_id() should up the structural reference. ENGINE_init() should up the functional and structural reference. ENGINE_free() should decrement the structural reference. ENGINE_set_default() should increment both structural and functional references by an amount which depends on the number of default algorithms added (which might be zero for some ENGINEs). ENGINE_load_private_key() should be passed a valid functional reference. A correct implementation of an ENGINE private key function should call EVP_PKEY_assign which will also up functional and structural reference counts. Steve. -- Dr Stephen N. Henson. OpenSSL project core developer. Commercial tech support now available see: http://www.openssl.org __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: What is the REALLY proper way to use an ENGINE?
On 4 January 2011 16:54, Dr. Stephen Henson st...@openssl.org wrote: I can't see how that would happen: I find a way to reproduce it: just call ENGINE_load_builtin_engines() twice. Or call ENGINE_load_builtin_engines() after OPENSSL_config() which calls it too. It happens because internal ccgost AMETH structures are static (which, obviously, does matter). First call of ENGINE_load_builtin_engines() creates new ENGINE structure and store it in some list, etc. Second call to ENGINE_load_builtin_engines() creates new engine structure AGAIN, but AMETH part of it reffers to the same static variable (ameth_GostR3410_2001). But ENGINE_add() in second call fails, because engine with the same ID already in the list. So, reference count not increased, and just created engine released in the very next line. AMETH structures being destoryed, and it makes them invalid for originally created engine too (because there is only one such structure). Ways to avoid it: 1. Make ccgost engine allocates AMETH structures dynamically. As there is no other engines, used this functionality (calls to ENGINE_set_pkey_asn1_meths() ), I can't compare. But from my point of view this is a bad idea, because it introduces unnecessary complexity. And, there is no clear reason to create them more that once. Of cource we can imagine some engine which provides different NID_id's depend on configuration. But ccgost is not the one. 2. Make ENGINE_load_builtin_engines() single-callable. Just like OPENSSL_config(). I can't see any reason why it have to be called more that once. Would be really appreciated if guru comment on that. Andrey.
What is the REALLY proper way to use an ENGINE?
If we take a look at any ENGINE_load_XXX function, we find that they all has similar structure: ENGINE *toadd = engine_XXX(); if(!toadd) return; ENGINE_add(toadd); ENGINE_free(toadd); ERR_clear_error(); My question is: why we need call ENGINE_free(toadd) ?? Somewhere inside it calls EVP_PKEY_asn1_free(), which besides everything else desroy all AMETH structures, created during engine initialization via EVP_PKEY_asn1_set_* . So, let's consider following example: CRYPTO_malloc_init(); ERR_load_crypto_strings(); ENGINE_load_builtin_engines(); e = ENGINE_by_id(XXX); ENGINE_set_default(e, ENGINE_METHOD_ALL); OpenSSL_add_all_algorithms(); OpenSSL_add_ssl_algorithms(); EVP_PKEY *signing_key = ENGINE_load_private_key(e, NULL, NULL, NULL); // bla-bla-bla... And, if in engine load priv func we write something like EVP_PKEY *pkey = EVP_PKEY_new(); pkey-ameth = ENGINE_get_pkey_asn1_meth(eng, NID_id_SOME_NID); it's not gonna work, because all ASN1 structures, carefully created by engine via calls to EVP_PKEY_asn1_set_* are invalid and possibly corrupted at this moment. Is it a design issue, or provided example is invalid (but it taken from openssl sources, hm...)?
Re: What is the REALLY proper way to use an ENGINE?
Update: adding ENGINE_init(e) after e = ENGINE_by_id(XXX); doesn't make any difference, as in my case functional reference count is 8(???) at the moment of ENGINE_init(e) call, so engine is not re-initialised. :( On 4 January 2011 04:12, Andrey Kulikov amde...@gmail.com wrote: If we take a look at any ENGINE_load_XXX function, we find that they all has similar structure: ENGINE *toadd = engine_XXX(); if(!toadd) return; ENGINE_add(toadd); ENGINE_free(toadd); ERR_clear_error(); My question is: why we need call ENGINE_free(toadd) ?? Somewhere inside it calls EVP_PKEY_asn1_free(), which besides everything else desroy all AMETH structures, created during engine initialization via EVP_PKEY_asn1_set_* . So, let's consider following example: CRYPTO_malloc_init(); ERR_load_crypto_strings(); ENGINE_load_builtin_engines(); e = ENGINE_by_id(XXX); ENGINE_set_default(e, ENGINE_METHOD_ALL); OpenSSL_add_all_algorithms(); OpenSSL_add_ssl_algorithms(); EVP_PKEY *signing_key = ENGINE_load_private_key(e, NULL, NULL, NULL); // bla-bla-bla... And, if in engine load priv func we write something like EVP_PKEY *pkey = EVP_PKEY_new(); pkey-ameth = ENGINE_get_pkey_asn1_meth(eng, NID_id_SOME_NID); it's not gonna work, because all ASN1 structures, carefully created by engine via calls to EVP_PKEY_asn1_set_* are invalid and possibly corrupted at this moment. Is it a design issue, or provided example is invalid (but it taken from openssl sources, hm...)?
Re: What is the REALLY proper way to use an ENGINE?
On Tue, Jan 04, 2011, Andrey Kulikov wrote: If we take a look at any ENGINE_load_XXX function, we find that they all has similar structure: ENGINE *toadd = engine_XXX(); if(!toadd) return; ENGINE_add(toadd); ENGINE_free(toadd); ERR_clear_error(); My question is: why we need call ENGINE_free(toadd) ?? To avoid a memory leak. When you call engine_XXX() you get a reference to the ENGINE. When it is added to the ENGINE list the reference count is incremented. When you call ENGINE_free() the count is decremented and the ENGINE is freed only if the reference count is zero. Steve. -- Dr Stephen N. Henson. OpenSSL project core developer. Commercial tech support now available see: http://www.openssl.org __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: What is the REALLY proper way to use an ENGINE?
Thanks for a explanations. Let's consider following main, using ccgost engine: main(){ OPENSSL_config(NULL); ENGINE *e = ENGINE_by_id(gost); ENGINE_init(e); ENGINE_free(e); ENGINE_set_default(e, ENGINE_METHOD_ALL); OpenSSL_add_all_algorithms(); // emulating ENGINE_load_private_key() EVP_PKEY *pkey = EVP_PKEY_new(); pkey-ameth = ENGINE_get_pkey_asn1_meth(e, NID_id_GostR3410_2001); bla-bla-bla... //end emulating EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new(pkey, e); } Result ctx is always NULL. It happens because ENGINE_get_pkey_asn1_meth() returns pointer to corrupted internall ccgost structure ameth_GostR3410_2001. It IS initialized, then being freed. And contains garbage. Specifically pkey-ameth-pkey_id contains some random value instead of 811 (NID_id_GostR3410_2001). Is this code contain some error or invalid engine API usage? On 4 January 2011 06:23, Dr. Stephen Henson st...@openssl.org wrote: On Tue, Jan 04, 2011, Andrey Kulikov wrote: If we take a look at any ENGINE_load_XXX function, we find that they all has similar structure: ENGINE *toadd = engine_XXX(); if(!toadd) return; ENGINE_add(toadd); ENGINE_free(toadd); ERR_clear_error(); My question is: why we need call ENGINE_free(toadd) ?? To avoid a memory leak. When you call engine_XXX() you get a reference to the ENGINE. When it is added to the ENGINE list the reference count is incremented. When you call ENGINE_free() the count is decremented and the ENGINE is freed only if the reference count is zero. Steve.