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

How to handle Ambiguous? #201

Closed
ReactorScram opened this issue Aug 5, 2024 · 20 comments · Fixed by #203
Closed

How to handle Ambiguous? #201

ReactorScram opened this issue Aug 5, 2024 · 20 comments · Fixed by #203
Labels

Comments

@ReactorScram
Copy link
Contributor

From firezone/firezone#6175

I got into a weird situation, my dev VM is returning the Ambiguous error for both get_password and set_password on the current 3.x version.

This doesn't happen when I use the same code on my test VM, so I believe it's a problem with my secret service state in the dev VM, not with my code.

But I'm not sure how I would fix this if it happened on a production system. If delete_credential can also return Ambiguous, does that mean there is no way to repair this incorrect state using only keyring-rs itself? (aside from some workaround like picking a new key name and saving it to disk)

I'm also not sure how I got into this state. I don't run many programs on the dev VM, though I have run multiple builds of the Firezone Client, including builds with the old 2.x keyring-rs library.

So I am hesitant to fix the VM without knowing how to replicate or even fix the issue.

Thoughts?

@ReactorScram ReactorScram changed the title What do if I always get Ambiguous? What to do if I always get Ambiguous? Aug 5, 2024
@ReactorScram ReactorScram changed the title What to do if I always get Ambiguous? How to handle Ambiguous? Aug 5, 2024
@brotskydotcom
Copy link
Collaborator

Hi @ReactorScram, thanks much for reporting this, because I've always wondered whether anyone has ever actually gotten an Ambiguous error in production use :)! The error came about because of changes in the way v1 and v2 work:

  • The v1 keyring's secret service credential store looks for credentials only in the default collection. The v2/v3 keyring's secret service credential store looks for credentials "service-wide," that is, it asks the service to check for credentials with matching attributes regardless of which collection they live in.
  • The v1 keyring did not explicitly add the 'target' attribute when creating a secret-service credential, and the v2/v3 keyrings know this, so if you search for a credential with an explicit target then v2/v3 will find credentials put in by v1 that don't have that attribute as well as credentials put in by v2/v3 that do have that attribute.
  • It is impossible, as far as I know, to get an Ambiguous error using just the keyring unless you have pre-existing v1 credentials in your store that were never "upgraded" to v2/v3 credentials (by doing a v2/v3 set_password or set_secret without a explicit target).

Hearing that it is only your dev machine that is exhibiting this problem does not surprise me. Here's my suggestion on how to proceed if you want to use only the keyring to debug and fix the situation:

  • Use the keyring-cli in verbose mode to print out the Ambiguous error so you can see all the attributes of the different credentials. (Just do an ambiguous keyring-cli -v get). The label attribute will reveal whether you have both v1 and v2/v3 credentials.
  • If that's the problem, you should see an explicit target attribute on the v2/v3 credential that the v1 doesn't have. By specifying that target explicitly, you should be able to unambiguously delete just the v2/v3 credential.
  • Once you've deleted the ambiguous v2/v3 credential, do a set_password on the v1 credential to upgrade it.

Alternatively, and possibly preferentially, I would suggest that you use a UI-based secret service client (typically this is included in your system settings UI) to find the ambiguous credentials and look at their attributes. My guess above may be correct, but it may also be the case that they are both v2/v3 credentials and they are sitting in different collections (one of which used to be the default but then had its collection file renamed to something other than "login"). If that's the case, you will probably have to use the UI tool to fix the issue; the keyring's cross-platform interface doesn't have the juice to deal with platform-specific issues like this (or, frankly, with ambiguity arising from the use of credential managers with other attribute conventions).

Please let me know if this helps, and what you find! As I said at the beginning, I really want to know!

@ReactorScram
Copy link
Contributor Author

ReactorScram commented Aug 6, 2024

Okay! So I've run into a couple strange things already:

  • I forgot to mention, I'm passing blank strings, that might be confusing secret-service. I call keyring::Entry::new_with_target("dev.firezone.client/token", "", "") since the latter two args are ignored on Windows. (I hit this back in docs: document platform-specific behavior on Windows  #152)
  • Running the CLI actually doesn't show any stored password, I used cargo run --features crypto-rust,sync-secret-service,windows-native --release --example keyring-cli -- -v --target "dev.firezone.client/token" --service "" --user "" password which should match the features and arguments used in Firezone. That returns No credential found for '[dev.firezone.client/token]user@'
  • KDE's wallet manager doesn't show any stored credentials either
  • I never actually used keyring = 1.x, I did a git bisect and the first version I used was 2.x back in 2023.
  • I'm on Ubuntu 20.04.5, which I know is 4 years old now, but it's our oldest supported Linux platform for Firezone. Maybe 20.04 just has a busted secret service impl?
  • The dev system still throws Ambiguous when checking on our stored credential
image

@brotskydotcom
Copy link
Collaborator

Wow! Great detective work! You've already found at least one bug... in the CLI! :) Looking at these lines:

if args.user.is_empty() || args.user.eq_ignore_ascii_case("<logged-in username>") {
    args.user = whoami::username()
}

I can see that the CLI is not letting you replicate the API call that firezone is making: notice the additional user@ in the output of the cli. I will fix that right away. In the meantime, in your fork, can you just remove the arg.user.is_empty() || from that line and rebuild; that should at least allow you to repro the issue and get the CLI to show you the ambiguous credentials.

Since firezone is using a target, keyring will be creating a secret-service collection named for the target and storing the credential there. I don't have any experience either with the KWallet or the KDEWalletManager, so I'm not exactly sure how one can use the manager UI to see non-default collections created by the secret service. Looking at this page on the ArchWiki, it seems like looking at the ~/.local/share/kwalletd folder might be useful in figuring out whether KWallet is implementing that collection by creating a different wallet completely, and whether there's a way to get the wallet manager to look at that wallet. Also, per this page, the wallet manager should be able to open a sidebar that displays wallets other than the default. Maybe that will help you to locate the ambiguous credentials?

Please let me know what you find out; I really appreciate you digging into this!

brotskydotcom added a commit to brotskydotcom/keyring-rs that referenced this issue Aug 6, 2024
@ReactorScram
Copy link
Contributor Author

that should at least allow you to repro the issue and get the CLI to show you the ambiguous credentials.

I removed that entire if-statement but now I'm just getting the same output as Firezone:

More than one credential found for '[dev.firezone.client/token]@': [Any { .. }, Any { .. }]

I'm not sure how it's an Any. It should be a trait object implementing Credential, and I should be able to downcast it, right? I don't know enough about runtime typing in Rust to figure it out. But it's not giving me any new debug info. :/

Also I somehow got KWallet and gnome-keyring installed side-by-side, so I used secret-tool search --all target dev.firezone.client/token to search for secrets:

[/org/freedesktop/secrets/collection/dev_2efirezone_2eclient_5ftoken/17]
label = keyring-rs v3.0.4 for target 'dev.firezone.client/token', service '', user ''
secret = [REDACTED]
created = 2024-07-30 15:40:25
modified = 2024-07-30 15:40:25
schema = org.freedesktop.Secret.Generic
attribute.application = rust-keyring
attribute.service = 
attribute.target = dev.firezone.client/token
attribute.username =

But I only see one secret there.

@brotskydotcom
Copy link
Collaborator

Also I somehow got KWallet and gnome-keyring installed side-by-side, so I used secret-tool search --all target dev.firezone.client/token to search for secrets

Using the target as the search attribute is not reliable for finding the duplicate credential, because the duplicate probably doesn't have a target attribute. Try searching against an attribute that you know is identical (and non-empty) between the two, such as the application attribute (always rust-keyring for this crate). So, for example, try secret-tool search --all application rust-keyring to see if that digs up the other one as well.

I'm not sure how it's an Any. It should be a trait object implementing Credential, and I should be able to downcast it, right? I don't know enough about runtime typing in Rust to figure it out.

Wow! Another bug you've found!! The CLI should be down-casting the Any to the correct platform-specific credential type so that it can be debug-printed (which would also provide clients an example of how to do that)! I will fix that right now also!

@ReactorScram
Copy link
Contributor Author

Okay now I'm seeing this from secret-tool search --all --unlock application rust-keyring

[/org/freedesktop/secrets/collection/dev_2efirezone_2eclient_5ftoken/17]
label = keyring-rs v3.0.4 for target 'dev.firezone.client/token', service '', user ''
secret = [REDACTED]
created = 2024-07-30 15:40:25
modified = 2024-07-30 15:40:25
schema = org.freedesktop.Secret.Generic
attribute.application = rust-keyring
attribute.service = 
attribute.target = dev.firezone.client/token
attribute.username = 

[/org/freedesktop/secrets/collection/dev_2efirezone_2eclient_5ftest_5fBYKPFT6P_5ftoken/1]
label = keyring-rs v3.0.3 for target 'dev.firezone.client/test_BYKPFT6P/token', service '', user ''
secret = asdf
created = 2024-07-22 21:18:49
modified = 2024-07-22 21:18:49
schema = org.freedesktop.Secret.Generic
attribute.application = rust-keyring
attribute.service = 
attribute.target = dev.firezone.client/test_BYKPFT6P/token
attribute.username = 

And noticing a couple more oddities:

  • Sometimes when I first see a secret in secret-tool, it won't show me any label or something. Then it shows on later runs, as if there's a caching bug.
  • I deliberately used different targets for the unit tests, so these shouldn't conflict...... unless the forward slash is interpreted in some special way, triggering a bug in keyring or secret-service? In which case I would give up and change my target to a random base32 string :P

@brotskydotcom
Copy link
Collaborator

Hmmmm. Two credentials that have explicit target attributes should not be seen as ambiguous. Yes, keyring does its initial search for credentials by using just the username and service attributes, but then the results of that search are further filtered by the target attribute (if it exists), so if you are specifying an explicit target in the get_password call you should not be getting both credentials back. So I can see two possibilities:

  1. There's a bug in the "explicit target" filtering happening in keyring. I will do some testing right now to check on that.

  2. There's yet another credential in your dev keyring somewhere that has no target attribute, an empty username attribute, and an empty service attribute. To test this, try doing secret-tool search --all --unlock service "" and secret-tool search --all --unlock username "", and see if one of them turns up another credential.

I'm fascinated to hear what you find! Thanks again for pushing on this!

@ReactorScram
Copy link
Contributor Author

I tried those commands and it's just returning the same two secrets. I don't see any obvious bugs in the keyring target filtering code either.

Very strange.

@brotskydotcom
Copy link
Collaborator

OK, we obviously need the CLI to tell us what it's finding, in detail. I'm working on that, stay tuned.

brotskydotcom added a commit to brotskydotcom/keyring-rs that referenced this issue Aug 7, 2024
As reported in hwchen#201, the existing debug print of credentials gives no information about the underlying platform credential. This fixes that.
brotskydotcom added a commit to brotskydotcom/keyring-rs that referenced this issue Aug 7, 2024
As reported in hwchen#201, the existing debug print of credentials gives no information about the underlying platform credential. This fixes that.
brotskydotcom added a commit to brotskydotcom/keyring-rs that referenced this issue Aug 7, 2024
As reported in hwchen#201, the existing debug print of credentials gives no information about the underlying platform credential. This fixes that.
@brotskydotcom
Copy link
Collaborator

OK, please upgrade to v3.1.0 and try the CLI again to see which credentials it thinks are ambiguous. I'm very interested! I'm going to leave this question open until we figure out what's going on with your dev machine.

@brotskydotcom brotskydotcom reopened this Aug 7, 2024
@brotskydotcom brotskydotcom removed the bug label Aug 7, 2024
@ReactorScram
Copy link
Contributor Author

Huh, well it's not saying ambiguous anymore.

cargo run --features crypto-rust,sync-secret-service,windows-native --release --example keyring-cli -- -v --target "dev.firezone.client/token" --service "" --user "" password returns:

[REDACTED STRING #1]
Password for '[dev.firezone.client/token]@' is '[REDACTED STRING#1]'

I checked out the main branch of Firezone, which had been replicating this, and it no longer replicates, it logs in just fine.

Does keyring 3.1 do anything that could possibly repair this on-disk, or did my dev VM just fix itself overnight at random? 🤔

@brotskydotcom
Copy link
Collaborator

I believe your dev VM just fixed itself overnight. Did you log out or reboot or take an update or do anything else that might have caused the secret service to be restarted? Given what you reported yesterday about it sometimes showing you labels and sometimes not, I suspect there may have been cached data that was corrupt and coming back as partial results, and that's why you were getting the Ambiguous error. It's even possible that calls made through the secret-tool kicked it enough that it flushed the corrupt data.

I'm going to close this issue. While I'm happy things are working for you, I'm kind of bummed we didn't get to try out the shiny new diagnostics it caused me to build :). Ah, well, I'll have to be satisfied with my manufactured tests...

Thanks again for reporting this and working it with me. It's a delight for me to have the opportunity for a give and take with a client dev! It's exactly the kind of thing open source makes possible and "my real job" never did!

@ReactorScram
Copy link
Contributor Author

ReactorScram commented Aug 7, 2024

I think I did reboot it, the UTM VMs hang for no reason every now and then, I think it hanged yesterday.

It shows an uptime of 1 day, 1:45 which I guess means 1h45m, so I must have rebooted it yesterday before my last comment where secret-tool was returning something odd. I thought I had replicated the issue in Firezone after that but maybe not.

Yeah, on the Firezone issue I'll chalk it up to "My VM is bad please re-open if this happens to anyone else"

@ReactorScram
Copy link
Contributor Author

It may still just be my dev VM randomly corrupting its own state, but it's happening again. This is at a time when the VM's DNS is also broken, so unrelated proof that something is wrong in my VM.

I restarted the VM hoping to clear any in-memory state that's breaking the DNS and secret service, but even after restarting I'm still getting this in Firezone

(Had to screenshot because copy-paste quit working in UTM as I was making this post)

image

So I doubt it's any fault of keyring but I thought I'd mention it for the record.

As you can see from the error string, we haven't updated to the new keyring. I'll do that in a dev branch and see what happens. Assuming ofc that my VM can find github.com and crates.io...

@ReactorScram
Copy link
Contributor Author

okay well the new keyring isn't on crates and Cargo is telling me that some underlying Windows dep clashes with chrono anyway 🤷‍♀️

I'll come back to this

@brotskydotcom
Copy link
Collaborator

So sorry about the failure to publish 3.1 to crates.io: that's now fixed.

On the Windows side, I took a dependabot-suggested update to the latest Microsoft-supported windows crate as part of 3.1. Let me know if that introduces some conflict; I am willing to roll it back.

@brotskydotcom
Copy link
Collaborator

By the way, if you can get 3.1 working on your dev machine, I would love to see the error output from 3.1 (which will show us the credential details).

@ReactorScram
Copy link
Contributor Author

ReactorScram commented Aug 15, 2024

Yeah 3.1 does work standalone.

Output of cargo run --features crypto-rust,sync-secret-service,windows-native --example keyring-cli -- --target "dev.firezone.client/token" -v -s "" -u "" password is

(Pretty-printed by hand, secrets didn't appear to be included)

More than one credential found for '[dev.firezone.client/token]@': [
    SsCredential { 
        attributes: {
            "gkr:compat:hashed:username": "d41d8cd98f00b204e9800998ecf8427e", 
            "gkr:compat:hashed:service": "d41d8cd98f00b204e9800998ecf8427e", 
            "xdg:schema": "org.freedesktop.Secret.Generic", 
            "gkr:compat:hashed:target": "17d58222bb2f0367a7a27a97ae3e5382", 
            "gkr:compat:hashed:application": "79d05a70f3e1b0a4365c7912ee036a95"
        }, 
        label: "", 
        target: None 
    }, 
    SsCredential { 
        attributes: {
            "xdg:schema": "org.freedesktop.Secret.Generic", 
            "gkr:compat:hashed:target": "a6e156c65c6804d9bdb95f480310ea36", 
            "gkr:compat:hashed:application": "79d05a70f3e1b0a4365c7912ee036a95", 
            "gkr:compat:hashed:service": "d41d8cd98f00b204e9800998ecf8427e", 
            "gkr:compat:hashed:username": "d41d8cd98f00b204e9800998ecf8427e"
        }, 
        label: "",
        target: None
    }
]

Here they are in sorted order:

  • gkr:compat:hashed:application - 79d05a70f3e1b0a4365c7912ee036a95 (Set by keyring to a const IIRC)
  • gkr:compat:hashed:service - d41d8cd98f00b204e9800998ecf8427e
  • gkr:compat:hashed:target - Varies
  • gkr:compat:hashed:username - d41d8cd98f00b204e9800998ecf8427e (must be the hash of the blank string, with no salt or pepper?)
  • xdg:schema - org.freedesktop.Secret.Generic

So how are they ambiguous if the target actually does vary?

@brotskydotcom
Copy link
Collaborator

brotskydotcom commented Aug 15, 2024

OK, now we are getting somewhere!

  1. Yes, you're right, the secrets are intentionally omitted. They aren't actually fetched in the ambiguous case.
  2. The fact that there's no label on either of those indicates that neither of them was actually written by keyring. Even in v1 keyring always put a label on every credential it created. In fact no secret service client is ever supposed to create an entry with no label: the spec requires a label (although I don't recall whether it can be empty).
  3. Keyring is showing you that neither of those credentials has a target. None of those attributes prefixed with gkr:compat:hashed: mean anything to keyring.
  4. I've done some research and figured out that those attributes are related to this fixed bug in the Gnome keyring code. They were first reported in this bug.

Here's my current thinking about what's going on:

  • When you do a search for a specific target and an empty username and service, the keyring does a search just on the empty username and service. Apparently, this search is returning credentials that don't even have a username or service attribute, which surprises me. Since I'm surprised, so is the keyring :), and it doesn't know to filter them out of the search results. I think that's a bug, and it's easy to fix, so I have reported it as Keyring doesn't know how to disambiguate elements from locked collections. #204 and will fix it.
  • Since neither of those credentials has a target attribute, they aren't filtered out of the search in the post-processing pass (to maintain compatibility with keyring v1). The result is that they are considered ambiguous.
  • I suspect that both of those credentials are left from long, long ago tests (or debugging sessions) that used the buggy Gnome secret-service implementation. In any event, it should be completely safe to delete them. But if you would be willing to leave them around until I can release the fix for Keyring doesn't know how to disambiguate elements from locked collections. #204, I would be very grateful, because then we would have a real-world test case.

@brotskydotcom
Copy link
Collaborator

OK, with a bunch more research, I think I know exactly what's happening on your dev machine. Please see the comments on #204 for a reproduced example. I am thinking about the right way to handle this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants