diff --git a/internal/brokers/broker.go b/internal/brokers/broker.go index eddc7da7ff..b88b04ffd0 100644 --- a/internal/brokers/broker.go +++ b/internal/brokers/broker.go @@ -355,6 +355,9 @@ func validateUserInfo(uInfo types.UserInfo) (err error) { if uInfo.Name == "" { return errors.New("empty username") } + if err := types.ValidateUsername(uInfo.Name); err != nil { + return err + } // Validate home and shell directories if !filepath.IsAbs(filepath.Clean(uInfo.Dir)) { diff --git a/internal/brokers/broker_test.go b/internal/brokers/broker_test.go index 7abd54ff03..988bd823f3 100644 --- a/internal/brokers/broker_test.go +++ b/internal/brokers/broker_test.go @@ -229,6 +229,7 @@ func TestIsAuthenticated(t *testing.T) { "Error_when_broker_returns_invalid_access": {sessionID: "ia_invalid_access"}, "Error_when_broker_returns_invalid_userinfo": {sessionID: "ia_invalid_userinfo"}, "Error_when_broker_returns_userinfo_with_empty_username": {sessionID: "ia_info_empty_user_name"}, + "Error_when_broker_returns_userinfo_with_invalid_username": {sessionID: "ia_info_invalid_username"}, "Error_when_broker_returns_userinfo_with_empty_group_name": {sessionID: "ia_info_empty_group_name"}, "Error_when_broker_returns_userinfo_with_invalid_homedir": {sessionID: "ia_info_invalid_home"}, "Error_when_broker_returns_userinfo_with_invalid_shell": {sessionID: "ia_info_invalid_shell"}, diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_userinfo_with_invalid_username b/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_userinfo_with_invalid_username new file mode 100644 index 0000000000..6e50a8e529 --- /dev/null +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_userinfo_with_invalid_username @@ -0,0 +1,4 @@ +FIRST CALL: + access: + data: + err: provided userinfo is invalid: username "-invalid_username" is not valid: it must match ^[a-z_][-a-z0-9_.@]*[$]?$ diff --git a/internal/services/pam/pam_test.go b/internal/services/pam/pam_test.go index 77beba8367..ff212ecfba 100644 --- a/internal/services/pam/pam_test.go +++ b/internal/services/pam/pam_test.go @@ -456,6 +456,7 @@ func TestIsAuthenticated(t *testing.T) { "Error_when_broker_returns_invalid_access": {username: "ia_invalid_access@example.com"}, "Error_when_broker_returns_invalid_data": {username: "ia_invalid_data@example.com"}, "Error_when_broker_returns_invalid_userinfo": {username: "ia_invalid_userinfo@example.com"}, + "Error_when_broker_returns_invalid_username": {username: "ia_info_invalid_username@example.com"}, "Error_when_calling_second_time_without_cancelling": {username: "ia_second_call@example.com", secondCall: true}, // local group error diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/IsAuthenticated b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/IsAuthenticated new file mode 100644 index 0000000000..717f814085 --- /dev/null +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/IsAuthenticated @@ -0,0 +1,4 @@ +FIRST CALL: + access: + msg: + err: provided userinfo is invalid: username "-invalid_username" is not valid: it must match ^[a-z_][-a-z0-9_.@]*[$]?$ diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/cache.db new file mode 100644 index 0000000000..03de3ff950 --- /dev/null +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/cache.db @@ -0,0 +1,4 @@ +users: [] +groups: [] +users_to_groups: [] +schema_version: 2 diff --git a/internal/testutils/broker.go b/internal/testutils/broker.go index 3ef9eb3d26..8be941a983 100644 --- a/internal/testutils/broker.go +++ b/internal/testutils/broker.go @@ -385,6 +385,8 @@ func userInfoFromName(sessionID string, extraGroups []groupJSONInfo) string { name = "" case "ia_info_mismatching_user_name": name = "different_username@example.com" + case "ia_info_invalid_username": + name = "-invalid_username" case "ia_info_empty_group_name": group = "" case "ia_info_empty_ugid": diff --git a/internal/users/types/userinfo.go b/internal/users/types/userinfo.go index 54e44136ef..f9afc8d080 100644 --- a/internal/users/types/userinfo.go +++ b/internal/users/types/userinfo.go @@ -1,6 +1,29 @@ package types -import "github.com/canonical/authd/internal/sliceutils" +import ( + "fmt" + "regexp" + + "github.com/canonical/authd/internal/sliceutils" +) + +// usernameRegexp is the regexp that a valid username must match. +// It follows the Debian/Ubuntu username policy as defined by shadow-utils/useradd rules, +// extended to allow '@' and '.' characters for cloud identity provider email-style usernames +// (e.g. user@example.com). +var usernameRegexp = regexp.MustCompile(`^[a-z_][-a-z0-9_.@]*[$]?$`) + +// ValidateUsername checks if the given username is valid. +// Valid usernames follow the Debian/Ubuntu naming convention: they start with a lowercase letter or +// underscore, followed by lowercase letters, digits, hyphens, underscores, dots, or '@', with an +// optional trailing dollar sign. The '@' and '.' characters are also permitted to support +// email-style usernames used by cloud identity providers. +func ValidateUsername(name string) error { + if !usernameRegexp.MatchString(name) { + return fmt.Errorf("username %q is not valid: it must match %s", name, usernameRegexp) + } + return nil +} // Equals checks that two users are equal. func (u UserInfo) Equals(other UserInfo) bool { diff --git a/internal/users/types/userinfo_test.go b/internal/users/types/userinfo_test.go index 2e1e2703b1..561261f9a7 100644 --- a/internal/users/types/userinfo_test.go +++ b/internal/users/types/userinfo_test.go @@ -136,3 +136,65 @@ func TestUserInfoEquals(t *testing.T) { }) } } + +func TestValidateUsername(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + username string + wantErr bool + }{ + // Valid usernames + "Valid_lowercase_name": {username: "user"}, + "Valid_name_starting_with_underscore": {username: "_user"}, + "Valid_name_with_hyphen": {username: "user-name"}, + "Valid_name_with_underscore": {username: "user_name"}, + "Valid_name_with_digits": {username: "user1"}, + "Valid_name_with_trailing_dollar_sign": {username: "user$"}, + "Valid_single_character_name": {username: "a"}, + "Valid_name_with_mixed_allowed_chars": {username: "a-b_c0"}, + "Valid_email_style_name": {username: "user@example.com"}, + "Valid_email_style_name_with_subdomain": {username: "user@sub.example.com"}, + "Valid_name_with_dot": {username: "first.last"}, + + // Invalid usernames + "Error_on_empty_username": {username: "", wantErr: true}, + "Error_on_uppercase_character": {username: "User", wantErr: true}, + "Error_on_uppercase_email": {username: "User@example.com", wantErr: true}, + "Error_on_name_starting_with_digit": {username: "1user", wantErr: true}, + "Error_on_name_starting_with_hyphen": {username: "-user", wantErr: true}, + "Error_on_name_with_dollar_not_at_end": {username: "user$name", wantErr: true}, + "Error_on_name_with_space": {username: "user name", wantErr: true}, + + // Injection / path traversal characters must be rejected + "Error_on_name_with_slash": {username: "user/name", wantErr: true}, + "Error_on_name_with_backslash": {username: `user\name`, wantErr: true}, + "Error_on_name_with_single_quote": {username: "user'name", wantErr: true}, + "Error_on_name_with_double_quote": {username: `user"name`, wantErr: true}, + "Error_on_name_with_backtick": {username: "user`name", wantErr: true}, + "Error_on_name_with_semicolon": {username: "user;name", wantErr: true}, + "Error_on_name_with_ampersand": {username: "user&name", wantErr: true}, + "Error_on_name_with_pipe": {username: "user|name", wantErr: true}, + "Error_on_name_with_null_byte": {username: "user\x00name", wantErr: true}, + "Error_on_name_with_newline": {username: "user\nname", wantErr: true}, + "Error_on_name_with_tab": {username: "user\tname", wantErr: true}, + "Error_on_name_with_colon": {username: "user:name", wantErr: true}, + "Error_on_name_with_exclamation": {username: "user!name", wantErr: true}, + "Error_on_name_with_open_paren": {username: "user(name", wantErr: true}, + "Error_on_name_with_close_paren": {username: "user)name", wantErr: true}, + "Error_on_name_with_less_than": {username: "username", wantErr: true}, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + err := types.ValidateUsername(tc.username) + if tc.wantErr { + require.Error(t, err, "ValidateUsername should return an error for %q, but did not", tc.username) + return + } + require.NoError(t, err, "ValidateUsername should not return an error for %q, but did", tc.username) + }) + } +}