-
Notifications
You must be signed in to change notification settings - Fork 202
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
Design: Handle complex PAM authentifications #244
Comments
I have another case, if I used |
So basically you have multiple passwords. That generally falls into the 'control flow should be driven by pam' as in if and when a password is requested depends on pam configuration, and the hardcoded flow to send one password and wait for something to happen is not suited for all cases. |
Also this would be #270 |
Yeah, this should totally be driven by PAM, with swaylock having
callback functions depending on the next step as requested by PAM, like
- message the user for action (-> touch hardware token / touch
fingerprint reader)
- message the user, asking for input (-> provide password, provide
hardware token PIN code)
- etc...
linux-pam actually has a generic conversation function here (for text
application):
https://github.com/linux-pam/linux-pam/blob/cce619f09baf6247b32562f99497f4661068249e/libpam_misc/misc_conv.c#L280
I had entertain the idea of proposing them to add a generic conversatio function for non-text application, which would use callback (=function pointers)
to get back info from the application, but I haven't yet took/had the time for that.
|
I do not really see how that conversation function is text-specific. The only obvious problem is that the prompt sent by PAM may be much longer than what can fit into the swaylock circle. That is, however, swaylock design choice, not limitation of graphic applications in general. |
I see, it does not tell you if it expects an in-band (eg. password) or out-of-band (eg. touching sensor) response from the user. |
I do not really see how that conversation function is text-specific.
It read and writes from tty (look at the `read_string` helper), and do so in-process (if I remember correctly swaylock forks and do the pam conversation in a distinct process to avoid blocking the main one, see the OP)
|
The input to misc_conv are the callback pointers. So if you want callback pointers then you should not call the function, and use the pointers. |
As long as the sequence is strictly ordered with nothing happening in parallel (e.g., not attempting to allow fingerprint or password, whichever is first), the only issues are:
That is, if swaylock is made to show notices and wait for enter on these, and take input as many times as PAM asks for it rather than assume only one input, then it should work.
Nothing within swaylock or PAM modules should read from/write to a tty. If you guys want to do a live discussion, maybe consider #sway-devel @ irc.libera.chat. |
What blocks the flow, though? Why is pressing enter required to use a security key, that looks superfluous. And why can't swaylock advance the flow, even if pam itself would block (provided that the information that the prompt does not require a reply is available)? |
PAM is a blocking API. swaylock cannot continue past swaylock cannot return from the conversation function until the requested response is obtained, as obtaining the response is the whole point of the conversation function. There is also no way for a PAM module to "cancel" a conversation, so once a question or notice is presented, you're stuck until a response is given. The only way for a PAM module to wait for something other than the user is to not ask a question in the first place, but then you have no UI. Advancing blindly on info or error conversations might lead to problems. E.g., if you assume that you can just show an info prompt in a sticky way and advance, if that info message is the last message in the authentication flow and nothing further is required from the user, swaylock will end up exiting before the user had a chance to read it. This isn't as much of an issue when dealing with Note that all this is discussion about adding the support for info and error messages. Right now, swaylock ignores any info and error message, and answers any question with the typed password, no matter what the questions are and how many times they ask. |
Regarding the "parallel auth", (out of scope here)For more details why the "any of " scheme is hard/not doable with PAM, see linux-pam/linux-pam#301
Theoretically, with the current stack PAM, I think 'no-input' methods could happen in parallel, because swaylock can (as long as swaylock handle all of the PAM messages in the array received by the conversation function) Anything that needs a actual pam response with content would prevent that though, for the reasons you mention. But that's not really swaylock problem, so, out of scope for this discussion
This should be handled by treating PAM_PROMPT_ECHO_OFF and PAM_PROMPT_ECHO_ON differently (from man 3 pam_conv).
-> PAM_TEXT_INFO
What I'm wondering is how should multiples PAM messages (in one call to the conversation function) should be handled, |
While theoretically possible are there PAM modules that would require more than one input at once? Designing the UI for multiple inputs would be challenging, and there is no point going through that if it's never used in practice. |
You'd go with the same style as for a text-based prompt: presenting and processing them in sequence. A fancier UI with actual input fields could handle it differently, but swaylock is not that.
See note from above:
|
That special case that it's the last message can be detected, there is no reason to block on informational messages otherwise. |
> What I'm wondering is how should multiples PAM messages (in one call to the conversation function) should be handled,
You'd go with the same style as for a text-based prompt: presenting and processing them in sequence. A fancier UI with actual input fields could handle it differently, but swaylock is not that.
ACK.
> -> PAM_TEXT_INFO
See note from above:
> Advancing blindly on info or error conversations might lead to problems. E.g., if you assume that you can just show an info prompt in a sticky way and advance, if that info message is the last message in the authentication flow and nothing further is required from the user, swaylock will end up exiting before the user had a chance to read it. This isn't as much of an issue when dealing with login(1), sudo(1) or ssh(1) where any messages stay on the terminal after successful or attempted login.
Is that much of a problem ? I mean, if authentication is successful, swaylock exit anyway, does it not ?
https://github.com/swaywm/swaylock/blob/cca2436ba535d17c49b7d7ba75ebfa6a18ec9209/pam.c#L91-L112
In case of authentication failure, pam_authenticate would return here, right ? so it would be up to swaylock to choose what to do with the remaining messages, I think ?
I'd be more worried about a sequence of PAM_TEXT_INFO or PAM_ERROR_MSG in the pam conversation with no time between them (= the pam module itself not waiting like pam_u2f for a fido authenticator to be touched or fprintd for the fingerprinting device), because the second would erase the first (and so on).
|
Hi, I would like to have a go at this. I'm using a fingerprint reader. To summarize, currently there are two pipes for communication between the main thread / UI and the backend / PAM:
To properly handle the conversation, I think we need to slightly rework the communication model to have three pipes:
(to clarify: "PAM" refers to the backend process that runs Both of the PAM -> UI communication channels can be handled asynchronously in the event loop like it is done currently for the password check result. I believe that it's ok for the PAM thread / conversation to block on the prompt, this is how it would work in a text-based application. I think this should also work fine for the non-PAM backend in It's basically a coincidence that currently auth works with the fingerprint reader after pressing enter. Pressing enter unblocks the read waiting for the password submission here, and then enters the PAM conversation. But PAM is actually asking for the fingerprint, which is then read by PAM independently of swaylock. I'm working on a prototype that implements the proposed communication, I'll try to prepare a PR in the next few days. I think that will make easier to think about the UI/UX. (edited: grammar, clarification) |
Why would you have two PAM -> UI pipes? Nothing can occur in parallel (PAM is not thread-safe) and PAM only has a single user-facing mechanism: a conversation. When done right, there is no "password check" - only an answer to a conversations, some of which may be secrets. You can just prefix (or JSON encode) the message with the type, and either show the received text, the password wheel of dealing with a secret, a red wheel if you got an error, or unlock if you received a success. |
It's true that in principle one pipe is enough, but handling the different kinds of messages depending on the pipe contents feels messy. With different pipes for each message kind, we have separate callback functions for each, which have to be concerned with only one thing instead of branching depending on the message type. Is it expensive to create pipes? The "success/error" message is actually pretty distinct from the PAM conversation. We don't know the auth result inside the conversation function (where the prompt messages would be sent and secrets received), it's only known from the return value of |
Multiple pipes means that everything happens asynchronously, ie. two messages sent one after the other on one side might come out-of-order on the other side. This is a footgun. |
That's fair. Though I don't quite see what problems could arise if we switch the order of receiving the auth result flag and a message like "incorrect password". Maybe the UI state can be messed up? Although the UI elements corresponding to the message and success indicator should be independent anyways. Messages will always be in order because they use the same pipe. |
Even if out-of-order delivery isn't required for a specific implementation of this feature, it's (1) easy to get wrong and (2) not future-proof if the IPC needs to be expanded in the future. In other words, it's not a maintainable solution. |
Fair enough, I'll do it using one pipe. To me it also feels error-prone to encode/decode messages sent through the pipe, but perhaps it's less so than async communication. |
Hello,
I started to play with my Yubikeys recently and would like to work on swaylock
so it can handle the UI part a bit better.
(The objective would be to fix #61 and #213)
I'm not super familiar with the codebase so I wanted to check with you before
starting to code.
As I understand, currently, swaylock handle the auth process in a separate
process, which communicates with the main one (doing rendering, event looping
etc) using pipes. (in pam.c and comm.c). The main process pass the password
string and the "auth" process respond with ok/ko (it's a simplification)
In order to support "richer" conversation with PAM, we need:
can displays prompts from PAM.
My first question, should I re-use the same pipe used to communicate the
password string to the child process to send back prompt string, or should I use
a dedicated one, something else altogether ? (Is there a particular policy on
how the password is handled, or something like that ?)
Secondly, I think the control flow should be driven by the code in pam.c,
particularly in the
handle_converstation
fonction. The initial state ofswaylock (requiring a password, prompting user to insert a token, etc) will
depend on the particular pam configuration and what msg_style pam has sent us.
That seems like a big change so would it be ok ?
I think we need two new states (for lack of a better word):
ENTER)
Those two could be assimilated, to, respectively, the AUTH_SPACE_INPUT (writing
the password) and AUTH_STATE_VALIDATING ("verifying") (which is mostly what's
going on currently I think) but it's not a very clear UX.
I'm only testing with a Yubikey (using pam_u2f) so I'm only considering those
states/action right now but if you see other cases don't hesitate to mention it.
I would very much appreciate feedback on this very rough sketch, or things you
think I should think about, or questions if I'm not clear.
The text was updated successfully, but these errors were encountered: