Skip to content

Conversation

@alt3r-3go
Copy link
Contributor

@alt3r-3go alt3r-3go commented Feb 20, 2024

As discussed in OpenSC/OpenSC#3017, here are the basic testing results (pkcs11-tool --login --test) for Nitrokey Start and Pro 2 (full output below for completeness).

Start seems to have a problem at the very last step (thus "many tests passed" and not "all"), but this was present before the changes for 0.25.0-rc1 went in (master as of at least a month or so ago). I plan to look into this later, after I update the token to the latest FW (RTM.13), which I'm having some troubles with at the moment.

# Nitrokey Start
[user@host dir]$ ./install/bin/pkcs11-tool --module=./install/lib/opensc-pkcs11.so --test --login --pin <removed>
Using slot 0 with a present token (0x0)
C_SeedRandom() and C_GenerateRandom():
  seeding (C_SeedRandom) not supported
  seems to be OK
Digests:
  all 4 digest functions seem to work
  MD5: OK
  RIPEMD160: OK
  SHA-1: OK
  SHA256: OK
Ciphers: not implemented
Signatures (currently only for RSA)
  testing key 0 (Encryption key)  -- can't be used for signature, skipping
  testing key 1 (Authentication key) 
  all 4 signature functions seem to work
  testing signature mechanisms:
    RSA-PKCS: OK
    SHA1-RSA-PKCS: OK
    MD5-RSA-PKCS: OK
    RIPEMD160-RSA-PKCS: OK
    SHA256-RSA-PKCS: OK
  testing key 1 (Authentication key) with 1 mechanism
    RSA-PKCS: OK
Verify (currently only for RSA)
  testing key 0 (Encryption key) -- can't be used to sign/verify, skipping
  testing key 1 (Authentication key) with 1 mechanism
    RSA-PKCS: OK
Unwrap: not implemented
Decryption (currently only for RSA)
  testing key 0 (Encryption key)
    RSA-PKCS: OK
  testing key 1 (Authentication key)
error: PKCS11 function C_Decrypt failed: rv = CKR_GENERAL_ERROR (0x5)
Aborting.
    RSA-PKCS: [user@host dir]$ 

# Nitrokey Pro 2
[user@host dir]$ ./install/bin/pkcs11-tool --module=./install/lib/opensc-pkcs11.so --test --login --pin <removed>
C_SeedRandom() and C_GenerateRandom():
  seeding (C_SeedRandom) not supported
  seems to be OK
Digests:
  all 4 digest functions seem to work
  MD5: OK
  RIPEMD160: OK
  SHA-1: OK
  SHA256: OK
Ciphers: not implemented
Signatures (currently only for RSA)
  testing key 0 (Encryption key)  -- can't be used for signature, skipping
  testing key 1 (Authentication key) 
  all 4 signature functions seem to work
  testing signature mechanisms:
    RSA-PKCS: OK
    SHA1-RSA-PKCS: OK
    MD5-RSA-PKCS: OK
    RIPEMD160-RSA-PKCS: OK
    SHA256-RSA-PKCS: OK
  testing key 1 (Authentication key) with 1 mechanism
    RSA-PKCS: OK
Verify (currently only for RSA)
  testing key 0 (Encryption key) -- can't be used to sign/verify, skipping
  testing key 1 (Authentication key) with 1 mechanism
    RSA-PKCS: OK
Unwrap: not implemented
Decryption (currently only for RSA)
  testing key 0 (Encryption key)
    RSA-PKCS: OK
  testing key 1 (Authentication key)
    RSA-PKCS: OK
No errors

@dengert
Copy link
Member

dengert commented Feb 20, 2024

error: PKCS11 function C_Decrypt failed: rv = CKR_GENERAL_ERROR (0x5)

can be fixed with OpenSC/OpenSC@1b95de4
which is part of a much larger set changes in OpenSC/OpenSC@master...X25519-improvements-2

@alt3r-3go
Copy link
Contributor Author

alt3r-3go commented Feb 20, 2024

Oh, I see, thank you and indeed - case solved then :)

@Jakuje
Copy link
Member

Jakuje commented Feb 21, 2024

Oh, I see, thank you and indeed - case solved then :)

Do I read that correctly that this commit solves the issue for you? @dengert can you submit this commit as separate PR so it can be merged for the next rc?

@dengert
Copy link
Member

dengert commented Feb 21, 2024

@dengert can you submit this commit as separate PR so it can be merged for the next rc?
will do.

@alt3r-3go
Copy link
Contributor Author

Oh, I see, thank you and indeed - case solved then :)

Do I read that correctly that this commit solves the issue for you? @dengert can you submit this commit as separate PR so it can be merged for the next rc?

I haven't tested it, just relied on debugging @dengert did when working on that commit. I can test if that's desired, though it will likely need to wait until the weekend.

@alt3r-3go
Copy link
Contributor Author

Actually, scratch the last part - turns out I can do a quick test right away.

@alt3r-3go
Copy link
Contributor Author

alt3r-3go commented Feb 21, 2024

Yes, that patch fixes it for me too (see below). The Pro 2 still works fine, no change in the test output.

[user@dev-alt3r opensc-src]$ /home/user/dev/opensc/install/bin/pkcs11-tool --module=/home/user/dev/opensc/install/lib/opensc-pkcs11.so --test --login
Using slot 0 with a present token (0x0)
Logging in to "OpenPGP card (User PIN)".
Please enter User PIN: 
C_SeedRandom() and C_GenerateRandom():
  seeding (C_SeedRandom) not supported
  seems to be OK
Digests:
  all 4 digest functions seem to work
  MD5: OK
  RIPEMD160: OK
  SHA-1: OK
  SHA256: OK
Ciphers: not implemented
Signatures (currently only for RSA)
  testing key 0 (Encryption key)  -- can't be used for signature, skipping
  testing key 1 (Authentication key) 
  all 4 signature functions seem to work
  testing signature mechanisms:
    RSA-PKCS: OK
    SHA1-RSA-PKCS: OK
    MD5-RSA-PKCS: OK
    RIPEMD160-RSA-PKCS: OK
    SHA256-RSA-PKCS: OK
  testing key 1 (Authentication key) with 1 mechanism
    RSA-PKCS: OK
Verify (currently only for RSA)
  testing key 0 (Encryption key) -- can't be used to sign/verify, skipping
  testing key 1 (Authentication key) with 1 mechanism
    RSA-PKCS: OK
Unwrap: not implemented
Decryption (currently only for RSA)
  testing key 0 (Encryption key)
    RSA-PKCS: OK
  testing key 1 (Authentication key) -- can't be used to decrypt, skipping
No errors

@alt3r-3go
Copy link
Contributor Author

I see 2f9ab4d was merged in the meanwhile, and it also mentions Nitrokey Start, although without FW version. Should I remove the Start from this PR, or maybe @xhanulik could qualify currently committed result with a version too (I can add it in this PR while rebasing)?

@Jakuje
Copy link
Member

Jakuje commented Mar 4, 2024

I see 2f9ab4d was merged in the meanwhile, and it also mentions Nitrokey Start, although without FW version. Should I remove the Start from this PR, or maybe @xhanulik could qualify currently committed result with a version too (I can add it in this PR while rebasing)?

Sorry for a delay. We had larger bunch of cards tested including the Nitrokey start so please, rebase on top of current changes so we can merge your testing results too. Feel free to add the FW version of your nitrokey start too.

@alt3r-3go alt3r-3go force-pushed the 0-25-rc1-nk-test-results branch from 0b42331 to 2d80352 Compare March 4, 2024 20:50
@alt3r-3go
Copy link
Contributor Author

alt3r-3go commented Mar 4, 2024

Thank you and done. The line is now longer than the other ones, so being unsure of what's the formatting preference here, I used the approach from one of the earlier updates (just let the line stick out, and no space padding before the closing |) - please let me know if some other approach is desired.

I also took the liberty of fixing a whitespace error + changing the "NitroKey" to "Nitrokey", as that's what they use as the name all over the NK documentation and marketing materials.

@Jakuje
Copy link
Member

Jakuje commented Mar 5, 2024

Than you. Looks good. I will leave it up to @xhanulik to merge and update the main repository.

@Jakuje Jakuje requested a review from xhanulik March 5, 2024 08:11
@xhanulik
Copy link
Member

xhanulik commented Mar 5, 2024

I also took the liberty of fixing a whitespace error + changing the "NitroKey" to "Nitrokey", as that's what they use as the name all over the NK documentation and marketing materials.

Thank you, merging.

@xhanulik xhanulik merged commit 1faa516 into OpenSC:master Mar 5, 2024
@alt3r-3go
Copy link
Contributor Author

Thanks everyone!

@alt3r-3go alt3r-3go deleted the 0-25-rc1-nk-test-results branch March 5, 2024 19:30
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 this pull request may close these issues.

4 participants