shared: fix keyring handling in ask-password-api#6145
shared: fix keyring handling in ask-password-api#6145eworm-de wants to merge 1 commit intosystemd:masterfrom
Conversation
|
This deals with #5522 , at least parts of it. |
0d84cb7 to
7890fcc
Compare
|
Any news or comments on this one? |
88a08c1 to
065ceaa
Compare
|
Oh, a simple rebase makes the CI tests fail? What's wrong here? |
Commit 74dd6b5 (core: run each system service with a fresh session keyring) broke keyring handling in ask-password. The key was added to user keyring, but permissions denied to access it. So instead add the key to session keyring, set permission, link to user keyring and unlink from session keyring. For whatever reason requesting a key does not longer work. Search instead.
|
Rebased again, CI tests pass again. |
poettering
left a comment
There was a problem hiding this comment.
I am tempted to say that we should instead change the keyring code in the service activation logic after all, so that the user keyring of system users is linked into the session keyring we create...
| serial = keyctl(KEYCTL_SEARCH, | ||
| KEY_SPEC_USER_KEYRING, | ||
| (unsigned long) "user", | ||
| (unsigned long) keyname, 0); |
There was a problem hiding this comment.
KEYCTL_SEARCH and request_key() are substantially different, as the latter might result in a userspace callout from the kernel, and the fomer won't. I don't think it's OK to just replace one by the other...
There was a problem hiding this comment.
Do we want the kernel to do a user callback? IMHO using KEYCTL_SEARCH would be the correct thing to do, no?
Anyway... request_key() does not work with current code. Let's see how things go if we link the keyrings.
| return r; | ||
|
|
||
| serial = add_key("user", keyname, p, n, KEY_SPEC_USER_KEYRING); | ||
| serial = add_key("user", keyname, p, n, KEY_SPEC_SESSION_KEYRING); |
There was a problem hiding this comment.
I am not sure I follow on this one? why not place this in the user keyring directly?
There was a problem hiding this comment.
That's the whole point about this commit (and issue #5522): Adding a key to user keyring results in a key that can not be access any more.
|
Ok, will have a look at the code about service activation logic then. |
|
OK, let's close this one in favour of #6275 |
Commit 74dd6b5 (core: run each system
service with a fresh session keyring) broke keyring handling in ask-password.
The key was added to user keyring, but permissions denied to access it. So
instead add the key to session keyring, set permission, link to user keyring
and unlink from session keyring.
For whatever reason requesting a key does not longer work. Search instead.