Menu
▾
▴
Re: [Opensc-devel] ECDSA, engine_pkcs11, libp11 and OpenSSL
|
From: Douglas E. E. <dee...@an...> - 2013-09-21 19:21:18
|
On 9/21/2013 10:29 AM, Alon Bar-Lev wrote: > On Fri, Sep 20, 2013 at 7:08 PM, Alon Bar-Lev <alo...@gm...> wrote: >> On Fri, Sep 20, 2013 at 4:09 PM, Douglas E. Engert <dee...@an...> wrote: >>> >>> >>> On 9/20/2013 1:19 AM, Alon Bar-Lev wrote: >>>> >>>> On Fri, Sep 20, 2013 at 12:51 AM, Douglas E. Engert <dee...@an...> >>>> wrote: >>>>> >>>>> >>>>> >>>>> On 9/19/2013 4:02 PM, Alon Bar-Lev wrote: >>>>>> >>>>>> >>>>>> On Thu, Sep 19, 2013 at 11:31 PM, Douglas E. Engert <dee...@an...> >>>>>> wrote: >>>>>>> >>>>>>> >>>>>>> Modifications to engine_pkcs11 and libp11 to support ECDSA >>>>>>> are available at github for testing, and I am looking for >>>>>>> comments. >>>>>>> >>>>>>> https://github.com/dengert/libp11 >>>>>>> >>>>>>> https://github.com/dengert/engine_pkcs11 >>>>>>> >>>>>> >>>>>> Hi, >>>>>> >>>>>> This is great, I also recently updated pkcs11-helper[1] to support >>>>>> ecdsa as well. >>>>>> >>>>>> What I am missing in the new solution[2] is finish method as in other >>>>>> methods, this will allow cleanup method instance resources. >>>>> >>>>> >>>>> >>>>> Yes, OpenSSL had the init and finish with "#if 0". >>>>> and yes they may be needed, I think that is why they left them in the >>>>> code, but commented out, and why they do not want to expose >>>>> the ECDSA_METHOD structure. >>>> >>>> >>>> Right, you have an opened channel with openssl developers, can you >>>> please ask them to add the finish to the interface? >>> >>> >>> I got my foot in the door, I will see what I can do. >>> Do you have some example of what you need to do during finish? >>> I need to make an argument that the finish is needed. >>> >>> I believe the RSA finish is called when a key is freed. >>> There is an engine finish that could also be used. >> >> Engine finish does not get a reference to key when released and having >> engine is not always required. >> >> What I used so far is method override that handles multiple instances >> of keys, attaching the key specific data to the key. Then when finish >> is called release the key specific data. >> >> Reference implementation[1] >> >> I can probably create new instance of engine for each instance of key, >> but I do think that adding that missing finish in ec method to match >> other methods is the correct solution. >> >> [1] https://github.com/alonbl/pkcs11-helper/blob/ec/lib/pkcs11h-openssl.c#L375 > > Also... reading[1]. > > What is the reason of forcing allocated? The OpenSSL developers asked for a ECDSA_METHOD_new and they wanted a flag that said it was allocated, so the ECDSA_METHOD_free would not try to free a static structure. The default method structures are all currently static. And since the definition of the structure is held in the internal ecs_locl.h, only engines built withing the OpenSSL souce can have static method structures. Just doing what they asked... Let's say I want to add flags > to existing non-allocated method? Normally you would not do that, since the methods in effect is static, and could be used by other routines for other keys. > > +void ECDSA_METHOD_set_flags(ECDSA_METHOD *ecdsa_method, int flags) > + { > + /* ecdsa_method->flags = flags | ECDSA_METHOD_FLAG_ALLOCATED; */ /* More like this to preserve the value of ECDSA_METHOD_FLAG_ALLOCATED */ ecdsa_method->flags = flags | (ecdsa_method->flags & ECDSA_METHOD_FLAG_ALLOCATED); > + } > > Also: > > When is free expected to be called? I would expect that for > EC_KEY_free or similar. No. The same method is normally shared with all keys using the same engine. The method does not have any key data. It may have some engine specific routines. See: https://github.com/dengert/engine_pkcs11/commit/4870dafd4ff1f586288a016af781498487427c20 The free is only called when the engine is destroyed. > Even if we add finish method, do you expect it to call free using its > own reference? No, See above code in engine, and https://github.com/dengert/libp11/blob/master/src/p11_ec.c lines 256 to 273 use new ECDSA_METHOD functions the OpenSSL developers asked for. The ECDSA_METHOD_new(NULL); is called only once when engine created, and ECDSA_METHOD_free(ops) only once when engine is freed. Lines 277 to 296 use the ecs_locl.h to create a static version, with line 285 coping the structure, and lines 286 and 287 setting the two functions needed. (On Monday, I am gint to run valgrind againts all this to see if I have memory leaks which might require a finish routine that is called when a key is freed.) Think of the method as a C++ or JAVA class, with the key being an instance of the class. And each engine being a virtual implementation of the class. (note the correct C++ or Java terms, but I hope you get the idea.) > > +void ECDSA_METHOD_free(ECDSA_METHOD *ecdsa_method) > + { > + if (ecdsa_method->flags & ECDSA_METHOD_FLAG_ALLOCATED) > + OPENSSL_free(ecdsa_method); > + } > > Solution is to never call ECDSA_METHOD_new(), but always overwrite > fields in existing method... but then the force flags is something in > the way. No, if you had a complicate openssl script with multiple engines, or where using keys from software and some keys from an engine, you might be overwriting the software function pointers with the engines functions pointer, causes all sorts of problems. > > Alon > > [1] http://git.openssl.org/gitweb/?p=openssl.git;a=commit;h=94c2f77a62be7079ab1893ab14b18a30157c4532 > >>> >>>> >>>>> >>>>>> >>>>>> I am far from being openssl expert, but I did expect to see these >>>>>> do_sign and sign_setup to accept ecdsa as parameter and not ec... >>>>> >>>>> >>>>> >>>>> Since both ECDSA and ECDH methods can use EC keys, they split up >>>>> some of that. Look at the ecdsa_check() >>>>> "checks whether ECKEY->meth_data is a pointer to a ECDSA_DATA >>>>> structure" >>>>> That then points to the ECDSA_METHOD. >>>>> and the ecdh_check() >>>> >>>> >>>> Yes, I saw that, but still, to be consistent and remove this check for >>>> all functions they could have just have two set of "keys", as once EC >>>> set with DSA or DH it cannot be used for the other. >>> >>> >>> Not really. A key is a key, you can use it with either. A prime >>> example: an EC key on the smart card that is intended for key derivation, >>> (ECDH) needs a matching certificate. The certificate request needs >>> to be signed using ECDSA. That is how I have been testing >>> the engine to sign a cert request. >> >> As far as I can see the EC_KEY can be initialized for one at a time, >> and it uses casting of method to reference the right methods... so >> these are really two wrapped in one. >> >>> >>>> >>>>> It might be the ecdsa_data_st, need to have the finish It has an init. >>>>> the inti or finish might depend on how the key is used last. >>>>> >>>>> I don't thing the OpenSSL developers have a good handle on what >>>>> the engine might need. >>>> >>>> >>>> Right... but I will be very happy to see that finish :) >>>> >>>> Thanks!!! >>>> >>>>> >>>>> My ecdsa code may also have memory leaks too... >>>>> >>>>> >>>>>> >>>>>> Regards, >>>>>> Alon >>>>>> >>>>>> [1] https://github.com/alonbl/pkcs11-helper/commits/ec >>>>>> [2] >>>>>> >>>>>> http://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=HEAD;hp=96006022671b4db342a4dcfc3d96edbb3337bb4e >>>>>> >>>>> >>>>> -- >>>>> >>>>> Douglas E. Engert <DEE...@an...> >>>>> Argonne National Laboratory >>>>> 9700 South Cass Avenue >>>>> Argonne, Illinois 60439 >>>>> (630) 252-5444 >>>> >>>> >>> >>> -- >>> >>> Douglas E. Engert <DEE...@an...> >>> Argonne National Laboratory >>> 9700 South Cass Avenue >>> Argonne, Illinois 60439 >>> (630) 252-5444 > . > -- Douglas E. Engert <DEE...@an...> Argonne National Laboratory 9700 South Cass Avenue Argonne, Illinois 60439 (630) 252-5444 |
