Secrets App Password Safe support for Nitrokey 3

szszszsz commented 1 year ago

This PR adds support for the new Password Safe feature of Secrets App

Changes

Add Password Safe API to Secrets App
CLI changes:
- OTP secret is now an optional named argument: --secret (was positional)
- Register command now accepts PWS fields
- New command: get-password - get the PWS fields from the Credential
- List command now shows the Credential's type
API change: for OTP Credentials Kind has no longer a default type
Add tests
Allow to create APDUs longer than 256 bytes
Backwards compatibility with the current stable 1.3.1:
- Register command will be rejected with PWS fields, or with the Kind not set
- Get-Credential command will be rejected as unknown one

Discussion

To discuss:

[x] how to present passwords to user (could be improved in the next PR)
[x] is proper user-informing message needed for the users with the older firmware / secrets app version

Checklist

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

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

Test Environment and Execution

OS: Linux Fedora 37
device's model: USB/IP Sim
device's firmware version: Secrets App 0.10.0-3-g49d772eb

Relevant Output Example

Welcome to fish, the friendly interactive shell
Type help for instructions on how to use fish
~/w/pynitrokey (secrets-pws|✔) $ ./venv/bin/nitropy nk3 secrets register --help
Command line tool to interact with Nitrokey devices 0.4.36
Usage: nitropy nk3 secrets register [OPTIONS] NAME

  Register OTP/Password Safe Credential.

  Write Credential under the NAME.

Options:
  --secret TEXT                   The shared secret string (encoded in base32,
                                  e.g. AAAAAAAA)
  --digits-str [6|8]              Digits count
  --kind [HOTP|TOTP|HOTP_REVERSE|NOT_SET]
                                  OTP mechanism to use. Case insensitive.
  --hash [SHA1|SHA256]            Hash algorithm to use
  --counter-start INTEGER         Starting value for the counter (HOTP only)
  --touch-button                  This credential requires button press before
                                  use
  --protect-with-pin              This credential should be additionally
                                  encrypted with a PIN, which will be required
                                  before each use
  --login TEXT                    Password Safe Login
  --password TEXT                 Password Safe Password
  --metadata TEXT                 Password Safe Metadata - additional field,
                                  to which extra information can be encoded in
                                  the future
  --help                          Show this message and exit.
~/w/pynitrokey (secrets-pws|✔) $ ./venv/bin/nitropy nk3 secrets register
Command line tool to interact with Nitrokey devices 0.4.36
Usage: nitropy nk3 secrets register [OPTIONS] NAME
Try 'nitropy nk3 secrets register --help' for help.

Error: Missing argument 'NAME'.
~/w/pynitrokey (secrets-pws|✔) [2]$ ./venv/bin/nitropy nk3 secrets
Command line tool to interact with Nitrokey devices 0.4.36
Usage: nitropy nk3 secrets [OPTIONS] COMMAND [ARGS]...

  Nitrokey Secrets App. Manage OTP secrets on the device. Use
  NITROPY_SECRETS_PASSWORD to pass password for the scripted execution.

Options:
  --help  Show this message and exit.

Commands:
  get           Generate OTP code from registered credential.
  get-password  Get Password Safe Entry
  list          List registered OTP credentials.
  register      Register OTP/Password Safe Credential.
  remove        Remove OTP credential.
  reset         Remove all OTP credentials from the device.
  set-pin       Set or change the PIN used to authenticate to other...
  status        Show application status
  verify        Proceed with the incoming OTP code verification (aka...
~/w/pynitrokey (secrets-pws|✔) $ ./venv/bin/nitropy nk3 secrets get-password --help
Command line tool to interact with Nitrokey devices 0.4.36
Usage: nitropy nk3 secrets get-password [OPTIONS] NAME

  Get Password Safe Entry

Options:
  --help  Show this message and exit.
~/w/pynitrokey (secrets-pws|✔) $ ./venv/bin/nitropy nk3 secrets list
Command line tool to interact with Nitrokey devices 0.4.36
Please provide PIN to show PIN-protected entries (if any), or press ENTER to skip
Please touch the device if it blinks
Current PIN (8 attempts left):
Please touch the device if it blinks
PWS  : CRED ID
~/w/pynitrokey (secrets-pws|✔) $ ./venv/bin/nitropy nk3 secrets get-password "CRED ID"
Command line tool to interact with Nitrokey devices 0.4.36
Please touch the device if it blinks
Credential not found. Please provide PIN below to search in the PIN-protected database.
Current PIN (8 attempts left):
Please touch the device if it blinks
login               : =======login========
password            : ======password======
metadata            : ======metadata======
properties          : 41
name                : CRED ID
~/w/pynitrokey (secrets-pws|✔) $

Connected:

https://github.com/Nitrokey/trussed-secrets-app/pull/63

szszszsz commented 1 year ago

@robin-nitrokey Can you take a look at the Discussion chapter as well?

szszszsz commented 1 year ago

I think it would be more intuitive if there were separate commands for adding an OTP and adding a static password. Otherwise it is hard to figure out which arguments are valid or required in which case. What do you think?

Makes sense. The whole Credential could be made in 2 steps, with 2 appropriate register calls, where the second would update the existing one instead of overwriting it.

Do you have names propositions? I understand we would leave register to keep the user API changes count down.

robin-nitrokey commented 1 year ago

Do you have names propositions? I understand we would leave register to keep the user API changes count down.

Maybe add-otp and add-password? register could act as an alias for add-otp for compatibility and print a deprecation warning. In that case I’d also use get-otp and get-password with get being an alias for get-otp.

robin-nitrokey commented 1 year ago

how to present passwords to user (could be improved in the next PR)

The current approach looks good to me, but I think there should be additional options:

It should be possible to only print the password for easy scripting.
A machine-readable output format could also be useful for more advanced use cases (CSV or JSON).

is proper user-informing message needed for the users with the older firmware / secrets app version

Yes, I think that would be good. It should be pretty easy to check the firmware version and cache it on device level. We should probably have this as a default for all functionality we add.

szszszsz commented 1 year ago

Sounds good! I do not see that as a blocking for merging. I think it would be worth to work on the propositions in separate tickets:

change command names (improve UI)
additional output presentation
update credential feature
check firmware / app version for the PWS features

szszszsz commented 1 year ago

Sure, it can wait. I do not have any particular need to push it out in the first coming release.

szszszsz commented 1 year ago

@robin-nitrokey I presume this could be merged now, right?

Nitrokey / pynitrokey