crucial issue (for me): slot ID vs index in engine_pkcs11 / libp11

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

crucial issue (for me): slot ID vs index in engine_pkcs11 / libp11

Umberto Rustichelli aka Ubi
Dear all, I find myself working with an old version of engine_pkcs11 /
libp11 libraries, that served me well for years.
The issue almost certainly apply to the most recent release (NOTICE: the
libp11 dowload link on the page
https://www.opensc-project.org/opensc/wiki/libp11 is broken).

It is not uncommon to have slot ID which are quite high, for instance
761406623 with an HSM (my case).
It almost always happens when using multiple PKCS#11 drivers, that is how
I found out about the problem...

The point is that the slot ID (as numbered by the PKCS#11 drver) has
nothing to do with the index of the slots array generated by libp11, only
accidentally they match when you're using one driver only.

If you identify a key with, for instance, the name
"slot_761406623-id_1307301149164400" (reporting the slot ID), it will
miserably fail (and it is a good thing, at least it is not trying to
access the wrong slot with a bad PIN) because it finds out that  761406623
is not good. The message is "Invalid slot number: 761406623" even if the
slot ID is exactly that.

Now, I suspect that the original intention is to put the slot ID, not the
slot array index, in the string... is my observation correct?
Or did I make any mistake in my analysis of the code?

If I am right, I am likely going to work on my old version and change the
code for my purpose even if the intention was not to indicate the slot
ID...: do you have some important advice regarding my attempt?





------------------------------------------------------------------------------
Get your SQL database under version control now!
Version control is standard for application code, but databases havent
caught up. So what steps can you take to put your SQL databases under
version control? Why should you start doing it? Read more to find out.
http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel
Reply | Threaded
Open this post in threaded view
|

Re: crucial issue (for me): slot ID vs index in engine_pkcs11 / libp11

Douglas E. Engert


On 7/30/2013 10:49 AM, [hidden email] wrote:
> Dear all, I find myself working with an old version of engine_pkcs11 /
> libp11 libraries, that served me well for years.
> The issue almost certainly apply to the most recent release (NOTICE: the
> libp11 dowload link on the page
> https://www.opensc-project.org/opensc/wiki/libp11 is broken).

OpenSC is now using GitHub.
https://github.com/OpenSC

The old www.opensc-project.org should reflect that GitHub is the source.


https://github.com/OpenSC
>
> It is not uncommon to have slot ID which are quite high, for instance
> 761406623 with an HSM (my case).
> It almost always happens when using multiple PKCS#11 drivers, that is how
> I found out about the problem...
>
> The point is that the slot ID (as numbered by the PKCS#11 drver) has
> nothing to do with the index of the slots array generated by libp11, only
> accidentally they match when you're using one driver only.

There was a change like this for OpenSC, pkcs11.
Sounds like a change is needed in libp11.

>
> If you identify a key with, for instance, the name
> "slot_761406623-id_1307301149164400" (reporting the slot ID), it will
> miserably fail (and it is a good thing, at least it is not trying to
> access the wrong slot with a bad PIN) because it finds out that  761406623
> is not good. The message is "Invalid slot number: 761406623" even if the
> slot ID is exactly that.
>
> Now, I suspect that the original intention is to put the slot ID, not the
> slot array index, in the string... is my observation correct?

Yes, the slot is  not an index, but more of a handle.

> Or did I make any mistake in my analysis of the code?


The pkcs11-tool  has options:
   -L, --list-slots              List available slots
   -T, --list-token-slots        List slots with tokens

  --slot <arg>              Specify the ID of the slot to use
  --slot-description <arg>  Specify the description of the slot to use
  --slot-index <arg>        Specify the index of the slot to use

Maybe the engine_pkcs11 and libp11 need to support one of these
methods to find a slot.

>
> If I am right, I am likely going to work on my old version and change the
> code for my purpose even if the intention was not to indicate the slot
> ID...: do you have some important advice regarding my attempt?
>
>
>
>
>
> ------------------------------------------------------------------------------
> Get your SQL database under version control now!
> Version control is standard for application code, but databases havent
> caught up. So what steps can you take to put your SQL databases under
> version control? Why should you start doing it? Read more to find out.
> http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk
> _______________________________________________
> Opensc-devel mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/opensc-devel
>

--

  Douglas E. Engert  <[hidden email]>
  Argonne National Laboratory
  9700 South Cass Avenue
  Argonne, Illinois  60439
  (630) 252-5444

------------------------------------------------------------------------------
Get your SQL database under version control now!
Version control is standard for application code, but databases havent
caught up. So what steps can you take to put your SQL databases under
version control? Why should you start doing it? Read more to find out.
http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel
Reply | Threaded
Open this post in threaded view
|

Re: crucial issue (for me): slot ID vs index in engine_pkcs11 / libp11

Anthony Foiani
In reply to this post by Umberto Rustichelli aka Ubi
Greetings.

(Note, I find it a bit easier to reply if there's a human name there
somewhere.  :)

On Tue, Jul 30, 2013 at 9:49 AM, <[hidden email]> wrote:

> The point is that the slot ID (as numbered by the PKCS#11 drver) has
> nothing to do with the index of the slots array generated by libp11, only
> accidentally they match when you're using one driver only.
>
> If you identify a key with, for instance, the name
> "slot_761406623-id_1307301149164400" (reporting the slot ID), it will
> miserably fail (and it is a good thing, at least it is not trying to
> access the wrong slot with a bad PIN) because it finds out that  761406623
> is not good. The message is "Invalid slot number: 761406623" even if the
> slot ID is exactly that.
>
> Now, I suspect that the original intention is to put the slot ID, not the
> slot array index, in the string... is my observation correct?
> Or did I make any mistake in my analysis of the code?

A patch would be interesting, especially if it is obvious that any
large number is intended as slot id, not array index -- that could
minimize compatibility headaches.

> If I am right, I am likely going to work on my old version and change the
> code for my purpose even if the intention was not to indicate the slot
> ID...: do you have some important advice regarding my attempt?

If you can use a token label, does that work for your cases?  libp11
allows access via "label_...." if I remember correctly.

Failing that, can you enumerate the slots via libp11 and get a
(presumably valid) handle that way?

Good luck.

Best regards,
Anthony Foiani

------------------------------------------------------------------------------
Get your SQL database under version control now!
Version control is standard for application code, but databases havent
caught up. So what steps can you take to put your SQL databases under
version control? Why should you start doing it? Read more to find out.
http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel
Reply | Threaded
Open this post in threaded view
|

Re: crucial issue (for me): slot ID vs index in engine_pkcs11 / libp11

Umberto Rustichelli aka Ubi

Douglas, Anthony, thank you very much for your enlighting replies.
My name is Umberto Rustichelli (aka Ubi).
For the moment, I'm going to try a quick-and-dirty hack in engine_pkcs11.c
(version is 0.1.5, I know it's old but I'm tied to that by a number of
dependencies in an old environment), see more lines later.
Then, I'd like to do something about libp11 but it's not smart to work on
old code... also: I don't want to break applications that rely on the fact
that the array index is used

> Greetings.
>
> (Note, I find it a bit easier to reply if there's a human name there
> somewhere.  :)
>
> On Tue, Jul 30, 2013 at 9:49 AM, <[hidden email]> wrote:
>> The point is that the slot ID (as numbered by the PKCS#11 drver) has
>> nothing to do with the index of the slots array generated by libp11,
>> only accidentally they match when [...]
>> Now, I suspect that the original intention is to put the slot ID, not
>> the slot array index [...]

> A patch would be interesting, especially if it is obvious that any
> large number is intended as slot id, not array index -- that could
> minimize compatibility headaches.

In functions pkcs11_load_key and pkcs11_load_cert I plan to insert this
code (not compiled yet, some errors may occur):

// Ubi: 20130730: now convert the slot_nr, which is a slot ID for me,
// into the slot array index, which is what the code here expects
#if 1
  if (slot_nr >= 0)
  {
    int slotapos; // position in array
    unsigned long curr_slot_ID;
    int found;
    found = 0;
    for (slotapos = 0; slotapos < count; slotapos++)
    {
       curr_slot_ID = slot_list[slotapos]->priv->id;
       if (curr_slot_ID == slot_nr)
       {
         slot_nr = slotapos;
         found = 1;
         break;
       }
    }
    if (!found)
    {
      fprintf(stderr, "Ubi: slot ID %d to idx failed\n", slot_nr);
      PKCS11_release_all_slots(ctx, slot_list, count);
      return NULL;
    }
    else
    {
      fprintf(stderr,
        "Ubi: performed conversion slot ID to idx %d\n", slot_nr);
    }
        }
#endif

just before the existing part that rejects the slot ID:

if (slot_nr == -1) {
  if (!(slot = PKCS11_find_token(ctx, slot_list, count)))
    fail("didn't find any tokens\n");
} else if (slot_nr >= 0 && slot_nr < count)
  slot = slot_list + slot_nr;
  else {
    fprintf(stderr, "Invalid slot number: %d\n", slot_nr);
    PKCS11_release_all_slots(ctx, slot_list, count);
    return NULL;
}

> [...] If you can use a token label, does that work for your cases?
> libp11 allows access via "label_...." if I remember correctly.

I think the label is for the key label only, not for the slot label.

> Failing that, can you enumerate the slots via libp11 and get a
> (presumably valid) handle that way?

I can, but I rely on engine_pkcs11 so I will first change the code there.
In my opinion, libp11 shoould provide functions that work on the array
index, for backward compatibility, but also expose the slot ID in the
first place.
Putting together pkcs11_load_key and pkcs11_load_cert is not a bad idea,
too, they have so much code in common...

Bye

   Ubi




------------------------------------------------------------------------------
Get your SQL database under version control now!
Version control is standard for application code, but databases havent
caught up. So what steps can you take to put your SQL databases under
version control? Why should you start doing it? Read more to find out.
http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel
Reply | Threaded
Open this post in threaded view
|

Re: crucial issue (for me): slot ID vs index in engine_pkcs11 / libp11

Alon Bar-Lev
Hi,

Slot id is not fixed in PKCS#11 to allow plug and play. What I suggest
is using token serial number instead to consistent behavior.

Alon

On Wed, Jul 31, 2013 at 9:35 AM,  <[hidden email]> wrote:

>
> Douglas, Anthony, thank you very much for your enlighting replies.
> My name is Umberto Rustichelli (aka Ubi).
> For the moment, I'm going to try a quick-and-dirty hack in engine_pkcs11.c
> (version is 0.1.5, I know it's old but I'm tied to that by a number of
> dependencies in an old environment), see more lines later.
> Then, I'd like to do something about libp11 but it's not smart to work on
> old code... also: I don't want to break applications that rely on the fact
> that the array index is used
>
>> Greetings.
>>
>> (Note, I find it a bit easier to reply if there's a human name there
>> somewhere.  :)
>>
>> On Tue, Jul 30, 2013 at 9:49 AM, <[hidden email]> wrote:
>>> The point is that the slot ID (as numbered by the PKCS#11 drver) has
>>> nothing to do with the index of the slots array generated by libp11,
>>> only accidentally they match when [...]
>>> Now, I suspect that the original intention is to put the slot ID, not
>>> the slot array index [...]
>
>> A patch would be interesting, especially if it is obvious that any
>> large number is intended as slot id, not array index -- that could
>> minimize compatibility headaches.
>
> In functions pkcs11_load_key and pkcs11_load_cert I plan to insert this
> code (not compiled yet, some errors may occur):
>
> // Ubi: 20130730: now convert the slot_nr, which is a slot ID for me,
> // into the slot array index, which is what the code here expects
> #if 1
>   if (slot_nr >= 0)
>   {
>     int slotapos; // position in array
>     unsigned long curr_slot_ID;
>     int found;
>     found = 0;
>     for (slotapos = 0; slotapos < count; slotapos++)
>     {
>        curr_slot_ID = slot_list[slotapos]->priv->id;
>        if (curr_slot_ID == slot_nr)
>        {
>          slot_nr = slotapos;
>          found = 1;
>          break;
>        }
>     }
>     if (!found)
>     {
>       fprintf(stderr, "Ubi: slot ID %d to idx failed\n", slot_nr);
>       PKCS11_release_all_slots(ctx, slot_list, count);
>       return NULL;
>     }
>     else
>     {
>       fprintf(stderr,
>         "Ubi: performed conversion slot ID to idx %d\n", slot_nr);
>     }
>         }
> #endif
>
> just before the existing part that rejects the slot ID:
>
> if (slot_nr == -1) {
>   if (!(slot = PKCS11_find_token(ctx, slot_list, count)))
>     fail("didn't find any tokens\n");
> } else if (slot_nr >= 0 && slot_nr < count)
>   slot = slot_list + slot_nr;
>   else {
>     fprintf(stderr, "Invalid slot number: %d\n", slot_nr);
>     PKCS11_release_all_slots(ctx, slot_list, count);
>     return NULL;
> }
>
>> [...] If you can use a token label, does that work for your cases?
>> libp11 allows access via "label_...." if I remember correctly.
>
> I think the label is for the key label only, not for the slot label.
>
>> Failing that, can you enumerate the slots via libp11 and get a
>> (presumably valid) handle that way?
>
> I can, but I rely on engine_pkcs11 so I will first change the code there.
> In my opinion, libp11 shoould provide functions that work on the array
> index, for backward compatibility, but also expose the slot ID in the
> first place.
> Putting together pkcs11_load_key and pkcs11_load_cert is not a bad idea,
> too, they have so much code in common...
>
> Bye
>
>    Ubi
>
>
>
>
> ------------------------------------------------------------------------------
> Get your SQL database under version control now!
> Version control is standard for application code, but databases havent
> caught up. So what steps can you take to put your SQL databases under
> version control? Why should you start doing it? Read more to find out.
> http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk
> _______________________________________________
> Opensc-devel mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/opensc-devel

------------------------------------------------------------------------------
Get your SQL database under version control now!
Version control is standard for application code, but databases havent
caught up. So what steps can you take to put your SQL databases under
version control? Why should you start doing it? Read more to find out.
http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel
Reply | Threaded
Open this post in threaded view
|

Re: crucial issue (for me): slot ID vs index in engine_pkcs11 / libp11

Umberto Rustichelli aka Ubi

Here is the hack that works for me, in engine_pkcs11.c
Of course it is not a solution to the general issue and yes, it is a bit
verbose.
Sorry for I cannot provide a proper patch but I am editing code which is
not the original: I already changed it some time ago to fix an issue with
key ID string parsing (that fix I submitted to the list some time ago, the
official code took the fix into account but in a later version).

// Ubi: 20130730: I need ths for my hack. This struct is not exposed by
libp11.
// Also, I am not using CK_SLOT_ID, etc. which are not known at this level

typedef struct pkcs11_slot_private {
  void *parent;
  unsigned char haveSession, loggedIn;
  unsigned long id;
  unsigned long session;
} PKCS11_SLOT_private;

[...] this is what I put just before the "if (slot_nr == -1) ..."

// Ubi: 20130730: now convert the slot_nr, which is a slot ID for me,
// into the slot array index, which is what the code here expects
#if 1
if (slot_nr >= 0)
{
  int slotapos; // position in array
  unsigned long curr_slot_ID;
  PKCS11_SLOT_private *slidata;
  int found;
  found = 0;
  for (slotapos = 0; slotapos < count; slotapos++)
  {
    slot = slot_list + slotapos;
    slidata = slot->_private;
    curr_slot_ID = slidata->id;
    if (curr_slot_ID == slot_nr)
    {
      slot_nr = slotapos;
      found = 1;
      break;
    }
  }
  if (!found)
  {
    fprintf(stderr, "Ubi: slot ID %d to idx failed\n", slot_nr);
    PKCS11_release_all_slots(ctx, slot_list, count);
    return NULL;
  }
  else
  {
    fprintf(stderr, "Ubi: performed conversion slot ID to idx %d\n",
slot_nr);
  }
}
#endif


> Hi,
>
> Slot id is not fixed in PKCS#11 to allow plug and play. What I suggest
> is using token serial number instead to consistent behavior.

This is a very good idea, the point is that libp11, at least the version
that I have, must expose some more slot information in the first place.
The "_private" structure is unknown to the caller.
It can stay hidden, but functions to query about a slot ID and token
serial (for the given slot) would be useful.
Also, the string slot_<slot ID>-... must be redefined first to allow for
passing the token serial, and I am not the person who can decide about
that.
Personally, I would also like to hava available:
tkserial_<token serial>-id_<key ID>
and
tklabel_<token label>-id_<key ID>

Thanks everybody

   Umberto Rustichelli aka Ubi

> Alon
>
> On Wed, Jul 31, 2013 at 9:35 AM,  <[hidden email]> wrote:
>>
>> Douglas, Anthony, thank you very much for your enlighting replies.
>> My name is Umberto Rustichelli (aka Ubi).
>> For the moment, I'm going to try a quick-and-dirty hack in
>> engine_pkcs11.c
>> (version is 0.1.5, I know it's old but I'm tied to that by a number of
>> dependencies in an old environment), see more lines later.
>> Then, I'd like to do something about libp11 but it's not smart to work
>> on
>> old code... also: I don't want to break applications that rely on the
>> fact
>> that the array index is used
>>
>>> Greetings.
>>>
>>> (Note, I find it a bit easier to reply if there's a human name there
>>> somewhere.  :)
>>>
>>> On Tue, Jul 30, 2013 at 9:49 AM, <[hidden email]> wrote:
>>>> The point is that the slot ID (as numbered by the PKCS#11 drver) has
>>>> nothing to do with the index of the slots array generated by libp11,
>>>> only accidentally they match when [...]
>>>> Now, I suspect that the original intention is to put the slot ID, not
>>>> the slot array index [...]
>>
>>> A patch would be interesting, especially if it is obvious that any
>>> large number is intended as slot id, not array index -- that could
>>> minimize compatibility headaches.
>>
>> In functions pkcs11_load_key and pkcs11_load_cert I plan to insert this
>> code (not compiled yet, some errors may occur):
>>
[... removed, it was broken]

>>
>> just before the existing part that rejects the slot ID:
>>
>> if (slot_nr == -1) {
>>   if (!(slot = PKCS11_find_token(ctx, slot_list, count)))
>>     fail("didn't find any tokens\n");
>> } else if (slot_nr >= 0 && slot_nr < count)
>>   slot = slot_list + slot_nr;
>>   else {
>>     fprintf(stderr, "Invalid slot number: %d\n", slot_nr);
>>     PKCS11_release_all_slots(ctx, slot_list, count);
>>     return NULL;
>> }
>>
>>> [...] If you can use a token label, does that work for your cases?
>>> libp11 allows access via "label_...." if I remember correctly.
>>
>> I think the label is for the key label only, not for the slot label.
>>
>>> Failing that, can you enumerate the slots via libp11 and get a
>>> (presumably valid) handle that way?
>>
>> I can, but I rely on engine_pkcs11 so I will first change the code
>> there.
>> In my opinion, libp11 shoould provide functions that work on the array
>> index, for backward compatibility, but also expose the slot ID in the
>> first place.
>> Putting together pkcs11_load_key and pkcs11_load_cert is not a bad idea,
>> too, they have so much code in common...
>>
>> Bye
>>
>>    Ubi



------------------------------------------------------------------------------
Get your SQL database under version control now!
Version control is standard for application code, but databases havent
caught up. So what steps can you take to put your SQL databases under
version control? Why should you start doing it? Read more to find out.
http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel
Reply | Threaded
Open this post in threaded view
|

Re: crucial issue (for me): slot ID vs index in engine_pkcs11 / libp11

Umberto Rustichelli aka Ubi
>
> Here is the hack that works for me, in engine_pkcs11.c
>
[...]
>>
>> Slot id is not fixed in PKCS#11 to allow plug and play. What I suggest
>> is using token serial number instead to consistent behavior.
>
> This is a very good idea, the point is that libp11, at least the version
> that I have, must expose some more slot information in the first place.
> The "_private" structure is unknown to the caller.
> [...]

I'd just point out that the function PKCS11_enumerate_slots should not
acquire the token serials: that should be on demand (and possibly
per-token), as the PKCS11_enumerate_slots only performs PKCS#11 functions
up to C_GetSlotInfo and is not intended for accessing the tokens, just the
slots.

  Ubi



------------------------------------------------------------------------------
Get your SQL database under version control now!
Version control is standard for application code, but databases havent
caught up. So what steps can you take to put your SQL databases under
version control? Why should you start doing it? Read more to find out.
http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel