Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
rgl opened this issue Aug 23, 2020 · 8 comments · Fixed by #90
Closed

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

rgl opened this issue Aug 23, 2020 · 8 comments · Fixed by #90

Comments

@rgl
Copy link
Contributor

rgl commented Aug 23, 2020

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
Copy link
Collaborator

danni commented Aug 23, 2020

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
Copy link
Collaborator

danni commented Aug 23, 2020

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
Copy link
Contributor Author

rgl commented Aug 24, 2020

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
Copy link
Collaborator

danni commented Aug 24, 2020

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
Copy link
Contributor Author

rgl commented Aug 29, 2020

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
Copy link
Collaborator

danni commented Aug 29, 2020 via email

@rgl
Copy link
Contributor Author

rgl commented Aug 30, 2020

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
Copy link
Collaborator

danni commented Aug 30, 2020

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.

@danni danni closed this as completed in #90 Aug 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants