Closed rgerganov closed 4 years ago
apparently if an rp is too dumb to set a name for the credential (like it is possible in my sandbox) this thing apparently breaks upon running list:
Relying Party Username Credential ID
-----------------------------------------------------
Traceback (most recent call last):
File "/home/my1/.local/bin/solo", line 11, in <module>
sys.exit(solo_cli())
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 829, in __call__
return self.main(*args, **kwargs)
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 782, in main
rv = self.invoke(ctx)
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 1259, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 1259, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 1259, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 1066, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 610, in invoke
return callback(*args, **kwargs)
File "/home/my1/.local/lib/python3.6/site-packages/solo/cli/key.py", line 525, in cred_ls
print("{:20}{:20}{}".format(rp_id, user['name'], cred_id_b64))
(line numbers not accurate as i have this and the file signing PR both monkeypatched in there)
Thanks for the report, will update the patch soon
bonus tip: if #76 passes you can change the top few lines of every command's PIN request to this:
client = solo.client.find(serial, udp=udp)
if client.has_pin() is False:
print ("Credential Management needs to have a PIN set up, please Set a PIN.")
sys.exit(1)
if not pin:
pin = getpass.getpass("PIN: ")
(and drop the client.find that happens later)
to have users get a pretty error if a PIN isnt set yet
bonus error if no RKs stored:
my1@my1-qb:~$ solo key credential list
PIN:
Traceback (most recent call last):
File "/home/my1/.local/bin/solo", line 11, in <module>
sys.exit(solo_cli())
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 829, in __call__
return self.main(*args, **kwargs)
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 782, in main
rv = self.invoke(ctx)
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 1259, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 1259, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 1259, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 1066, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 610, in invoke
return callback(*args, **kwargs)
File "/home/my1/.local/lib/python3.6/site-packages/solo/cli/key.py", line 527, in cred_ls
rps = cm.enumerate_rps()
File "/home/my1/.local/lib/python3.6/site-packages/fido2/ctap2.py", line 1071, in enumerate_rps
n_rps = first[CredentialManagement.RESULT.TOTAL_RPS]
TypeError: 'NoneType' object is not subscriptable
as if stuff hadnt been crazy enough, it gets weirder.
Just Updated my Somu Secure (so sadly no somu-side debug) from 3.1.1 to 4 and it's weird.
getting RK info tells that it has zero keys stored, yet when actually attempting to use an rk it totally lets you do so. I cant check whether that given RK works or not as it was probably before I moved hosting so I dont have the old db with me, but it is weird none the less.
totally forgot the important part: using list yields the following:
my1@my1-qb:~$ solo key credential list
PIN:
Traceback (most recent call last):
File "/home/my1/.local/bin/solo", line 11, in <module>
sys.exit(solo_cli())
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 829, in __call__
return self.main(*args, **kwargs)
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 782, in main
rv = self.invoke(ctx)
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 1259, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 1259, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 1259, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 1066, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/home/my1/.local/lib/python3.6/site-packages/click/core.py", line 610, in invoke
return callback(*args, **kwargs)
File "/home/my1/.local/lib/python3.6/site-packages/solo/cli/key.py", line 527, in cred_ls
rps = cm.enumerate_rps()
File "/home/my1/.local/lib/python3.6/site-packages/fido2/ctap2.py", line 1074, in enumerate_rps
rest = [self.enumerate_rps_next() for _ in range(1, n_rps)]
File "/home/my1/.local/lib/python3.6/site-packages/fido2/ctap2.py", line 1074, in <listcomp>
rest = [self.enumerate_rps_next() for _ in range(1, n_rps)]
File "/home/my1/.local/lib/python3.6/site-packages/fido2/ctap2.py", line 1063, in enumerate_rps_next
return self._call(CredentialManagement.CMD.ENUMERATE_RPS_NEXT, auth=False)
File "/home/my1/.local/lib/python3.6/site-packages/fido2/ctap2.py", line 1032, in _call
return self.ctap.credential_mgmt(**kwargs)
File "/home/my1/.local/lib/python3.6/site-packages/fido2/ctap2.py", line 817, in credential_mgmt
args(sub_cmd, sub_cmd_params, pin_protocol, pin_auth),
File "/home/my1/.local/lib/python3.6/site-packages/fido2/ctap2.py", line 650, in send_cbor
expected = cbor.encode(cbor.decode(enc))
File "/home/my1/.local/lib/python3.6/site-packages/fido2/cbor.py", line 171, in decode
value, rest = decode_from(data)
File "/home/my1/.local/lib/python3.6/site-packages/fido2/cbor.py", line 167, in decode_from
return _DESERIALIZERS[fb >> 5](fb & 0b11111, data[1:])
File "/home/my1/.local/lib/python3.6/site-packages/fido2/cbor.py", line 149, in load_map
v, data = decode_from(data)
File "/home/my1/.local/lib/python3.6/site-packages/fido2/cbor.py", line 167, in decode_from
return _DESERIALIZERS[fb >> 5](fb & 0b11111, data[1:])
File "/home/my1/.local/lib/python3.6/site-packages/fido2/cbor.py", line 149, in load_map
v, data = decode_from(data)
File "/home/my1/.local/lib/python3.6/site-packages/fido2/cbor.py", line 167, in decode_from
return _DESERIALIZERS[fb >> 5](fb & 0b11111, data[1:])
File "/home/my1/.local/lib/python3.6/site-packages/fido2/cbor.py", line 132, in load_text
return enc.decode("utf8"), rest
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xb3 in position 15: invalid start byte
Just Updated my Somu Secure (so sadly no somu-side debug) from 3.1.1 to 4 and it's weird.
I think this is expected as we changed how RKs are represented in versions >= 3.2.0
maybe it might be useful then to either grill RKs or maybe if possible fix the RKs stored some way.
ideally without resetting the key in its entirety, as apparently adding new RKs doesnt fix the issue, and you can really delete old ones without pinpointing the old ones
@My1 the reason we yanked release 3.2.0 (and instead released 4.x) is exactly because it's a breaking change (as signaled by major version bump).
Sure but it's kinda breaking in a really weird way and maybe it might be nice to have it break cleaner (like by grilling rks, ideally without wiping the entire solo) and not everyone is gonna read changelogs.
Yeah, I think we should actually prevent major updates on non-reset state keys (https://github.com/solokeys/solo-python/issues/77 is relevant). However we can't add that on existing keys...
well while not possible on device level the client could give a confirmation.
although ideally if a breaking change doesnt need a full reset it would be FAR better in my opinion, and if possible to avoid changes by adapting the changes, even better.
Let's please stop derailing review of the PR at hand.
okay I'll split this into an issue
but on topic, feature detection might be useful (as GetInfo shows credMgmt or whatever it was on Fido2 devices with this feature)
Ping. Does this require more work?
you could throw in an error if 1) the device doesnt show the feature (e.g. solo outdated) 2) there is no PIN
Issue: #68