some pkinit plugin PKCS11 related issues

[Greg Hudson]

On 05/13/2014 06:41 PM, Will Fiveash wrote:
> The first is that way the "slotid" in krb5.conf is handled.  It
> should be used as a filter to choose one or more slots from the list of
> slots returned by C_GetSlotList()

PKCS11 doesn't seem to say whether C_GetSlotList is allowed to crash on
an invalid slot ID.  But I assume it does so on Solaris, which is reason
enough to change it.

I would assume that at most one slot from the list will match the slot
number in a sane PKCS11 implementation.

>     if (cctx->p11->C_GetSlotList(TRUE, slotlist, &count) != CKR_OK) {
[...]
>     /* examine all the tokens */
[...]
>         if (cctx->slotid != PK_NOSLOT && cctx->slotid != slotlist[i])
>             continue;
[...]
>         if ((r = cctx->p11->C_OpenSession(slotlist[i], CKF_SERIAL_SESSION,
>                           NULL, NULL, &tmpsession)) != CKR_OK) {

This seems reasonable, and should come to a little less code than is
already present.

>         for (cp = tinfo.label + sizeof (tinfo.label) - 1;
>              *cp == '\0' || *cp == ' '; cp--)
>             *cp = '\0';
[...]
>         if (cctx->token_label == NULL ||
>             !strcmp((char *) cctx->token_label, (char *) tinfo.label))

That does seem like a botch.  It should find the last non-whitespace
character, then compare the length against strlen(cctx->token_label) and
memcmp() the contents.  There should be no need to alter tinfo.label as
we scan backwards, but we should guard against underrunning tinfo.label.