opensc-devel — discussion of developement of OpenSC and related projects

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

[Markus K. <ko...@rr...>]

On 09/22/2013 10:43 AM, Markus Kötter wrote:
> libp11 leaking the ERR strings when unloading the engine:
>
> https://github.com/commonism/libp11/commit/46dacbe8f5badde89d25289faab82232311822b4
> https://github.com/commonism/engine_pkcs11/commit/67244a1cef3decc5b896be5adb9dd771262ab37a
>
> engine_pkcs11 free's the mallocs claimed by PKCS11_ structures.
> https://github.com/commonism/engine_pkcs11/commit/fbae0727e88fd20e1cba6ec60799dc4fe705cf97
>

Combined savings when ...
loading 100 keys

before:
==12577== LEAK SUMMARY:
==12577==    definitely lost: 10,409 bytes in 112 blocks
==12577==    indirectly lost: 705,606 bytes in 14,710 blocks
==12577==      possibly lost: 0 bytes in 0 blocks
==12577==    still reachable: 1,684 bytes in 4 blocks
==12577==         suppressed: 0 bytes in 0 blocks

after:
==11916== LEAK SUMMARY:
==11916==    definitely lost: 7,209 bytes in 112 blocks
==11916==    indirectly lost: 406 bytes in 10 blocks
==11916==      possibly lost: 0 bytes in 0 blocks
==11916==    still reachable: 1,684 bytes in 4 blocks
==11916==         suppressed: 0 bytes in 0 blocks


loading 1000 keys

before:
==12675== LEAK SUMMARY:
==12675==    definitely lost: 82,329 bytes in 1,011 blocks
==12675==    indirectly lost: 7,044,290 bytes in 146,860 blocks
==12675==      possibly lost: 8,196 bytes in 151 blocks
==12675==    still reachable: 1,684 bytes in 4 blocks
==12675==         suppressed: 0 bytes in 0 blocks

after:
==13261== LEAK SUMMARY:
==13261==    definitely lost: 50,409 bytes in 1,012 blocks
==13261==    indirectly lost: 406 bytes in 10 blocks
==13261==      possibly lost: 0 bytes in 0 blocks
==13261==    still reachable: 1,684 bytes in 4 blocks
==13261==         suppressed: 0 bytes in 0 blocks

The remaining leaks are burried in OpenSC itself, and may depend on the 
card used, I used a Feitian PKI Smartcard.


MfG
Markus

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

[Markus K. <ko...@rr...>]

On 09/21/2013 11:45 PM, Markus Kötter wrote:
> And I even tried to do it, n with ECDSA but fixing the memory leaks of
> the RSA code.

I forked and created a branches.
https://github.com/commonism/libp11/commits/memoryleaks
https://github.com/commonism/engine_pkcs11/commits/memoryleaks


libp11 leaking the ERR strings when unloading the engine:

https://github.com/commonism/libp11/commit/46dacbe8f5badde89d25289faab82232311822b4
https://github.com/commonism/engine_pkcs11/commit/67244a1cef3decc5b896be5adb9dd771262ab37a

engine_pkcs11 free's the mallocs claimed by PKCS11_ structures.
https://github.com/commonism/engine_pkcs11/commit/fbae0727e88fd20e1cba6ec60799dc4fe705cf97

> Problems ...
>
> idx 0 - which is returned by default, is used by p11 already as app_data.

I have a funny workaround in place.

> ex data slots claimed with RSA_get_ex_new_index last forever.
> Unload the engine, there is no way to drop the slot, so the callback
> will be called, if the memory is unmapped as the engine is unloaded, you
> are lost.
> Reload the engine, get mapped in the same location, you get the
> callbacks for the unmapped engines as well.

Not fixable.
"Therefore - better not un-load the engine."
Currently this is broken for all dynamic engines.

> The API exposed by OpenSSLs ex_data.c is not sufficient to remove the
> slot manually.

Once OpenSSL comes up with a way to get rid of a claimed slot, I will 
update this.

> Calling PKCS11_release_all_slots with the required arguments in the ex
> data destructor of an EVP_PKEY/RSA results in recursion, as
> PKCS11_release_all_slots calls EVP_PKEY_free as well.

Fixed, remembering the claimed PKCS11_KEY and setting the evp_key NULL 
before destroying the rest prevents the recursion easily.


If we can agree this is a proper approach to free the claimed memory, 
I'll make this work for certs as well, currently only keys are taken 
care of.


MfG
Markus

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

[Markus K. <ko...@rr...>]

On 09/19/2013 10:31 PM, Douglas E. Engert wrote:
> Modifications to engine_pkcs11 and libp11 to support ECDSA
> are available at github for testing, and I am looking for
> comments.

I see the use of ECDSA_set_ex_data to associate the PKCS11_KEY with the 
EC_KEY/EVP_PKEY.
Yet, I'm not convinced this can free the resources claimed when loading 
a key.

As comparison let's have a look on the RSA code.
pkcs11_load_key
https://github.com/OpenSC/engine_pkcs11/blob/master/src/engine_pkcs11.c#L541

If the key is found, all allocated PKCS11_* structures are leaked.
For every private key, public key, certificate loaded.

This patch:
https://github.com/tkil/engine_pkcs11/commit/91133b719c71d0c92c233a0c29b0d5b94c4ee102
reduces the leak by caching the results, not fixing the real problem but 
masking it.

I'd propose to allocate an index for the data using 
RSA_get_ex_new_index, and attach all data to the RSA structure using 
RSA_set_ex_data.
The destructor provided when creating the index can be used to clean 
everything up.


The current ECDSA patch do not allocate an index, it just claims index 
0, which is the same as app_data(), collisions with other software are 
not that unlikely.
If 0 is used by something else as app_data already, you'll run into 
problems. Additionally, while index 0 is reserved for app_data, OpenSSL 
still returns it as valid entry.

The current ECDSA patch do not provide a destructor, so it is 'unlikely' 
the complete PKCS11_ structure set is free'd, most likely either just 
the PKCS11_KEY associated with the EC_KEY, or nothing at all as there is 
no destructor.

Internally EC_KEY has EC_KEY_insert_key_method_data which can be used 
with EC_EX_DATA_set_data, EC_KEY_insert_key_method_data is visible 
externally as well and could be used instead of ECDSA_get_ex_new_index.

Still, I'd use ECDSA_get_ex_new_index/ECDSA_set_ex_data to have the same 
code path as -to be- used by RSA.

And, I'd propose to have this functionality in engine_pkcs11 instead of 
p11. engine_pkcs11 allocates the PKCS11_ structures (by calling p11) and 
has to free them in time.
p11 will not even know which RSA/ECDSA to associate the PKCS11_ 
structures with, as they are allocated way before the getting to the key.
Once the key is loaded, the engine can attach all relevant data to the key.

And I even tried to do it, n with ECDSA but fixing the memory leaks of 
the RSA code.
Problems ...

idx 0 - which is returned by default, is used by p11 already as app_data.

ex data slots claimed with RSA_get_ex_new_index last forever.
Unload the engine, there is no way to drop the slot, so the callback 
will be called, if the memory is unmapped as the engine is unloaded, you 
are lost.
Reload the engine, get mapped in the same location, you get the 
callbacks for the unmapped engines as well.
The API exposed by OpenSSLs ex_data.c is not sufficient to remove the 
slot manually.

Calling PKCS11_release_all_slots with the required arguments in the ex 
data destructor of an EVP_PKEY/RSA results in recursion, as 
PKCS11_release_all_slots calls EVP_PKEY_free as well.


Ideas?


MfG
Markus Kötter

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

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

On Sat, Sep 21, 2013 at 10:21 PM, Douglas E. Engert <dee...@an...> wrote:
>
>
> 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...

I do not understand why you force this flag when
ECDSA_METHOD_set_flags, this means that this method is unusable for
static allocated method, continuing that logic you should have set
this flag also when setting method callbacks.

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

and if you see my reference implementation I create a duplicate.

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

Right, and we need a cleanup method for the key releasing any resource
associate with, just like in RSA and DSA.

>
> See:
> https://github.com/dengert/engine_pkcs11/commit/4870dafd4ff1f586288a016af781498487427c20
>
> The free is only called when the engine is destroyed.

This is one form of implementation.

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

Again, this is good when engine is used, not when method is overridden
without engine.

Also, this is good if there is new instance of engine for every key,
which is something that I do not like.

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

this is incorrect as there is no destructor (finish)...
and engine does not get the instance, unless is instantiate for every key.

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

Again, I've never seen that problems, and as far as I can see the
method is duplicated.

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

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

[Douglas E. E. <dee...@an...>]

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

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

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

[Ludovic R. <lud...@gm...>]

2013/9/20 Douglas E. Engert <dee...@an...>:
>
>
> On 9/20/2013 2:45 AM, Jean-Michel Pouré - GOOZE wrote:
>> Le jeudi 19 septembre 2013 à 15:31 -0500, Douglas E. Engert a écrit :
>>> Modifications to engine_pkcs11 and libp11 to support ECDSA
>>> are available at github for testing, and I am looking for
>>> comments.
>>
>> This is nice to have them on board.
>>
>> My only comment is that, according to rumors, Elliptic curves are
>> reported broken by NSA crypto-analysts. The reason is that Elliptic
>> curves offer more space for mathematics and are quite new, offering
>> space for discoveries in factorization.
>
> I have not heard those rumors. I have heard there are some curves,
> that should not be used.   On the contrary, there is more discussion
> about breaking RSA in the next few years and the industry better be in
> a position to have a replacement implemented, i.e. ECDSA and ECDH.

Maybe Jean-Michel is revering to this article "La NSA est suspectée
d'avoir altéré un standard cryptographique" [1] (in French) that link
to a New York Times article "Government Announces Steps to Restore
Confidence on Encryption Standards" [2].

If we can't trust NIST any more then we can move to "the other side"
by using the GHOST cryptosystems [3] from the Soviet Union.

Bye

[1] http://www.numerama.com/magazine/26979-la-nsa-est-suspectee-d-avoir-altere-un-standard-cryptographique.html
[2] http://bits.blogs.nytimes.com/2013/09/10/government-announces-steps-to-restore-confidence-on-encryption-standards/?_r=0
[3] https://en.wikipedia.org/wiki/GOST

-- 
 Dr. Ludovic Rousseau

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

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

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

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

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

[Douglas E. E. <dee...@an...>]

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.

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

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

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

[Douglas E. E. <dee...@an...>]

On 9/20/2013 2:45 AM, Jean-Michel Pouré - GOOZE wrote:
> Le jeudi 19 septembre 2013 à 15:31 -0500, Douglas E. Engert a écrit :
>> Modifications to engine_pkcs11 and libp11 to support ECDSA
>> are available at github for testing, and I am looking for
>> comments.
>
> This is nice to have them on board.
>
> My only comment is that, according to rumors, Elliptic curves are
> reported broken by NSA crypto-analysts. The reason is that Elliptic
> curves offer more space for mathematics and are quite new, offering
> space for discoveries in factorization.

I have not heard those rumors. I have heard there are some curves,
that should not be used.   On the contrary, there is more discussion
about breaking RSA in the next few years and the industry better be in
a position to have a replacement implemented, i.e. ECDSA and ECDH.

ECC is not that new it has been around for years. Its implementations
are new. RSA is wide use so interest in implementation EC has been low.

EC have an infinite number of curves, which complicated in implementation
and security. The industry appears to be settling on a small set of
named curves that can be trusted. This also makes it easier to implement.

Maybe a little out dated, but from 2009:
http://www.nsa.gov/business/programs/elliptic_curve.shtml

Implementation of EC is falling into place. The mods are a part of that,
implementing ECDSA in the engine, to supportsmart cards such as the PIV
card with ECDSA and ECDH.

I would rather have two crypto algorithms implemented, just in case.

>
> Kind regards,
>
>
>
> ------------------------------------------------------------------------------
> LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99!
> 1,500+ hours of tutorials including VisualStudio 2012, Windows 8, SharePoint
> 2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack includes
> Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13.
> http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk
>
>
>
> _______________________________________________
> Opensc-devel mailing list
> Ope...@li...
> https://lists.sourceforge.net/lists/listinfo/opensc-devel
>

-- 

  Douglas E. Engert  <DEE...@an...>
  Argonne National Laboratory
  9700 South Cass Avenue
  Argonne, Illinois  60439
  (630) 252-5444

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

[Jean-Michel P. - G. <jm...@go...>]

Le jeudi 19 septembre 2013 à 15:31 -0500, Douglas E. Engert a écrit :
> Modifications to engine_pkcs11 and libp11 to support ECDSA
> are available at github for testing, and I am looking for
> comments. 

This is nice to have them on board.

My only comment is that, according to rumors, Elliptic curves are
reported broken by NSA crypto-analysts. The reason is that Elliptic
curves offer more space for mathematics and are quite new, offering
space for discoveries in factorization.

Kind regards,
-- 
                  Jean-Michel Pouré - Gooze - http://www.gooze.eu

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

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

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

> 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

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

[Douglas E. E. <dee...@an...>]

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.

>
> 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()

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.

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

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

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

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.

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

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

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

[Douglas E. E. <dee...@an...>]

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

These can be used with OpenSC-0.13.0 and OpenSSL > 1.0.0
and have been tested using OpenSSL-1.0.1e both with and
without modifications for OpenSSL bug report #2459.
(See below.)

As interest in ECDSA and PKCS#11 has increased over the last
few months, I have sent out modifications developed in 2011
to a number of people, including Sanaullah, who reported he
has the older modifications working to allow OpenSSL to
generate certificate requests using EC keys and signed
using the EC key via PKCS#11 with softhsm.

I have tested using PIV smart cards that support EC keys.

There is also another OpenSSL modification that may be
needed if you tryn and use the OpenSSL dgst with the engine
and ECDSA. (See the attachment, and the reference
to the e-mail from 2010.)


Git commit comment for libp11 modification:

Experimental ECDSA support

Support for ECDSA is added for used with OpenSSL > 1.0.0.
OpenSSL has an outstanding bug report, #2459, that requests
the structure ecdsa_method be exposed. An engine needs to create
such a structure which internal to OpenSSL is static, which
has pointers into functions within the engine.

Engines using RSA do not have this problem, because the
rsa_method_st is exposed in rsa.h This allows an engine such
as the combination of libp11 and engine_pkcs11 to compile in
a static version of the rsa_method_st.

Modifications have been submitted to OpenSSL at the suggestion
of the OpenSSL developers to add a set of functions to build a
ecdsa_method structure and to set the needed functions into
the structure.

These libp11 modifications are designed to allow building
libp11 using either the internal OpenSSL
crypto/ecdsa/ecs_locl.h or the new ECDSA_METHOD_new
function in ecdsa.h

By default the ECDSA_METHOD_new will be used if present.

To build using the ecs_locl.h, one must have access to the
OpenSSL source and add to the libp11 build process,
-DBUILD_WITH_ECS_LOCL_H -I/<path.to.OpenSSL.source>/crypto/edcsa

(Note: that using an internal header file may require libp11
to be rebuilt to match the specific version of OpenSSL being
used, and may not work in future versions.)

Once the OpenSSL modifications for #2459 are accepted
libp11 will be changed to remove the old method.

The intent of this dual build it to allow people to use
ECDSA even if OpenSSL does not implement ECDSA_METHOD_new
or does not implement it in the near future.

-- 

   Douglas E. Engert  <DEE...@an...>
   Argonne National Laboratory
   9700 South Cass Avenue
   Argonne, Illinois  60439
   (630) 252-5444

Re: [Opensc-devel] [engine_pkcs11] Ajf fixes 201305 (#3)

[Anthony F. <ant...@gm...>]

Michael --

On Mon, Sep 16, 2013 at 10:51 AM, Anthony Foiani
<ant...@gm...> wrote:

> Having said that, I don't recall making any change to the
> functionality there, just refactoring / moving code around.  It's been
> a while, however.

At least the "use first certificate if nothing else matched" was
certainly there when I started this fork off commit
8d6264:

https://github.com/tkil/engine_pkcs11/blob/8d6264eea5fb16dcd2c8d1a9e5ca6e031f370241/src/engine_pkcs11.c#L509

And the logic above (for matching ID strings) should be the same, just
with different variable names / scopes.

It would be interesting to see if Petr fixed this logic but left the
code in the previous place.

I won't have time to deep dive on this for quite a while, I'm afraid.

Best regards,
Anthony Foiani

Re: [Opensc-devel] [engine_pkcs11] Ajf fixes 201305 (#3)

[Anthony F. <ant...@gm...>]

Michael --

Sorry to hear that the patch set is giving you issues.

As a counter-example, I have my patchset running on an embedded system
that grabs then releases the key once a minute, and it can run for
days without memory leaks; before, it was leaking about 2MB/day.

I'll admit that I retrieve my certificates directly from the token
(via libp11), so I never tested the engine interface.

Having said that, I don't recall making any change to the
functionality there, just refactoring / moving code around.  It's been
a while, however.

The main thrust of my patchset was to allow me to use private keys on
the token without leaking massive amounts of memory.  If you can find
a way to integrate the key memory management with Petr's patchset,
that would hopefully be the best of both worlds.

It's not clear to me whether this email thread is echoed to the opensc
devel list; if not, it should be, as there is at least one person
there (Douglas Engert) who is looking at integrating some of these
fixes as well as pushing through other features like EC keys.

I'm busy this afternoon, but I can try to take a look at it this
evening (North America time).

Sorry for the inconvenience.  :(

Good luck,
Tony

p.s. Apologies for top-posting, I'm a little pressed for time right now.

On Mon, Sep 16, 2013 at 7:13 AM, Michael Gebetsroither
<not...@gi...> wrote:
> hi @tkil! i just wanted to try your patches because we needed loading of
> certificates by label, but we had severe problems.
>
> Quick overview:
> 1) Broken loading of certificates, only ever giving back the first
> certificate
> 2) Loading certificates by Label is completely broken
> 3) Corrupted cert structures in function scan_certs parameters, possible
> stack corruption
>
> The whole problem showed signs of stack-corruption and segfaults without
> backtrace longer than 2-3 entries. Please take this with a grain of salt as
> we haven't exactly pinned down the culprint, except that
> https://github.com/petrpisaratlascz/engine_pkcs11 worked for days in our
> case.
>
> We used engine-pkcs11 for curl with client certificates with additional
> intermediate ca needed for it, everything loaded from the token in a long
> running process.
>
> What we have found:
>
> ad 1) Even with matching labels this patchset only ever returns the first
> certificate, not the first matching one. Code giving back a wrong
> certificate without notice.
> scan_certs:
>
> @@ static PKCS11_CERT *scan_certs(PKCS11_CERT *certs,
>                unsigned int cert_count,
>                char *id,
>                unsigned int id_len)
> /* If we couldn't find a match, just use the first one. */
> if (!rv) {
>     rv = certs;
> }
>
> ad 2) and again scan_certs called with what we suspect wrong parameters and
> wrong implementation:
> // the check for id_len not being 0 is pointless as for both call sites the
> size given is from a static buffer from stack
> // id_len is always MAX_VALUE_LEN / 2 (100 here) and the length of a buffer
> on stack in function pkcs11_load_cert/pkcs11_load_key
> // comparing c->id_len against id_len is pointless as both are static here
> and do NOT reflect the containing labels
> // c->id_len is always 20 in our case, and id_len is from a statically
> allocated buffer
> // which brings us to memcmp: this also does not work here because c->id is
> already corrupted
>
> @@ static PKCS11_CERT *scan_certs(PKCS11_CERT *certs,
>                unsigned int cert_count,
>                char *id,
>                unsigned int id_len)
>     if (id_len && c->id_len == id_len &&
>         memcmp(c->id, id, id_len) == 0) {
>         rv = c;
>         cert_num = n;
>     }
>
> The problem it seems is that all those bugs are hidden by point 1, that
> simply the first cert is returned without warning/log entry and not even a
> debug output.
>
> ad 3) Wrong function parameters usage of what the code expects, or at least
> strange things / possible corrupted stack.
> scan_certs:
> PKCS11_CERT *certs all have corrupted labels.
>
> Call stack with own debug output:
>
> pkcs11_load_cert: slot_id="label_bar"
> parse_slot_id_string_aux: slot_id="label_bar" slot_nr=-1 id="" id_len=100
> label="(null)" use="certificate"
> parse_slot_id_string: slot_id="label_bar" slot=-1 id="" id_len=100
> label="(null)"
> Looking in slot 1109423260 for certificate: label: 'bar'
> pkcs11_load_cert: cert_label="bar"
> scan_slots: Num slots: 2
>
> [18446744073709551615] Virtual hotplug slot       no token
> [1] Feitian ePass2003 00 00    login             (mytoken (User PIN))
> scan_slots: Found slot 'Feitian ePass2003 00 00', token 'mytoken (User PIN)'
> scan_certs: cert_count=2 id="" id_len=100
> scan_certs: Found 2 certificates:
> scan_certs: id_len=100 c->id_len=20 id="" c->id=")���Z2�7�K-ȹ� ��"
>    1    foofoofo (/C=XX/CN=xxxxxxxx)
> scan_certs: id_len=100 c->id_len=20 id="" c->id="����ze�� D���ܟ�"
>    2    bar (/C=XX/ST=XX/CN=XXXXXX)
> Did not find a matching certificate, using the first one
> Selecting certificate 0 ("foofoofo")
>
> —
> Reply to this email directly or view it on GitHub.

Re: [Opensc-devel] [engine_pkcs11] Various fixes and features

[Petr P. <pet...@at...>]

On Tue, Sep 10, 2013 at 10:28:55AM -0500, Douglas E. Engert wrote:
> Looks like it is time for some overhaul of engine_pkcs11...
> 
> You have some good patches here.
> 
> The OpenSC code including the engine_pkcs11 is maintained
> at https://github.com/OpenSC
> 
> The best way to get these patches added would be to fork the
>   https://github.com/OpenSC/engine_pkcs11
> add your patches to your fork, and submit pull requests.
>
Done <https://github.com/OpenSC/engine_pkcs11/pull/5>.

-- Petr

Re: [Opensc-devel] [engine_pkcs11] Various fixes and features

[Anthony F. <ant...@gm...>]

Douglas --

On Tue, Sep 10, 2013 at 9:39 AM, Douglas E. Engert <dee...@an...> wrote:
>
> On 9/10/2013 10:37 AM, Anthony Foiani wrote:
>
>> Yeah, I'm a little mystified that openssl just ignores patches sent to
>> their list.  :(
>
> #2459 does not have a patch. But since the structure that needs to be
> exposed has not changed in 2 years, I will be submitting a patch.

Ah, I was actually referring to a different issue, where I sent them a
one-line, obviously-correct patch:

http://permalink.gmane.org/gmane.comp.encryption.openssl.devel/23019

Crickets.  I just assumed that's how they responded in general.  :(

Thanks again for your work on this topic -- it's very welcome!

Best,
Anthony Foiani

Re: [Opensc-devel] [engine_pkcs11] Various fixes and features

[Douglas E. E. <dee...@an...>]

On 9/10/2013 10:37 AM, Anthony Foiani wrote:
> Douglas --
>
> On Tue, Sep 10, 2013 at 9:28 AM, Douglas E. Engert <dee...@an...> wrote:
>> Looks like it is time for some overhaul of engine_pkcs11...
>
> It'll be nice to see it get some love.
>
>> I also have mods to engine_pkcs11 for doing ECDSA, which
>> has been on the back burner since 2011. I have recently
>> been helping Sanaullah to get these working with softhsm.
>
> If the project I'm on gets an opportunity to do a version 1.1, we will
> definitely want to have the option to use ECDSA, so that would be
> great.
>
>> (I have been waiting for OpenSSL bug report #2459 02/23/2011
>> to be addressed. I might have a way to get around this,
>> or at least get the OpenSSL's attention.)
>
> Yeah, I'm a little mystified that openssl just ignores patches sent to
> their list.  :(

#2459 does not have a patch. But since the structure that needs to be
exposed has not changed in 2 years, I will be submitting a patch.

>
>> I am getting ready to submit these engine_patches,
>> and would like use both yours and Anthony's patches.
>
> You are of course welcome to take those patches at any time.  I don't
> have a lot of time, but if there are particular deficiencies, I can
> try to address them.
>
> Thanks for your efforts!
>
> Best regards,
> Anthony Foiani
>

-- 

  Douglas E. Engert  <DEE...@an...>
  Argonne National Laboratory
  9700 South Cass Avenue
  Argonne, Illinois  60439
  (630) 252-5444

Re: [Opensc-devel] [engine_pkcs11] Various fixes and features

[Anthony F. <ant...@gm...>]

Douglas --

On Tue, Sep 10, 2013 at 9:28 AM, Douglas E. Engert <dee...@an...> wrote:
> Looks like it is time for some overhaul of engine_pkcs11...

It'll be nice to see it get some love.

> I also have mods to engine_pkcs11 for doing ECDSA, which
> has been on the back burner since 2011. I have recently
> been helping Sanaullah to get these working with softhsm.

If the project I'm on gets an opportunity to do a version 1.1, we will
definitely want to have the option to use ECDSA, so that would be
great.

> (I have been waiting for OpenSSL bug report #2459 02/23/2011
> to be addressed. I might have a way to get around this,
> or at least get the OpenSSL's attention.)

Yeah, I'm a little mystified that openssl just ignores patches sent to
their list.  :(

> I am getting ready to submit these engine_patches,
> and would like use both yours and Anthony's patches.

You are of course welcome to take those patches at any time.  I don't
have a lot of time, but if there are particular deficiencies, I can
try to address them.

Thanks for your efforts!

Best regards,
Anthony Foiani

Re: [Opensc-devel] [engine_pkcs11] Various fixes and features

[Douglas E. E. <dee...@an...>]

Looks like it is time for some overhaul of engine_pkcs11...

You have some good patches here.

The OpenSC code including the engine_pkcs11 is maintained
at https://github.com/OpenSC

The best way to get these patches added would be to fork the
  https://github.com/OpenSC/engine_pkcs11
add your patches to your fork, and submit pull requests.

Anthony Foiani also has forked engine_pkcs11 and his changes
also look good too, so you may want to build with his mods
as well.

I also have mods to engine_pkcs11 for doing ECDSA, which
has been on the back burner since 2011. I have recently
been helping Sanaullah to get these working with softhsm.

(I have been waiting for OpenSSL bug report #2459 02/23/2011
to be addressed. I might have a way to get around this,
or at least get the OpenSSL's attention.)

I am getting ready to submit these engine_patches,
and would like use both yours and Anthony's patches.



On 8/30/2013 9:45 AM, Petr Písař wrote:
> Hello,
>
> while testing TLS client authentication using a cryprographical token in my
> project (libisds over cURL over OpenSSL with Athena USB token under OpenSC),
> I found a lot of bugs in the engine_pkcs11 plug-in for OpenSSL.
>
> Some of the bugs are so serious that they prevent from using the token through
> OpenSSL and can lead even to a segmentation fault. So I deciced to fix them
> and post the pathes here in hope the engine_pkcs11 maintainer will review them
> and merge them.
>
> Here is a short description, patches will be sent as replies:
>
> [PATCH 1/9] Unify PIN freeing
> [PATCH 2/9] Free PIN storage where needed
>
> These two patches fix memory leaks when storing a PIN code.
>
> [PATCH 3/9] Use user interface correctly
>
> This fixes a crash (segmenation fault) when loading a private key. Current
> code could never use a PIN passed from OpenSSL because of wrong usage of the
> user interface call-back data. I send a fix to cURL library
> <http://thread.gmane.org/gmane.comp.web.curl.library/40222> too and I tested
> the colaboration between cURL and engine_pkcs11 successfully.
>
> [PATCH 4/9] Hexadecimal ID string contains colons
>
> A certificate/key object hexadecimal ID is printed with colons (ab:cd:..)
> everywhere. Let's allow engine_pkcs11 to recognize it. Contrary current parser
> expects the colons by can not recognize such string as an ID. I believe it
> was not possible to use the hexadecimal ID before.
>
> [PATCH 5/9] Find token if no slot was specified
>
> Identifier wihout a slot number (e.g. a plain ID) always resulted to slot
> number 0. This searches all slots now.
>
> [PATCH 6/9] Search for a certificate by a label
>
> Searching a certificate by a label did not work and worked differently than
> searching a key. This caused a lot of confusion why OpenSSL can locate the key
> but it cannot locate the certificate.
>
> [PATCH 7/9] Decouple loging into the token
> [PATCH 8/9] Implement ENGINE_load_ssl_client_cert()
> [PATCH 9/9] Add load_ssl_client_cert test
>
> These tree patches implement ENGINE_load_ssl_client_cert() interface which
> allows automatic negotion of client certificate in TLS authenticatinon. The
> ninth patch provides a test.
>
> -- Petr
>
> ------------------------------------------------------------------------------
> Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
> Discover the easy way to master current and previous Microsoft technologies
> and advance your career. Get an incredible 1,500+ hours of step-by-step
> tutorial videos with LearnDevNow. Subscribe today and save!
> http://pubads.g.doubleclick.net/gampad/clk?id=58040911&iu=/4140/ostg.clktrk
> _______________________________________________
> Opensc-devel mailing list
> Ope...@li...
> https://lists.sourceforge.net/lists/listinfo/opensc-devel
>

-- 

  Douglas E. Engert  <DEE...@an...>
  Argonne National Laboratory
  9700 South Cass Avenue
  Argonne, Illinois  60439
  (630) 252-5444

Re: [Opensc-devel] pin-retry

[Douglas E. E. <dee...@an...>]

OpenSC has a number of PIN caching features. See the opensc.conf


On 9/3/2013 8:04 AM, J.W...@mi... wrote:
> Hi all
>
> I just had a discussion about pin-retry. Where is it defined?
> On one of the recent HSM-pages, I noticed it is a parameter you can (default = 3) define.
>
> But how about "regular" smart cards?
> I was expecting this to be an option at pkcs15-init..
> Or is this something specific to the underlying hardware?

OpenSC has a number of PIN caching features. See the opensc.conf
A card may enforce the PIN entry before every signature operation,
see the comments about  user_consent and CKA_ALWAYS_AUTHENTICATE
PIN caching will not work with a PIN PAD READER.


>
> Hans
>
> ______________________________________________________________________
> Dit bericht kan informatie bevatten die niet voor u is bestemd. Indien u niet de geadresseerde bent of dit bericht abusievelijk aan u is toegezonden, wordt u verzocht dat aan de afzender te melden en het bericht te verwijderen. De Staat aanvaardt geen aansprakelijkheid voor schade, van welke aard ook, die verband houdt met risico's verbonden aan het electronisch verzenden van berichten.
>
> This message may contain information that is not intended for you. If you are not the addressee or if this message was sent to you by mistake, you are requested to inform the sender and delete the message. The State accepts no liability for damage of any kind resulting from the risks inherent in the electronic transmission of messages.
>
> ------------------------------------------------------------------------------
> Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
> Discover the easy way to master current and previous Microsoft technologies
> and advance your career. Get an incredible 1,500+ hours of step-by-step
> tutorial videos with LearnDevNow. Subscribe today and save!
> http://pubads.g.doubleclick.net/gampad/clk?id=58040911&iu=/4140/ostg.clktrk
> _______________________________________________
> Opensc-devel mailing list
> Ope...@li...
> https://lists.sourceforge.net/lists/listinfo/opensc-devel
>

-- 

  Douglas E. Engert  <DEE...@an...>
  Argonne National Laboratory
  9700 South Cass Avenue
  Argonne, Illinois  60439
  (630) 252-5444

[Opensc-devel] pin-retry

[<J.W...@mi...>]

Hi all

I just had a discussion about pin-retry. Where is it defined?
On one of the recent HSM-pages, I noticed it is a parameter you can (default = 3) define.

But how about "regular" smart cards? 
I was expecting this to be an option at pkcs15-init..
Or is this something specific to the underlying hardware?

Hans

______________________________________________________________________
Dit bericht kan informatie bevatten die niet voor u is bestemd. Indien u niet de geadresseerde bent of dit bericht abusievelijk aan u is toegezonden, wordt u verzocht dat aan de afzender te melden en het bericht te verwijderen. De Staat aanvaardt geen aansprakelijkheid voor schade, van welke aard ook, die verband houdt met risico's verbonden aan het electronisch verzenden van berichten.

This message may contain information that is not intended for you. If you are not the addressee or if this message was sent to you by mistake, you are requested to inform the sender and delete the message. The State accepts no liability for damage of any kind resulting from the risks inherent in the electronic transmission of messages.

Re: [Opensc-devel] SC-HSM under Windows 7 64-bit?

[Anthony F. <ant...@gm...>]

Greetings.

(Apologies for the previous empty message, hit the wrong button...)

Thank you all for your help and suggestions.

It turns out that the root cause of the failure was much more prosaic:
the author of the original scripts had OpenSC (and OpenSSL) in their
PATH, and did not check for this on other systems.

So when I ran it on my system, it was unable to find the needed
executables.  (Why "GENERAL ERROR" instead of "PATH NOT FOUND", I
don't know.)

Apologies for the noise, and thanks again for the help.  I'm up and
running now, and can begin to investigate the actual issue my client
is facing.  :)

Best regards,
Anthony Foiani