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

Add PIV commands #506

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Add PIV commands #506

wants to merge 13 commits into from

Conversation

sosthene-nitrokey
Copy link
Contributor

This PR adds PIV commands under nitropy nk3 piv

Checklist

Make sure to run make check and make fix before creating a PR, otherwise the CI will fail.

  • tested with Python3.9
  • [x[ signed commits
  • updated documentation (e.g. parameter description, inline doc, docs.nitrokey)
  • added labels

If we merge this before PIV is stabilised, this will need to be behind a --experimental flag.

@sosthene-nitrokey
Copy link
Contributor Author

Not sure how to deal with the modules that don't have type definitions.

How can I just ignore the errors for pyscard and asn1crypto? I don't think we can get rid of those, or add type checks.

ber-tlv is smaller and might be worth replacing with something homemade, we might want to avoid having a dependency as small as this one.

@robin-nitrokey
Copy link
Member

Generally, there are two solutions to dependencies without type annotations:

  1. manually add the annotations for those parts of the public API that we use in a stub
  2. just ignore the errors

Obviously, the first option is much better, but not always worth the effort, especially for legacy code. See the stubs folder for an example for (1), and the libraries without annotations section in pyproject.toml for (2).

Also make sure to check the typeshed repository. Maybe somebody already wrote a stub.

@tgahlx
Copy link

tgahlx commented Apr 11, 2024

I've found a bug testing this branch with the 1.6.0-test firmware

~/Projekte/nitro-ca-test ❯/home/tga/Projekte/pynitrokey/venv/bin/nitropy nk3 piv write-certificate --format PEM --key 9C --path ./jd-c.crt                                                                                                                                                                          17:05:25
Command line tool to interact with Nitrokey devices 0.4.45
Usage: nitropy nk3 piv write-certificate [OPTIONS] [ADMIN_KEY]
Try 'nitropy nk3 piv write-certificate --help' for help.

Error: Invalid value for '--key': '9C' is not one of '9A', ' 9C', ' 9D', ' 9E', ' 82', ' 83', ' 84', ' 85', ' 86', ' 87', ' 88', ' 89', ' 8A', ' 8B', ' 8C', ' 8D', ' 8E', ' 8F', ' 90', ' 91', ' 92', ' 93', ' 94', ' 95'.
~/Projekte/nitro-ca-test ❯ /home/tga/Projekte/pynitrokey/venv/bin/nitropy nk3 piv write-certificate --format PEM --key ' 9C' --path ./jd-c.crt                                                                                                                                                                       17:05:38
Command line tool to interact with Nitrokey devices 0.4.45
Critical error:
An unhandled exception occurred
	Exception encountered: KeyError(' 9C')

@sosthene-nitrokey sosthene-nitrokey changed the title App PIV commands Add PIV commands Apr 12, 2024
@sosthene-nitrokey
Copy link
Contributor Author

Thank you for the report.

It was a very simple typo, fixed in d5bc2af

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.

3 participants