pam: Allow SIGINT (Ctrl+C) while waiting for device press [proof of concept] #315
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a bad idea. This *might* work for sudo, since the sudo process blocks SIGINT and this change overrides that temporarily. However: a) There is no guarantee the pam-u2f plugin is being invoked by sudo b) Even when we are running in sudo, I'm guessing sudo is not expecting a plugin to change the signal handlers, which invites problems Even if we wanted to do something like this, the change is incomplete since no cleanup of the pam-u2f resources is done when a SIGINT happens.
|
Closing since I'm not actually proposing this particular approach. I wrote it up to leave breadcrumbs based on what I learned. |
|
Perhaps I too strongly implied that this approach is a bad idea. Even if sudo isn't the only process that uses libpam, maybe there is no other process that would use libpam in conjunction with pam-u2f where it would inherently be a bad idea to allow SIGINT. Phrase another way: if you're blocked for input (u2f device), wouldn't you want to allow SIGINT always, regardless of context? I leave it to others to work out the details, since, like I said before, I only know enough C programming to be dangerous. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I wrote this code to better understand why it is tricky to come up with a solution to #108
This change is a bad idea as-is. This might work for sudo, since the sudo process blocks SIGINT and this change overrides that temporarily. However:
Even if we wanted to do something like this, the change is incomplete since it doesn't handle pam-u2f resource cleanup, and it doesn't handle all the code paths that wait for user action.