Search feature

[wiimmers]

Basic searching implementation for all platforms and listing the results as a string.

Windows platform has no search functionality built into the windows_sys crate, it uses a custom regex search after getting all credentials.

Secret Service, Keyutils, and System Framework have built in search features that were used and implemented into the basic search structure Search. Some more work could be done to get all credentials in the platform specific store so users could perform their own custom search on these stores with custom regex, as well as some functionality to take a search result and create an Entry from the result for easy password updating.

---

Search::new() initializes the default platform's search structure, from here the by method can be called to then take a by argument and a query argument. Each platform's by parameter specifies what is and isn't allowed.

• Windows: "user", "service", "target"
• Mac/iOS: "account", "service", "label"
• Keyutils: "thread", "process", "session", "user", "user session", "group" (although the crate currently only has options to create entries in the session keyring, thus the test only reflects searching here)
• Secret Service: Because of the way credentials are stored in secret service (as a HashMap) the search is a bit more liberal, the by parameter can take any string slice as the attribute keys can differ by app.

---

List and Limit are used to create a string of the credentials formatted from the search results.

---

A new error type SearchError(String) was created to handle errors for this functionality.

---

let result = Search::new()
.expect("Failed to create new search")
.by("session", "keyring-rs:test-user@test-service");
let list = List::list_credentials(result, Limit::All)
.expect("Failed to parse search results to string);

[brotskydotcom]

Hi @wiimmers, thanks so much for taking a run at this feature! It's going to be about a week before I can really sit down and review this PR properly. In the meantime, can you please do two things to get this ready for review?

1. Please run cargo fmt and cargo clippy -- -D warnings over your code, and make sure the (now enabled) workflow checks pass on your updated PR.
2. Can you please reference this pull in a new comment on issue Entry listing and searching #144, to make others aware of it, in case they want to take a look?

Thanks again, and I'll get back to you next week once I've had a chance to play with this and review it.

[wiimmers]

Having issues with the keyutils test, currently. When run traditionally, I am seeing correct results. However, in the test returns an empty result. When testing with an already created credential instead of the generated one, the result is still empty. Tried a few different methods to try and diagnose the issue. I believe it was working when I initially submitted the PR, but could be wrong. Will try to do some research in the coming days but my schedule will be pretty busy until next week as well.

[brotskydotcom]

Are you seeing the test issue on more than one platform? Since each platform requires its own setup (Windows you have to be physically on the machine, Linux headless you have to run the special shell script) I often find that platform-specific test errors are actually platform errors.

Also: I notice that the CI tests are failing on one of the original tests on Windows. Perhaps something changed in that code or one of the dependencies?

[brotskydotcom]

@wiimmers Apparently Rust 1.78 introduced a change in the way slices are made from raw bytes. This is the underlying cause of #170. I am fixing this and re-releasing, so you will need to rebase your changes on top of the new master branch. Also this may have been the source of the test errors you were seeing?

[wiimmers]

Tests pass now, although it's just a bandaid for Keyutils. Built the search manually instead of using new. Same idea but Search isn't wrapped in a Result. Not sure if that's what is causing the unexpected result, although it is strange because no result should throw a KeyDoesNotExist linux-keyutils error and not just an empty map.

let result = Search {
    inner: Box::new(super::KeyutilsCredentialSearch {}),
}
    .by("session", &query);

[reyamir]

Hi. Any updates on this PR?
My app is required this feature

[brotskydotcom]

I have been trying to get to this. I am hoping to do the review this week.

[brotskydotcom]

Hi @wiimmers! Thanks again for all this work!

I have now been able to carefully read all your code. I haven't done any testing yet, because there is a very high-level issue I'd like to clarify with you before going any further.

This PR currently looks more to me like the basis of a separate crate rather than an extension of the current keyring crate, for these reasons (none of which are criticisms, mind you):

1. The current keyring functionality is quite restricted in terms of the types of credentials it can create or access, but your search functionality is impressively broad (in fact, exhaustive) in terms of the types of credentials that might come back in a search.

2. The search results don't include credentials (even for those results that might be accessible), and give no indication of which of the returned results might be accessible as credentials.

3. This create's credential API is intentionally platform-generic except for the optional target value, allowing clients to write cross-platform code. The PR's search API is very platform-specific in terms of meaningful values for the required "by" parameter, making it hard for clients to write cross-platform code.

4. None of this PR's search code relies on this crate's credential or keystore implementations. A few of this crate's types are used, but those are exported and could easily be used by a separate crate that does search.

5. There is no implementation provided for the mock credential store, which is required to use this code on platforms that don't support the in-built keystores (such as BSD). Providing a meaningful search would require extending the mock credential store data to support it.

Given the above, my first thought is that a terrific contribution to the "keyring community of users" would be for you to create a separate crate - perhaps called something like keyring-search - that folks could use in conjunction with (or even separate from) this crate.

My second thought is that, if you really want search to be a part of this crate, that you restrict the search capabilities to use the generic credential parameters (target, service, and user, all optional), and that you restrict the results to be those that are credentials accessible via the credential API. That would allow you to greatly simplify your search API by having it return credentials (created for the results) rather than bi-level hash maps, and it would allow the search API to be part of the platform-indepenent pluggable keystore API that all the built-in implementations conform to (and thus usable in a platform-independent way).

I want to reiterate that none of these thoughts imply any criticism of this contribution. I think this code is of great value to the keyring user community and has been eagerly anticipated by many. Regardless of whether you go the direction of separate crate or restricted search - or perhaps both! - I am very grateful for all this hard work.

[wiimmers]

Hey @brotskydotcom, thanks for the feedback! I am ecstatic to have been able to help with this project. I will start getting to work on some next steps sometime this week. I will keep you updated on what I eventually do - but I like the idea of a separate crate AND some restricted searching for this crate. Initially, I had a blank canvas and a broad vision for this feature, so your feedback is valuable moving forward. With this in mind for keyring-rs and how to better work with its vision, I have a clear idea of what it should look like.

I appreciate the kind words and would like to say I have had a blast diving into this over the past few weeks. My initial thought is I would like to make this a separate crate first, then move to provide something more in line with what this crate is trying to accomplish. The restrictions made attempting to make this fit keyring-rs could be lifted as a separate crate for an exhaustive search and left to the user's discretion, which I like the idea of.

[brotskydotcom]

Hi @wiimmers glad to hear this has been fun for you! I think the idea of doing a separate crate first and then narrowing the functionality to be keyring specific is wonderful. I will be happy to help by contributing search to the mock keystore if/when you get to that point.

[brotskydotcom]

See overall review discussion in PR. Let me know if you want me to leave this PR open while you figure out next steps. I look forward to seeing what comes next.

[wiimmers]

I have created a new repo named keyring-search if anyone wants to keep up with progress. My work machine is Windows so I'm only sure of this platform working. Will have to properly write tests and do a little more digging for FreeBSD and OpenBSD support with the mock module but should be relatively painless to get what is needed for a broad search feature in a separate library.

@brotskydotcom if you'd like to keep this PR open for the time being, I don't think that it should take long to get a separate library off the ground before I can take a look at this. A few tweaks and some help with the mock keystore should allow for this to feature to come to fruition!

[wiimmers]

keyring-search is on crates.io see #144

[brotskydotcom]

I am going to close the PR now that @wiimmers has released this code as a separate crate.