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

Update credential support #425

Merged
merged 2 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 66 additions & 1 deletion pynitrokey/cli/nk3/secrets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
73 changes: 63 additions & 10 deletions pynitrokey/nk3/secrets_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ class Instruction(Enum):
SetPIN = 0xB4
GetCredential = 0xB5
RenameCredential = 0xB6
UpdateCredential = 0xB7


class Tag(Enum):
Expand Down Expand Up @@ -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:
"""
Expand Down Expand Up @@ -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")
)
Expand All @@ -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`.
Expand Down
86 changes: 86 additions & 0 deletions pynitrokey/test_secrets_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 "")
Expand Down Expand Up @@ -1743,3 +1748,84 @@ 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 with empty strings, and rename again
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 is None
assert c.password is None
assert c.metadata is None

# Disallow to register a PWS credential with any 0-length strings field
app.verify_pin_raw(PIN)
with pytest.raises(SecretsAppException, match="IncorrectDataParameter"):
app.register(
CREDID2,
SECRET,
DIGITS,
kind=Kind.Hotp,
login=b"",
password=b"",
metadata=b"",
)
for i in ["login", "password", "metadata"]:
fields = {i: b""}
with pytest.raises(SecretsAppException, match="IncorrectDataParameter"):
app.register(CREDID2, SECRET, DIGITS, kind=Kind.Hotp, **fields)