Re: [Opensc-devel] crucial issue (for me): slot ID vs index in engine_pkcs11 / libp11

[<op...@se...>]

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,  <op...@se...> 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, <op...@se...> 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