pyauth / python-pkcs11

PKCS#11/Cryptoki support for Python
MIT License
149 stars 71 forks source link

Cannot get a token by label with lib.get_token #89

Closed rgl closed 4 years ago

rgl commented 4 years ago

I'm using a SmartCard-HSM-4K-Mini-SIM that I've initialized with:

pkcs11-tool \
    --module opensc-pkcs11.so \
    --init-token \
    --init-pin \
    --so-pin 3537363231383830 \
    --pin 648219 \
    --new-pin 648219 \
    --label test

And the token label ended up being:

pkcs11-tool --module opensc-pkcs11.so --list-slots
Available slots:
Slot 0 (0x0): Alcor Micro AU9560 00 00
  token label        : test (UserPIN)
  token manufacturer : www.CardContact.de
  token model        : PKCS#15 emulated
  token flags        : login required, rng, token initialized, PIN initialized
  hardware version   : 24.13
  firmware version   : 3.1
  serial num         : X
  pin min/max        : 6/15

But when I try to call lib.get_token('test (UserPIN)') it fails with:

Traceback (most recent call last):
  File "main.py", line 8, in <module>
    token = lib.get_token(token_label=token_label)
  File "pkcs11/_pkcs11.pyx", line 1544, in pkcs11._pkcs11.lib.get_token
pkcs11.exceptions.NoSuchToken: No token matching {'token_label': 'test (UserPIN)'}

The actual problem was revealed after enumerating the token labels with:

#!/usr/bin/python3

import os
import pkcs11
import binascii

lib = pkcs11.lib(os.environ['PKCS11_LIBRARY_PATH'])
token_label = os.environ['PKCS11_TOKEN_LABEL'] # pkcs11-tool --module opensc-pkcs11.so --list-slots

#token = lib.get_token(token_label=token_label)

for slot in lib.get_slots():
    token = slot.get_token()
    stripped_token_label = token.label.rstrip('\x00')
    print(f"token_label: `{token.label}` ({binascii.hexlify(token.label.encode('utf8'))} {type(token.label)} {len(token.label)})")
    print(f"token_label: `{token_label}` ({binascii.hexlify(token_label.encode('utf8'))} {type(token_label)} {len(token_label)})")
    print(f"equals? {token.label == token_label}")
    print(f"equals? {stripped_token_label == token_label}")

Which returned:

token_label: `test (UserPIN)` (b'7465737420285573657250494e29000000000000000000000000000000000000' <class 'str'> 32)
token_label: `test (UserPIN)` (b'7465737420285573657250494e29' <class 'str'> 14)
equals? False
equals? True

So it seems that, for some odd reason, the token label is padded with zeros...

And it only works when we do a token.label.rstrip('\x00') before comparing with the the user provided token label.

Can lib.get_token be modified to strip trailing NUL characters?

danni commented 4 years ago

Interesting. It feels like something has changed in the string implementation and we're no longer implicitly stripping nuls from the ends of strings. Can I confirm what Python version this is, and what Cython version (should appear in pip list).

danni commented 4 years ago

Oh, I see what's happened. At some point we started forcing the length of strings we received from PKCS#11 in case they weren't properly null terminated (happens), and so Cython treats them like Pascal strings, and not like C strings. But it only comes up if your HSM does something like returns a null-terminated, but longer buffer.

Can you try installing an editable version of python-pkcs11 pip install -e and try changing the implementation of types.py:_CK_UTF8CHAR_to_str to include an rstrip(0x0)?

rgl commented 4 years ago

I've changed it to:


def _CK_UTF8CHAR_to_str(data):
    """Convert CK_UTF8CHAR to string."""
    return data.decode('utf-8').rstrip('\0')

And it now works!

I'm not sure how to go from here... will you want a PR? will you commit the change yourself?

danni commented 4 years ago

Hi, yep, send a pull request, we can run the tests against it.

I wonder if it should be data.rstrip(b'\0').decode(...).rstrip()... That should more closely preserve the original intent.

rgl commented 4 years ago

Interestingly enough the https://github.com/miekg/pkcs11 Go library also has this behavior, maybe this is some kind of bug in the HSM or its pkcs11 module.

Either way, I'll send a PR soon.

Does this project as a CI somewhere? Are you interested in having a GitHub actions workflow for this project?

danni commented 4 years ago

Yeah there’s CI through Travis run against SoftHSM2.

You can also try running the test suite against your HSM but it can be fiddly depending on the features of your HSM. If you want to give it a go have a read of the test scripts. There’s decorators to mask out things we don’t correctly detect as unavailable. Make sure you can create session keys though. Else you’ll create 100s of objects on your HSM. On 30 Aug 2020, 05:19 +1000, Rui Lopes notifications@github.com, wrote:

Interestingly enough the https://github.com/miekg/pkcs11 Go library also has this behavior, maybe this is some kind of bug in the HSM or its pkcs11 module. Either way, I'll send a PR soon. Does this project as a CI somewhere? Are you interested in having a GitHub actions workflow for this project? — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

rgl commented 4 years ago

Its in #90, but I quite puzzled to why the build only failed in Python 3.5. Can you take a look?

BTW, the cryptography library is deprecating Python 3.5.

BTW, I also created #91 to make the travis build more discoverable.

danni commented 4 years ago

I don't know why that failed on Python 3.5, it passed when I reran it. That's weird. That code shouldn't even be passing through your changes. Maybe it's due to a bug in one of our deps. It's probably time to drop Python 3.5 tbh.