-
Notifications
You must be signed in to change notification settings - Fork 69
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 commands for credential management #71
Conversation
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:
(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: 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:
|
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:
|
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 (#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
|
Issue: #68