diff --git a/pynitrokey/cli/nk3/secrets.py b/pynitrokey/cli/nk3/secrets.py index 44c2a1c3..2644bb39 100644 --- a/pynitrokey/cli/nk3/secrets.py +++ b/pynitrokey/cli/nk3/secrets.py @@ -107,6 +107,71 @@ def call(app: SecretsApp) -> None: local_print("Done") +@secrets.command() +@click.pass_obj +@click.argument( + "name", + type=click.STRING, +) +@click.option( + "--login", + "login", + type=click.STRING, + help="Password Safe Login", + default=None, +) +@click.option( + "--password", + "password", + type=click.STRING, + help="Password Safe Password", + default=None, +) +@click.option( + "--metadata", + "metadata", + type=click.STRING, + help="Password Safe Metadata - additional field, to which extra information can be encoded", + default=None, +) +@click.option( + "--touch-button", + "touch_button", + type=click.BOOL, + help="Activate/deactivate touch button requirement", + default=None, +) +def update( + ctx: Context, + name: str, + new_name: Optional[bytes] = None, + login: Optional[bytes] = None, + password: Optional[bytes] = None, + metadata: Optional[bytes] = None, + touch_button: Optional[bool] = None, +) -> None: + """ + Update credential. Change Static Password fields, or touch button requirement attribute. + """ + with ctx.connect_device() as device: + app = SecretsApp(device) + ask_to_touch_if_needed() + + @repeat_if_pin_needed + def call(app: SecretsApp) -> None: + app.update_credential( + name.encode(), + new_name=new_name, + login=login, + password=password, + metadata=metadata, + touch_button=touch_button, + ) + + call(app) + local_print("Done") + + @secrets.command(aliases=["register"]) @click.pass_obj @click.argument( @@ -242,7 +307,7 @@ def call(app: SecretsApp) -> None: "--metadata", "metadata", type=click.STRING, - help="Password Safe Metadata - additional field, to which extra information can be encoded in the future", + help="Password Safe Metadata - additional field, to which extra information can be encoded", default=None, ) def add_password( diff --git a/pynitrokey/nk3/secrets_app.py b/pynitrokey/nk3/secrets_app.py index 2cc05b10..b3e9da4e 100644 --- a/pynitrokey/nk3/secrets_app.py +++ b/pynitrokey/nk3/secrets_app.py @@ -217,6 +217,7 @@ class Instruction(Enum): SetPIN = 0xB4 GetCredential = 0xB5 RenameCredential = 0xB6 + UpdateCredential = 0xB7 class Tag(Enum): @@ -489,12 +490,51 @@ def get_credential(self, cred_id: bytes) -> PasswordSafeEntry: p.properties = p.properties.hex().encode() if p.properties else None return p - def rename_credential(self, cred_id: bytes, cred_new_id: bytes) -> None: + def rename_credential(self, cred_id: bytes, new_name: bytes) -> None: + """ + Rename credential. + An alias for the update_credential() call. + @param cred_id: The credential ID to modify + @param new_name: New ID for the credential + """ + return self.update_credential(cred_id, new_name) + + def update_credential( + self, + cred_id: bytes, + new_name: Optional[bytes] = None, + login: Optional[bytes] = None, + password: Optional[bytes] = None, + metadata: Optional[bytes] = None, + touch_button: Optional[bool] = None, + ) -> None: + """ + Update credential fields - name, attributes, and PWS fields. + Unpopulated fields will not be encoded and used during the update process + (won't change the current value). + @param cred_id: The credential ID to modify + @param new_name: New ID for the credential + @param login: New login field content + @param password: New password field content + @param metadata: New metadata field content + @param touch_button: Set if the touch button use should be required + """ structure = [ tlv8.Entry(Tag.CredentialId.value, cred_id), - tlv8.Entry(Tag.CredentialId.value, cred_new_id), + tlv8.Entry(Tag.CredentialId.value, new_name) if new_name else None, + self.encode_properties_to_send(touch_button, False, tlv=True) + if touch_button is not None + else None, + tlv8.Entry(Tag.PwsLogin.value, login) if login is not None else None, + tlv8.Entry(Tag.PwsPassword.value, password) + if password is not None + else None, + tlv8.Entry(Tag.PwsMetadata.value, metadata) + if metadata is not None + else None, ] - self._send_receive(Instruction.RenameCredential, structure=structure) + structure = list(filter(lambda x: x is not None, structure)) + self._send_receive(Instruction.UpdateCredential, structure=structure) def delete(self, cred_id: bytes) -> None: """ @@ -566,13 +606,7 @@ def register( tlv8.Entry( Tag.Key.value, bytes([kind.value | algo.value, digits]) + secret ), - RawBytes( - [ - Tag.Properties.value, - (0x02 if touch_button_required else 0x00) - | (0x04 if pin_based_encryption else 0x00), - ] - ), + self.encode_properties_to_send(touch_button_required, pin_based_encryption), tlv8.Entry( Tag.InitialCounter.value, initial_counter_value.to_bytes(4, "big") ) @@ -585,6 +619,25 @@ def register( structure = list(filter(lambda x: x is not None, structure)) self._send_receive(Instruction.Put, structure) + @classmethod + def encode_properties_to_send( + cls, touch_button_required: bool, pin_based_encryption: bool, tlv: bool = False + ) -> RawBytes: + """ + Encode properties structure into a single byte + @param touch_button_required: whether the touch button use is required + @param pin_based_encryption: whether the PIN-encryption is requested (only during registration) + @param tlv: set True, if this should be encoded as TLV, as opposed to the default "TV", w/o L + """ + structure = [ + Tag.Properties.value, + 1 if tlv else None, + (0x02 if touch_button_required else 0x00) + | (0x04 if pin_based_encryption else 0x00), + ] + structure = list(filter(lambda x: x is not None, structure)) + return RawBytes(structure) + def calculate(self, cred_id: bytes, challenge: Optional[int] = None) -> bytes: """ Calculate the OTP code for the credential named `cred_id`, and with challenge `challenge`. diff --git a/pynitrokey/test_secrets_app.py b/pynitrokey/test_secrets_app.py index e972c5bf..bc032353 100644 --- a/pynitrokey/test_secrets_app.py +++ b/pynitrokey/test_secrets_app.py @@ -1703,6 +1703,11 @@ def test_rename_credential(secretsAppResetLogin): assert len(l) == 1 assert l[0].decode() == CREDID2 + # Old name should not be accessible + app.verify_pin_raw(PIN) + with pytest.raises(SecretsAppException, match="NotFound"): + app.get_credential(CREDID) + @pytest.mark.parametrize( "cred1_encryption", [True, False], ids=lambda x: "cred1" + ("_enc" if x else "") @@ -1743,3 +1748,69 @@ def test_rename_credential_to_existing( app.verify_pin_raw(PIN) # There should be still 2 credentials left assert set([CREDID.encode(), CREDID2.encode()]) == set(app.list()) + + +def test_update_credential(secretsAppResetLogin): + """ + Credential should change its properties. Test both PIN- and HW-encrypted credentials. + """ + app = secretsAppResetLogin + app.register(CREDID, SECRET, DIGITS, kind=Kind.Hotp) + app.verify_pin_raw(PIN) + l = app.list_with_properties() + assert len(l) == 1 + assert l[0].label.decode() == CREDID + assert not l[0].properties.touch_required + app.verify_pin_raw(PIN) + app.update_credential(CREDID, touch_button=True) + # There should be only one credential left, with the same name + app.verify_pin_raw(PIN) + l = app.list_with_properties() + assert len(l) == 1 + assert l[0].label.decode() == CREDID + # This credential should be listed now as requiring a touch button for use + assert l[0].properties.touch_required + + # Now add some PWS fields, and rename it too + app.verify_pin_raw(PIN) + c = app.get_credential(CREDID) + assert not c.login + assert not c.password + assert not c.metadata + app.verify_pin_raw(PIN) + app.update_credential( + CREDID, + new_name=CREDID2, + login=b"login", + password=b"password", + metadata=b"metadata", + ) + + # Check if PWS fields are there, and the "touch button required" flag is still present + app.verify_pin_raw(PIN) + c = app.get_credential(CREDID2) + assert c.login == b"login" + assert c.password == b"password" + assert c.metadata == b"metadata" + app.verify_pin_raw(PIN) + l = app.list_with_properties() + assert len(l) == 1 + assert l[0].properties.touch_required + + # Old name should not be accessible + app.verify_pin_raw(PIN) + with pytest.raises(SecretsAppException, match="NotFound"): + app.get_credential(CREDID) + + # Try to remove the PWS data, and rename again + # It's not possible currently to remove the PWS fields; hence we need to overwrite them + # with a single blank character. + app.verify_pin_raw(PIN) + app.update_credential( + CREDID2, new_name=CREDID, login=b" ", password=b" ", metadata=b" " + ) + app.verify_pin_raw(PIN) + c = app.get_credential(CREDID) + assert c.login == b" " + assert c.password == b" " + assert c.metadata == b" "