MatthiasValvekens / pyHanko

pyHanko: sign and stamp PDF files
MIT License
511 stars 74 forks source link

PKCS#11 PIN Pad not working #133

Closed pquL1 closed 2 years ago

pquL1 commented 2 years ago

Hi, this is a great CLI tool, thanks for creating it.

Describe the bug With prompt_pin: False, the PIN Pad should be usable. However, it's not activated and a signature fails with pkcs11.exceptions.NoSuchKey.

To Reproduce Set prompt_pin: False, don't supply a PIN in the config and try to add a signature using the CLI.

Expected behavior The PIN Pad on the card reader should activate and allow PIN entry.

Additional context python-pkcs11 expects PROTECTED_AUTH as the user pin when it's supposed to use the PIN Pad (see the documentation). In pkcs11.py:468, it's instead set to None.

Changing this from pin = str(pin) if pin is not None else None to pin = str(pin) if pin is not None else PROTECTED_AUTH (with the corresponding import) solves the issue and activates the PIN Pad when prompt_pin: False is set. Not sure if there's a better place to make that cast (thus no PR) or if this might break something else, but it seems to work in my setup.

MatthiasValvekens commented 2 years ago

Hi @pquL1,

Good catch! I actually don't own a device with a PIN pad---or rather, the one device I do own that manages its own pin entry does so entirely outside the scope of PKCS#11 (and there, passing None for the user PIN works just fine). Since I guess you do own such a device, could you perhaps do a quick PR with the setup that worked for you, and then I'll check if it still works with the devices I have. If it does, we can just go ahead and merge it. If not, I'll refine the PKCS#11 config mechanism so both ways of deferring pin entry can be used from the CLI.

That reminds me, I should try to find some time to systematise my SoftHSMv2 setup so it can run in CI...

pquL1 commented 2 years ago

Hi @MatthiasValvekens, I've added a PR with the configuration that works with my setup. Hope it doesn't break yours!

MatthiasValvekens commented 2 years ago

In light of my remarks on #134, I'm reclassifying this as an enhancement. I hope to find some time this weekend to amend #134 to something that works for everyone :).