diff --git a/.github/workflows/qa.yaml b/.github/workflows/qa.yaml index 26fe00e38..439f316f8 100644 --- a/.github/workflows/qa.yaml +++ b/.github/workflows/qa.yaml @@ -30,6 +30,7 @@ jobs: with: golangci-lint-configfile: ".golangci-ci.yaml" tools-directory: "tools" + generate-diff-paths-to-ignore: po/* docs/**/*.md README.md - name: C code formatting uses: jidicula/clang-format-action@v4.11.0 with: diff --git a/cmd/adsysd/client/policy.go b/cmd/adsysd/client/policy.go index e7581412d..21727545f 100644 --- a/cmd/adsysd/client/policy.go +++ b/cmd/adsysd/client/policy.go @@ -7,6 +7,7 @@ import ( "io" "os" "os/user" + "strconv" "strings" "github.com/fatih/color" @@ -18,6 +19,8 @@ import ( "github.com/ubuntu/adsys/internal/cmdhandler" "github.com/ubuntu/adsys/internal/consts" log "github.com/ubuntu/adsys/internal/grpc/logstreamer" + "github.com/ubuntu/decorate" + "golang.org/x/sys/unix" ) func (a *App) installPolicy() { @@ -95,6 +98,22 @@ func (a *App) installPolicy() { RunE: func(cmd *cobra.Command, args []string) error { return a.dumpCertEnrollScript() }, } debugCmd.AddCommand(certEnrollCmd) + ticketPathCmd := &cobra.Command{ + Use: "ticket-path", + Short: gotext.Get("Print the path of the current (or given) user's Kerberos ticket"), + Long: gotext.Get(`Infer and print the path of the current user's Kerberos ticket, leveraging the krb5 API. +The command is a no-op if the ticket is not present on disk or the detect_cached_ticket setting is not true.`), + Args: cmdhandler.ZeroOrNArgs(1), + ValidArgsFunction: cmdhandler.NoValidArgs, + RunE: func(cmd *cobra.Command, args []string) error { + var username string + if len(args) > 0 { + username = args[0] + } + return a.printTicketPath(username) + }, + } + debugCmd.AddCommand(ticketPathCmd) var updateMachine, updateAll *bool updateCmd := &cobra.Command{ @@ -300,6 +319,58 @@ func (a *App) dumpCertEnrollScript() error { return os.WriteFile("cert-autoenroll", []byte(script), 0600) } +// printTicketPath prints the path to the Kerberos ccache of the given (or current) user to stdout. +// The function is a no-op if the detect_cached_ticket setting is not enabled. +// No error is raised if the inferred ticket is not present on disk. +func (a *App) printTicketPath(username string) (err error) { + defer decorate.OnError(&err, gotext.Get("error getting ticket path")) + + // Do not print anything if the required setting is not enabled + if !a.config.DetectCachedTicket { + log.Debugf(a.ctx, "The detect_cached_ticket setting needs to be enabled to use this command") + return nil + } + + // Default to current user + if username == "" { + u, err := user.Current() + if err != nil { + return fmt.Errorf("failed to retrieve current user: %w", err) + } + username = u.Username + } + + user, err := user.Lookup(username) + if err != nil { + return err + } + uid, err := strconv.Atoi(user.Uid) + if err != nil { + return err + } + + // This effectively deescalates the current user's privileges with no + // possibility of turning back. We're doing this on purpose right before the + // code path that requires this, with the program exiting immediately after. + if err := unix.Setuid(uid); err != nil { + return fmt.Errorf(gotext.Get("failed to set privileges to UID %d: %v", uid, err)) + } + + krb5ccPath, err := ad.TicketPath() + if errors.Is(err, ad.ErrTicketNotPresent) { + log.Debugf(a.ctx, "No ticket found for user %s: %s", username, err) + return nil + } + + if err != nil { + return err + } + + fmt.Println(krb5ccPath) + + return nil +} + func colorizePolicies(policies string) (string, error) { first := true var out stringsBuilderWithError diff --git a/cmd/adsysd/integration_tests/adsysctl_policy_test.go b/cmd/adsysd/integration_tests/adsysctl_policy_test.go index 40111e7a4..9067d0d94 100644 --- a/cmd/adsysd/integration_tests/adsysctl_policy_test.go +++ b/cmd/adsysd/integration_tests/adsysctl_policy_test.go @@ -1248,6 +1248,77 @@ func TestPolicyDebugScriptDump(t *testing.T) { } } +func TestPolicyDebugTicketPath(t *testing.T) { + tests := map[string]struct { + username string + + configDisabled bool + pathNotPresent bool + pathIsDir bool + + wantOut string + wantErr bool + }{ + "Return path for current explicit user": {}, + "Return path for current implicit user": {username: "-"}, + + // No-op cases (return no error and no output) + "No-op when path not present on disk": {pathNotPresent: true}, + "No-op when detect_cached_ticket is not set": {configDisabled: true}, + + // Error cases + "Error when passed an invalid user": {username: "invaliduser", wantErr: true}, + "Error if ticket path is a directory": {pathIsDir: true, wantErr: true}, + } + for name, tc := range tests { + tc := tc + t.Run(name, func(t *testing.T) { + // Empty username means current user + if tc.username == "" { + u, err := user.Current() + require.NoError(t, err, "Setup: could not get current user") + tc.username = u.Username + } + // "-" username means empty argument, current user is inferred + if tc.username == "-" { + tc.username = "" + } + + // Ensure we start and finish the test with a clean slate on disk + uid := os.Getuid() + krb5ccname := filepath.Join(os.TempDir(), fmt.Sprintf("krb5cc_%d", uid)) + err := os.RemoveAll(krb5ccname) + require.NoError(t, err, "Setup: could not remove ticket path") + t.Cleanup(func() { + err := os.RemoveAll(krb5ccname) + require.NoError(t, err, "Teardown: could not remove ticket path") + }) + + if tc.pathIsDir { + err := os.MkdirAll(krb5ccname, 0700) + require.NoError(t, err, "Setup: could not create ticket directory") + } else if !tc.pathNotPresent { + err := os.WriteFile(krb5ccname, []byte("Some ticket content"), 0600) + require.NoError(t, err, "Setup: could not write ticket content") + tc.wantOut = krb5ccname + "\n" + } + + if tc.configDisabled { + tc.wantOut = "" + } + + conf := createConf(t, confDetectCachedTicket(!tc.configDisabled)) + out, err := runClient(t, conf, "policy", "debug", "ticket-path", tc.username) + if tc.wantErr { + require.Error(t, err, "command should exit with an error") + return + } + require.NoError(t, err, "command should exit with no error") + require.Equal(t, tc.wantOut, out, "command output should match") + }) + } +} + func modifyAndAddUsers(t *testing.T, new string, users ...string) (passwd string) { t.Helper() dest := filepath.Join(t.TempDir(), "passwd") diff --git a/e2e/cmd/run_tests/05_test_pam_krb5cc/main.go b/e2e/cmd/run_tests/05_test_pam_krb5cc/main.go new file mode 100644 index 000000000..bbeb5be67 --- /dev/null +++ b/e2e/cmd/run_tests/05_test_pam_krb5cc/main.go @@ -0,0 +1,108 @@ +// Package main provides a script that runs PAM krb5cc-related tests on the +// provisioned Ubuntu client. +package main + +import ( + "context" + "fmt" + "os" + "path/filepath" + + log "github.com/sirupsen/logrus" + "github.com/ubuntu/adsys/e2e/internal/command" + "github.com/ubuntu/adsys/e2e/internal/inventory" + "github.com/ubuntu/adsys/e2e/internal/remote" +) + +var sshKey string + +func main() { + os.Exit(run()) +} + +func run() int { + cmd := command.New(action, + command.WithValidateFunc(validate), + command.WithRequiredState(inventory.ADProvisioned), + ) + cmd.Usage = fmt.Sprintf(`go run ./%s [options] + +Perform PAM krb5cc-related tests on the provisioned Ubuntu client. + +The runner must be connected to the ADSys E2E tests VPN.`, filepath.Base(os.Args[0])) + + return cmd.Execute(context.Background()) +} + +func validate(_ context.Context, cmd *command.Command) (err error) { + sshKey, err = command.ValidateAndExpandPath(cmd.Inventory.SSHKeyPath, command.DefaultSSHKeyPath) + if err != nil { + return err + } + + return nil +} + +func action(ctx context.Context, cmd *command.Command) error { + rootClient, err := remote.NewClient(cmd.Inventory.IP, "root", sshKey) + if err != nil { + return fmt.Errorf("failed to connect to VM: %w", err) + } + + defer func() { + if _, err := rootClient.Run(ctx, "rm -f /etc/adsys.yaml"); err != nil { + log.Errorf("Teardown: Failed to remove adsys configuration file: %v", err) + } + }() + + // Install krb5-user to be able to interact with kinit + if _, err := rootClient.Run(ctx, "apt-get update && apt-get install -y krb5-user"); err != nil { + return fmt.Errorf("failed to install krb5-user: %w", err) + } + + /// detect_cached_ticket unset (disabled) + // Connect with pubkey to bypass pam_sss setting KRB5CCNAME + client, err := remote.NewClient(cmd.Inventory.IP, fmt.Sprintf("%s-usr@warthogs.biz", cmd.Inventory.Hostname), sshKey) + if err != nil { + return fmt.Errorf("failed to connect to VM as user with pubkey: %w", err) + } + if err := client.RequireEmpty(ctx, "echo $KRB5CCNAME"); err != nil { + return fmt.Errorf("KRB5CCNAME not empty: %w", err) + } + + // Create a ccache + if _, err := client.Run(ctx, fmt.Sprintf("kinit %s-usr@WARTHOGS.BIZ <<<'%s'", cmd.Inventory.Hostname, remote.DomainUserPassword)); err != nil { + return fmt.Errorf("failed to create ccache: %w", err) + } + + // Set detect_cached_ticket to true + if _, err := rootClient.Run(ctx, "echo 'detect_cached_ticket: true' > /etc/adsys.yaml"); err != nil { + return fmt.Errorf("failed to set detect_cached_ticket to true: %w", err) + } + + /// detect_cached_ticket enabled + // Reconnect as user + client, err = remote.NewClient(cmd.Inventory.IP, fmt.Sprintf("%s-usr@warthogs.biz", cmd.Inventory.Hostname), sshKey) + if err != nil { + return fmt.Errorf("failed to connect to VM as user with pubkey: %w", err) + } + if err := client.RequireNotEmpty(ctx, "echo $KRB5CCNAME"); err != nil { + return fmt.Errorf("KRB5CCNAME empty: %w", err) + } + + // Remove ticket cache + if _, err := rootClient.Run(ctx, "rm -f /tmp/krb5cc_*"); err != nil { + return fmt.Errorf("failed to remove ticket cache: %w", err) + } + + // Reconnect as user, KRB5CCNAME should be left unset + client, err = remote.NewClient(cmd.Inventory.IP, fmt.Sprintf("%s-usr@warthogs.biz", cmd.Inventory.Hostname), sshKey) + if err != nil { + return fmt.Errorf("failed to connect to VM as user with pubkey: %w", err) + } + if err := client.RequireEmpty(ctx, "echo $KRB5CCNAME"); err != nil { + return fmt.Errorf("KRB5CCNAME not empty: %w", err) + } + + return nil +} diff --git a/e2e/internal/remote/require.go b/e2e/internal/remote/require.go index 268f2c306..324103fca 100644 --- a/e2e/internal/remote/require.go +++ b/e2e/internal/remote/require.go @@ -2,6 +2,7 @@ package remote import ( "context" + "errors" "fmt" "strings" ) @@ -34,6 +35,29 @@ func (c Client) RequireContains(ctx context.Context, cmd string, expected string return nil } +// RequireEmpty runs the given command and returns an error if the output is not empty. +func (c Client) RequireEmpty(ctx context.Context, cmd string) error { + out, err := c.Run(ctx, cmd) + if err != nil { + return err + } + + if strings.TrimSpace(string(out)) != "" { + return fmt.Errorf("expected empty output, got %q", string(out)) + } + + return nil +} + +// RequireNotEmpty runs the given command and returns an error if the output is empty. +func (c Client) RequireNotEmpty(ctx context.Context, cmd string) error { + if err := c.RequireEmpty(ctx, cmd); err == nil { + return errors.New("expected non-empty output") + } + + return nil +} + // RequireFileExists returns an error if the given file does not exist. func (c Client) RequireFileExists(ctx context.Context, filepath string) error { _, err := c.Run(ctx, fmt.Sprintf("test -f %q", filepath)) diff --git a/e2e/scripts/first-run.sh b/e2e/scripts/first-run.sh index c91589b93..419d3b50f 100644 --- a/e2e/scripts/first-run.sh +++ b/e2e/scripts/first-run.sh @@ -9,3 +9,10 @@ hostnamectl set-hostname "$hostname" echo "Adding hostname to hosts file..." echo "127.0.0.1 $hostname" >> /etc/hosts + +# These overrides disable password authentication which we explicitly enabled +# during provisioning. Since they take precedence over the main sshd_config we +# have to remove them. +echo "Removing cloud-init ssh configuration overrides..." +rm -rf /etc/ssh/sshd_config.d +systemctl restart ssh diff --git a/e2e/scripts/patches/focal.patch b/e2e/scripts/patches/focal.patch index ffbc9f259..3c82d9cc2 100644 --- a/e2e/scripts/patches/focal.patch +++ b/e2e/scripts/patches/focal.patch @@ -1,5 +1,5 @@ diff --git a/debian/control b/debian/control -index d6fb19f2..f9f9fe0c 100644 +index ccf213e0..40a2aa9f 100644 --- a/debian/control +++ b/debian/control @@ -2,10 +2,10 @@ Source: adsys @@ -12,9 +12,9 @@ index d6fb19f2..f9f9fe0c 100644 dh-golang, - golang-go (>= 2:1.21~), + golang-1.21-go, - libsmbclient-dev, + apparmor, + dbus, libdbus-1-dev, - libglib2.0-dev, diff --git a/debian/rules b/debian/rules index 43646c6a..0708aa3d 100755 --- a/debian/rules diff --git a/e2e/scripts/patches/jammy.patch b/e2e/scripts/patches/jammy.patch index 00d105b92..338fae1d2 100644 --- a/e2e/scripts/patches/jammy.patch +++ b/e2e/scripts/patches/jammy.patch @@ -1,5 +1,5 @@ diff --git a/debian/control b/debian/control -index d6fb19f2..93764223 100644 +index ccf213e0..2d34a328 100644 --- a/debian/control +++ b/debian/control @@ -5,7 +5,7 @@ Maintainer: Ubuntu Developers @@ -8,9 +8,9 @@ index d6fb19f2..93764223 100644 dh-golang, - golang-go (>= 2:1.21~), + golang-1.21-go, - libsmbclient-dev, + apparmor, + dbus, libdbus-1-dev, - libglib2.0-dev, diff --git a/debian/rules b/debian/rules index 43646c6a..403e7bb9 100755 --- a/debian/rules diff --git a/e2e/scripts/provision.sh b/e2e/scripts/provision.sh index 8df89454f..ce5512811 100644 --- a/e2e/scripts/provision.sh +++ b/e2e/scripts/provision.sh @@ -14,7 +14,6 @@ echo "Configure PAM to register user sessions in the systemd control group hiera pam-auth-update --enable systemd echo "Enabling keyboard-interactive authentication for domain users..." -rm -rf /etc/ssh/sshd_config.d # this contains overrides that conflict with our changes sed -iE 's/^#\?PasswordAuthentication.*/PasswordAuthentication yes/' /etc/ssh/sshd_config sed -iE 's/^#\?KbdInteractiveAuthentication.*/KbdInteractiveAuthentication yes/' /etc/ssh/sshd_config diff --git a/internal/ad/krb5.go b/internal/ad/krb5.go index 70224fc15..35a8ace1a 100644 --- a/internal/ad/krb5.go +++ b/internal/ad/krb5.go @@ -37,6 +37,9 @@ import ( "github.com/leonelquinteros/gotext" ) +// ErrTicketNotPresent is returned when the ticket cache is not present or not accessible +var ErrTicketNotPresent = errors.New(gotext.Get("ticket not found or not accessible")) + // TicketPath returns the path of the default kerberos ticket cache for the // current user. // It returns an error if the path is empty or does not exist on the disk. @@ -54,7 +57,7 @@ func TicketPath() (string, error) { krb5ccPath := strings.TrimPrefix(krb5cc, "FILE:") fileInfo, err := os.Stat(krb5ccPath) if err != nil { - return "", fmt.Errorf(gotext.Get("%q does not exist or is not accessible: %v", krb5ccPath, err)) + return "", errors.Join(ErrTicketNotPresent, err) } if !fileInfo.Mode().IsRegular() { return "", fmt.Errorf(gotext.Get("%q is not a regular file", krb5ccPath)) diff --git a/internal/ad/krb5_test.go b/internal/ad/krb5_test.go index 73847ddc6..9fad5820f 100644 --- a/internal/ad/krb5_test.go +++ b/internal/ad/krb5_test.go @@ -28,7 +28,8 @@ func TestTicketPath(t *testing.T) { krb5Behavior string ccacheIsDir bool - wantErr bool + wantErr bool + wantErrType error }{ "Lookup is successful": {krb5Behavior: "return_ccache:FILE:%s"}, "Allow ccache without FILE identifier": {krb5Behavior: "return_ccache:%s"}, @@ -38,7 +39,7 @@ func TestTicketPath(t *testing.T) { "Error when initializing context": {krb5Behavior: "error_initializing_context", wantErr: true}, "Error on empty ticket path": {krb5Behavior: "return_empty_ccache", wantErr: true}, "Error on NULL ticket path": {krb5Behavior: "return_null_ccache", wantErr: true}, - "Error on non-FILE ccache": {krb5Behavior: "return_memory_ccache", wantErr: true}, + "Error on non-FILE ccache": {krb5Behavior: "return_memory_ccache", wantErrType: ad.ErrTicketNotPresent}, } for name, tc := range tests { @@ -61,8 +62,11 @@ func TestTicketPath(t *testing.T) { require.NoError(t, err, "Setup: Failed to create path to ticket cache") ticketPath, err := ad.TicketPath() - if tc.wantErr { + if tc.wantErr || tc.wantErrType != nil { require.Error(t, err, "TicketPath should have errored out") + if tc.wantErrType != nil { + require.ErrorIs(t, err, tc.wantErrType, "TicketPath should have returned the expected error type") + } return } require.NoError(t, err, "Call to TicketPath failed") diff --git a/pam/pam_adsys.c b/pam/pam_adsys.c index f1611ad16..838b1f0b3 100644 --- a/pam/pam_adsys.c +++ b/pam/pam_adsys.c @@ -309,6 +309,93 @@ static int set_dconf_profile(pam_handle_t *pamh, const char *username, int debug return retval; } +/* + * Get the ticket path for the user by calling adsysctl policy debug ticket-path + */ +static int get_krb5cc_ticket_path(pam_handle_t *pamh, const char *username, char **path) { + char **arggv; + arggv = calloc(6, sizeof(char *)); + if (arggv == NULL) { + return 1; + } + + arggv[0] = "/sbin/adsysctl"; + arggv[1] = "policy"; + arggv[2] = "debug"; + arggv[3] = "ticket-path"; + arggv[4] = (char *)(username); + arggv[5] = NULL; + + int pipefd[2]; + if (pipe(pipefd) == -1) { + pam_syslog(pamh, LOG_ERR, "Failed to create pipe: %m"); + return 1; + } + + pid_t pid = fork(); + if (pid == -1) { + pam_syslog(pamh, LOG_ERR, "Failed to fork process"); + return 1; + } + + if (pid > 0) { /* parent */ + pid_t retval; + int status = 0; + + while ((retval = waitpid(pid, &status, 0)) == -1 && errno == EINTR) { + }; + + if (retval == (pid_t)-1) { + pam_syslog(pamh, LOG_ERR, "waitpid returns with -1: %m"); + free(arggv); + return 1; + } else if (status != 0) { + if (WIFEXITED(status)) { + pam_syslog(pamh, LOG_ERR, "adsysctl policy debug ticket-path %s failed: exit code %d", username, + WEXITSTATUS(status)); + } else if (WIFSIGNALED(status)) { + pam_syslog(pamh, LOG_ERR, "adsysctl policy debug ticket-path %s failed: caught signal %d%s", username, + WTERMSIG(status), WCOREDUMP(status) ? " (core dumped)" : ""); + } else { + pam_syslog(pamh, LOG_ERR, "adsysctl policy debug ticket-path %s failed: unknown status 0x%x", username, + status); + } + free(arggv); + return 1; + } + free(arggv); + close(pipefd[1]); + + char ticket_path[PATH_MAX + 1]; + ssize_t n; + while ((n = read(pipefd[0], ticket_path, sizeof(ticket_path))) > 0) { + if (n == -1) { + pam_syslog(pamh, LOG_ERR, "Failed to read from pipe: %m"); + return 1; + } + ticket_path[n] = '\0'; + char *newline = strchr(ticket_path, '\n'); + if (newline != NULL) { + *newline = '\0'; + } + close(pipefd[0]); + *path = strdup(ticket_path); + return 0; + } + } else { /* child */ + dup2(pipefd[1], STDOUT_FILENO); + close(pipefd[0]); + close(pipefd[1]); + execv(arggv[0], arggv); + int i = errno; + pam_syslog(pamh, LOG_ERR, "execv(%s,...) failed: %m", arggv[0]); + free(arggv); + _exit(i); + } + + return 0; /* command had no output and exited with 0 */ +} + PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, const char **argv) { return PAM_IGNORE; } PAM_EXTERN int pam_sm_setcred(pam_handle_t *pamh, int flags, int argc, const char **argv) { return PAM_IGNORE; } @@ -340,7 +427,36 @@ PAM_EXTERN int pam_sm_open_session(pam_handle_t *pamh, int flags, int argc, cons */ const char *krb5ccname = pam_getenv(pamh, "KRB5CCNAME"); if (krb5ccname == NULL && strcmp(username, "gdm") != 0) { - return PAM_IGNORE; + char *ticket_path = NULL; + + // An error here means the detect_cached_ticket setting is enabled + if (get_krb5cc_ticket_path(pamh, username, &ticket_path) != 0) { + pam_syslog(pamh, LOG_ERR, "Failed to get ticket path for user %s", username); + return PAM_SYSTEM_ERR; + }; + + // The detect_cached_ticket setting is disabled or we weren't able to + // locate a the path returned by krb5 on disk + if (ticket_path == NULL || *ticket_path == '\0') { + return PAM_IGNORE; + } + + // We have a ticket, proceed with setting the environment variable + char *envvar; + if (asprintf(&envvar, "KRB5CCNAME=FILE:%s", ticket_path) < 0) { + pam_syslog(pamh, LOG_CRIT, "out of memory"); + free(ticket_path); + return PAM_BUF_ERR; + } + + retval = pam_putenv(pamh, envvar); + krb5ccname = strdup(ticket_path); + _pam_drop(envvar); + free(ticket_path); + if (retval != PAM_SUCCESS) { + pam_syslog(pamh, LOG_ERR, "Failed to set KRB5CCNAME to %s", ticket_path); + return PAM_SYSTEM_ERR; + } } // set dconf profile for AD and gdm user.