Skip to content
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

Support a SessionUserCommand for external session user mapping #21631

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chriswiggins
Copy link
Contributor

Continuing on from #21626, I've started on adding support into cockpit-session to call an external command (if defined), to allow external authentication mechanisms to map to PAM users, and follow the normal session flow. I didn't want to have to re-implement all of the session logic, because all I want to do is call a script and return a username.

This relatively closely mimics the certificate authentication, except relies on a defined application to validate the auth.

Usage

cockpit.conf

Define a new auth scheme section, use the same UnixPath as cockpit-session and then specify a SessionUserCommand to call

[x-my-auth-scheme]
UnixPath = /run/cockpit/sesson
SessionUserCommand  = /usr/bin/my-auth-scheme

/usr/bin/my-auth-scheme

#!/usr/bin/env python3

import sys

args = sys.argv
rhost = sys.argv[1]
auth_line = sys.argv[2]

if auth_line == "this_is_a_test":
  print("user-1")
  sys.exit(0)

print("Your auth line is incorrect", file=sys.stderr)
sys.exit(1)

When an auth request to cockpit with the header Authorization: x-my-auth-scheme this_is_a_test is sent, /usr/bin/my-auth-scheme is called by cockpit-session, which in turn returns user-1, and if that user exists, the PAM session is set up and the user is logged in.

We are using this with JWTs. The sub of the JWT has the username in it, and the python script validates that the JWT was signed by a known authority.

Happy to take guidance on the security aspect of calling an external script or whether or not some of the timing logic in the cert auth should be used here too - my C is a tad rusty!

@martinpitt martinpitt marked this pull request as draft February 18, 2025 05:09
@chriswiggins chriswiggins changed the title Support a UserSessionCommand for external session user mapping Support a SessionUserCommand for external session user mapping Feb 18, 2025
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed that a little in #21626. I see where you are coming from, but I am really nervous about this -- session.c is incredibly delicate and security sensitive, and we'll have to do a few rounds of reviews and scrutiny here. I did a first coarse review.

Note that you shouldn't specify UnixPath in your custom auth scheme -- this is cockpit-session, so it should use the implied default.

This needs to be documented in doc/authentication.md and the guide -- similar to doc/guide/cert-authentication.xml . Finally, this requires some integration test in test/verify/check-static-login with some example auth script, and exercise positive and negative cases. I am happy to give some guidance here of course, and also help (I have limited time, though). If you want to work on this, can you please have a look at https://github.com/cockpit-project/cockpit/blob/main/test/README.md ? That explains how to set up the devel container and run the integration test.

Thanks!



char *
run_user_command (const char *cmd, const char *rhost, const char *authorization)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really necessary to introduce a new function for that? There are a lot of things wrong with it, and we can go through it one by one -- but it seems to me that you should be able to use the existing spawn_and_wait() for this? You can remap fd 1 (stdout) to a pipe() and read the user name from that. Now, I don't know if you can actually do that after the process already exited (could work for small amounts of data depending on how much Linux buffers, but you only expect a user name). If not, then let's rather expand spawn_and_wait() to collect the output than having a similar duplicate function.

@@ -798,6 +799,53 @@ perform_tlscert (const char *rhost,
return pamh;
}

static pam_handle_t *
perform_external (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perform_session_user_command()?


res = open_session (pamh);

create_s4u_ticket (username);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels very strange here -- none of this starts a Kerberos authentication. Is that just an accidental copy&paste?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants