openssh unix dev - Jun 2016 - ssh-pkcs11.c

Hi,

It seems there is a bug with the pkcs11 feature where a zero-length
PIN is accepted. I believe this is a bug, since the user might want to
press return when asked for the PIN to ignore that slot/key.

This is caused at pkcs11_rsa_private_encrypt:

snprintf(prompt, sizeof(prompt),
   "Enter PIN for '%s': ", si->token.label);
pin = read_passphrase(prompt, RP_ALLOW_EOF);
if (pin == NULL)
return (-1); /* bail out */

Actually a zero-length PIN will not cause a NULL to be returned, so it
will still try to authenticate and fail the PIN login!

Also, I think it would be great to support the CKF_* flags to provide
some feedback to the user regarding PIN tries left remaining,
something like this:

if (info.flags & CKF_USER_PIN_COUNT_LOW)
   printf("WARNING: User PIN count low\n");
else if (info.flags & CKF_USER_PIN_FINAL_TRY)
   printf("WARNING: User PIN final try\n");
else if (info.flags & CKF_USER_PIN_LOCKED) /* Maybe we should bail out
here, or just try to continue? */
   printf("WARNING: User PIN reported locked\n");

Thanks,
Nuno

On 17 June 2016 at 20:58, Nuno Gon?alves <nunojpg at gmail.com>
wrote:
> Hi,
>
> It seems there is a bug with the pkcs11 feature where a zero-length
> PIN is accepted. I believe this is a bug, since the user might want to
> press return when asked for the PIN to ignore that slot/key.
Hi,

Empty PIN is valid case, not sure why you want to avoid supporting it.

Alon

On Fri, Jun 17, 2016 at 7:57 PM, Alon Bar-Lev <alon.barlev at gmail.com>
wrote:
> On 17 June 2016 at 20:58, Nuno Gon?alves <nunojpg at gmail.com>
wrote:
>> Hi,
>>
>> It seems there is a bug with the pkcs11 feature where a zero-length
>> PIN is accepted. I believe this is a bug, since the user might want to
>> press return when asked for the PIN to ignore that slot/key.
>
> Hi,
>
> Empty PIN is valid case, not sure why you want to avoid supporting it.
>
> Alon
I didn't know it was valid but the reasoning still applies. I don't
really know the standard use cases, but I think it could eventually be
useful for the user, when asked for the PIN, to decide not enter it.
Currently it can only be done by killing ssh. If empty PIN is valid,
but eventually not usual, maybe we should ask if the user really wants
to try a empty pin or just continue to another authentication option?


Regarding the CKF_USER_PIN flags, do you think it is a good idea to
implement the warning messages?

Thanks,
Nuno