ChocolateLoverRaj / pam-any

A PAM module that runs multiple other PAM modules in parallel, succeeding as long as one of them succeeds.
Apache License 2.0
8 stars 0 forks source link

Send info and error messages in a different thread that doesn't block… #6

Closed ChocolateLoverRaj closed 5 months ago

ChocolateLoverRaj commented 5 months ago

Fixes #2 @Skruppy can you review this?

skruppy commented 5 months ago

In my first test I authenticated only using fingerprint:

$ pamtester su skruppy authenticate
Input: {"modules": {"any-fingerprint": "Fingerprint", "any-password": "Password"}, "mode": "One"}
Input: Input {
    mode: One,
    modules: {
        "any-password": "Password",
        "any-fingerprint": "Fingerprint",
    },
}
[Password] Password: pamtester: successfully authenticated
$ [... no input is echoed beyond this point ...]

So, the authentication itself worked, but afterwards no key press was visible. This is probably because the conv callback of the tester disabled input echoing in order to secretly read the password from the user but was not able to restore the terminal echo settings because it was canceled by the successful result of the fingerprint authentication.

I guess the issue here is that PAM is not rely thread-save but you are calling the conversation from different threads. I guess you have to do all the conversation from the main thread. The bad news is, that this entails that I always have to enter an empty password, even if I only want to authenticate using fingerprint :/

On the upside, I found that pam_conv can handle multiple messages at once, so no need to combine the queued messages during a blocking promt call.

ChocolateLoverRaj commented 5 months ago

The libpam interfaces are only thread-safe if each thread within the multithreaded application uses its own PAM handle.

So that means that this PAM module is a hack. That's ok I guess.

For handling multiple messages at once then the underlying library https://github.com/anowell/pam-rs would need to implement multiple messages at once.

skruppy commented 5 months ago

I guess it's not the underlying library pam-rs here to blame, because this one just passes the calls on to whatever software instantiated the PAM stack (like sudo or GDM) and they in turn just adhere to the API specification.

So maybe what one could do is to extend the loop you already have where you wait for the authentication results and also allow receiving of those four message types through the same channel. They are then picked up one after another in the main thread (and not in parallel threads) and send using conv.send.

The sub PAM stacks running in the threads all have their own PamAnyConversation which just forwards all the messages to the extended loop in the main thread. Each thread also creates their own back-channel. If a sub PAM stack forwards a proming message they also attach the sending end of the back-channel to the forwarded message. Using this, the extended loop in the main thread can send back the result.

This gives us thread safety as well as queuing of massages and only those PAM sub stacks actually waiting for are user response are blocked. It does not prevent that you have to enter an empty password, but at least you don't have to care about any order (you can scan your fingerprint before or after you hit enter).

ChocolateLoverRaj commented 5 months ago

Ideally there shouldn't be a need for pressing enter after unlocking with fingerprint. I wonder if there is a way of doing this without messing up typing in the terminal.

ChocolateLoverRaj commented 5 months ago

@skruppy what are the uses for this pam module (see #7)? I'm thinking of possibly replacing the sudo command with something that runs multiple PAMs in parallel so we don't need to do anything hacky.

skruppy commented 5 months ago

From the documentation of sudo, su and doas, I couldn't figure out a way to use multiple PAM stacks in parallel. Also on the graphical DM side I only found GDM being able to use multiple PAM stacks.

Looking into how https://gitlab.com/mishakmak/pam-fprint-grosshack solves this problem: They just restore the terminal settings before they return to undo the password entry settings. But this is then only a hacky solution for terminal applications and not for example LightDM.

ChocolateLoverRaj commented 5 months ago

I think the "grosshack" method is actually pretty good. Maybe this program should have an option to choose between two modes.

ChocolateLoverRaj commented 5 months ago

I'm going to merge this with "gross hack" mode. We can add a non-hack mode and fix #3 along with it.