Re: [Opensc-devel] ECDSA, engine_pkcs11, libp11 and OpenSSL

[Alon Bar-L. <alo...@gm...>]

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? Let's say I want to add flags
to existing non-allocated method?

+void ECDSA_METHOD_set_flags(ECDSA_METHOD *ecdsa_method, int flags)
+       {
+       ecdsa_method->flags = flags | ECDSA_METHOD_FLAG_ALLOCATED;
+       }

Also:

When is free expected to be called? I would expect that for
EC_KEY_free or similar.
Even if we add finish method, do you expect it to call free using its
own reference?

+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.

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