Skip to content

Commit

Permalink
Generate unique UIDs and GIDs
Browse files Browse the repository at this point in the history
* Remove obsolete tests: They were testing behavior which has changed.
* Fix tests by using either predictable UIDs or replacing UIDs in golden
  files with placeholders (normalizing)
* Limit number of pre-auth user records to avoid denial of service by
  sending many SSH requests for unique users to fill up the UID range.
* Remove field UID from users.UserInfo struct because it's not set in
  internal/brokers/broker.go anymore and there is no need to expose this
  field.
* Rename localgroups package to localentries because it now also
  provides functions for passwd entries.
* Check for error when calling getpwent or getgrent
  • Loading branch information
adombeck committed Jan 6, 2025
1 parent ea0d887 commit 0eb85a5
Show file tree
Hide file tree
Showing 121 changed files with 2,369 additions and 445 deletions.
6 changes: 3 additions & 3 deletions cmd/authd/integrationtests.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"strings"

"github.com/ubuntu/authd/internal/services/permissions"
"github.com/ubuntu/authd/internal/users/localgroups"
"github.com/ubuntu/authd/internal/users/localentries"
)

// load any behaviour modifiers from env variable.
Expand All @@ -21,6 +21,6 @@ func init() {
if gpasswdArgs == "" || grpFilePath == "" {
panic("AUTHD_INTEGRATIONTESTS_GPASSWD_ARGS and AUTHD_INTEGRATIONTESTS_GPASSWD_GRP_FILE_PATH must be set")
}
localgroups.Z_ForTests_SetGpasswdCmd(strings.Split(gpasswdArgs, " "))
localgroups.Z_ForTests_SetGroupPath(grpFilePath)
localentries.Z_ForTests_SetGpasswdCmd(strings.Split(gpasswdArgs, " "))
localentries.Z_ForTests_SetGroupPath(grpFilePath)
}
11 changes: 5 additions & 6 deletions examplebroker/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -930,12 +930,11 @@ func userInfoFromName(name string) string {
Groups []groupJSONInfo
Gecos string
}{
Name: name,
UUID: "uuid-" + name,
Home: "/home/" + name,
Shell: "/usr/bin/bash",
Groups: []groupJSONInfo{{Name: "group-" + name, UGID: "ugid-" + name}},
Gecos: "gecos for " + name,
Name: name,
UUID: "uuid-" + name,
Home: "/home/" + name,
Shell: "/usr/bin/bash",
Gecos: "gecos for " + name,
}

switch name {
Expand Down
10 changes: 5 additions & 5 deletions internal/brokers/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"github.com/ubuntu/authd/internal/brokers/auth"
"github.com/ubuntu/authd/internal/brokers/layouts"
"github.com/ubuntu/authd/internal/log"
"github.com/ubuntu/authd/internal/users"
"github.com/ubuntu/authd/internal/users/types"
"github.com/ubuntu/decorate"
"golang.org/x/exp/slices"
)
Expand Down Expand Up @@ -318,16 +318,16 @@ func (b Broker) parseSessionID(sessionID string) string {
}

// unmarshalUserInfo tries to unmarshal the rawMsg into a userinfo.
func unmarshalUserInfo(rawMsg json.RawMessage) (users.UserInfo, error) {
var u users.UserInfo
func unmarshalUserInfo(rawMsg json.RawMessage) (types.UserInfo, error) {
var u types.UserInfo
if err := json.Unmarshal(rawMsg, &u); err != nil {
return users.UserInfo{}, fmt.Errorf("message is not JSON formatted: %v", err)
return types.UserInfo{}, fmt.Errorf("message is not JSON formatted: %v", err)
}
return u, nil
}

// validateUserInfo checks if the specified userinfo is valid.
func validateUserInfo(uInfo users.UserInfo) (err error) {
func validateUserInfo(uInfo types.UserInfo) (err error) {
defer decorate.OnError(&err, "provided userinfo is invalid")

// Validate username. We don't want to check here if it matches the username from the request, because it's the
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FIRST CALL:
access:
data:
err: message is not JSON formatted: json: cannot unmarshal string into Go value of type users.UserInfo
err: message is not JSON formatted: json: cannot unmarshal string into Go value of type types.UserInfo
21 changes: 13 additions & 8 deletions internal/services/nss/nss.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/ubuntu/authd/internal/proto/authd"
"github.com/ubuntu/authd/internal/services/permissions"
"github.com/ubuntu/authd/internal/users"
"github.com/ubuntu/authd/internal/users/types"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)
Expand Down Expand Up @@ -180,19 +181,23 @@ func (s Service) userPreCheck(ctx context.Context, username string) (pwent *auth
return nil, fmt.Errorf("user %q is not known by any broker", username)
}

var u users.UserEntry
var u types.UserEntry
if err := json.Unmarshal([]byte(userinfo), &u); err != nil {
return nil, fmt.Errorf("user data from broker invalid: %v", err)
}
// We need to generate the ID for the user, as its business logic is authd responsibility, not the broker's.
u.UID = s.userManager.GenerateUID(u.Name)
u.GID = s.userManager.GenerateGID(u.Name)

// Register a temporary user with a unique UID. If the user authenticates successfully, the user will be added to
// the database with the same UID.
u.UID, err = s.userManager.RegisterUserPreAuth(u.Name)
if err != nil {
return nil, fmt.Errorf("failed to add temporary record for user %q: %v", username, err)
}

return nssPasswdFromUsersPasswd(u), nil
}

// nssPasswdFromUsersPasswd returns a PasswdEntry from users.UserEntry.
func nssPasswdFromUsersPasswd(u users.UserEntry) *authd.PasswdEntry {
func nssPasswdFromUsersPasswd(u types.UserEntry) *authd.PasswdEntry {
return &authd.PasswdEntry{
Name: u.Name,
Passwd: "x",
Expand All @@ -205,17 +210,17 @@ func nssPasswdFromUsersPasswd(u users.UserEntry) *authd.PasswdEntry {
}

// nssGroupFromUsersGroup returns a GroupEntry from users.GroupEntry.
func nssGroupFromUsersGroup(g users.GroupEntry) *authd.GroupEntry {
func nssGroupFromUsersGroup(g types.GroupEntry) *authd.GroupEntry {
return &authd.GroupEntry{
Name: g.Name,
Passwd: "x",
Passwd: g.Passwd,
Gid: g.GID,
Members: g.Users,
}
}

// nssShadowFromUsersShadow returns a ShadowEntry from users.ShadowEntry.
func nssShadowFromUsersShadow(u users.ShadowEntry) *authd.ShadowEntry {
func nssShadowFromUsersShadow(u types.ShadowEntry) *authd.ShadowEntry {
return &authd.ShadowEntry{
Name: u.Name,
Passwd: "x",
Expand Down
12 changes: 10 additions & 2 deletions internal/services/nss/nss_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import (
"github.com/ubuntu/authd/internal/testutils/golden"
"github.com/ubuntu/authd/internal/users"
"github.com/ubuntu/authd/internal/users/cache"
localgroupstestutils "github.com/ubuntu/authd/internal/users/localgroups/testutils"
"github.com/ubuntu/authd/internal/users/idgenerator"
localgroupstestutils "github.com/ubuntu/authd/internal/users/localentries/testutils"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/credentials/insecure"
Expand Down Expand Up @@ -332,7 +333,14 @@ func newUserManagerForTests(t *testing.T, sourceDB string) *users.Manager {
}
cache.Z_ForTests_CreateDBFromYAML(t, filepath.Join("testdata", sourceDB), cacheDir)

m, err := users.NewManager(users.DefaultConfig, cacheDir)
managerOpts := []users.Option{
users.WithIDGenerator(&idgenerator.IDGeneratorMock{
UIDsToGenerate: []uint32{1234},
GIDsToGenerate: []uint32{1234},
}),
}

m, err := users.NewManager(users.DefaultConfig, cacheDir, managerOpts...)
require.NoError(t, err, "Setup: could not create user manager")

t.Cleanup(func() { _ = m.Stop() })
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: group1
passwd: x
passwd: ""
gid: 11111
members:
- user1
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: group1
passwd: x
passwd: ""
gid: 11111
members:
- user1
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
- name: group1
passwd: x
passwd: ""
gid: 11111
members:
- user1
- name: group2
passwd: x
passwd: ""
gid: 22222
members:
- user2
- name: group3
passwd: x
passwd: ""
gid: 33333
members:
- user3
- name: commongroup
passwd: x
passwd: ""
gid: 99999
members:
- user2
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: user-pre-check
passwd: x
uid: 1053432963
gid: 1053432963
uid: 1234
gid: 0
gecos: gecos for user-pre-check
homedir: /home/user-pre-check
shell: /bin/sh/user-pre-check
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: User-Pre-Check
passwd: x
uid: 1053432963
gid: 1053432963
uid: 1234
gid: 0
gecos: gecos for User-Pre-Check
homedir: /home/User-Pre-Check
shell: /bin/sh/User-Pre-Check
3 changes: 2 additions & 1 deletion internal/services/pam/pam.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/ubuntu/authd/internal/proto/authd"
"github.com/ubuntu/authd/internal/services/permissions"
"github.com/ubuntu/authd/internal/users"
"github.com/ubuntu/authd/internal/users/types"
"github.com/ubuntu/decorate"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
Expand Down Expand Up @@ -262,7 +263,7 @@ func (s Service) IsAuthenticated(ctx context.Context, req *authd.IARequest) (res
}, nil
}

var uInfo users.UserInfo
var uInfo types.UserInfo
if err := json.Unmarshal([]byte(data), &uInfo); err != nil {
return nil, fmt.Errorf("user data from broker invalid: %v", err)
}
Expand Down
30 changes: 21 additions & 9 deletions internal/services/pam/pam_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ import (
"github.com/ubuntu/authd/internal/testutils/golden"
"github.com/ubuntu/authd/internal/users"
"github.com/ubuntu/authd/internal/users/cache"
localgroupstestutils "github.com/ubuntu/authd/internal/users/localgroups/testutils"
"github.com/ubuntu/authd/internal/users/idgenerator"
localgroupstestutils "github.com/ubuntu/authd/internal/users/localentries/testutils"
userstestutils "github.com/ubuntu/authd/internal/users/testutils"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"
Expand Down Expand Up @@ -433,11 +434,9 @@ func TestIsAuthenticated(t *testing.T) {
"Update local groups": {username: "success_with_local_groups", localGroupsFile: "valid.group"},

// service errors
"Error when not root": {username: "success", currentUserNotRoot: true},
"Error when UID conflicts with existing different user": {username: "conflicting-uid", existingDB: "cache-with-conflicting-uid.db"},
"Error when GID conflicts with existing different group": {username: "conflicting-gid", existingDB: "cache-with-conflicting-gid.db"},
"Error when sessionID is empty": {sessionID: "-"},
"Error when there is no broker": {sessionID: "invalid-session"},
"Error when not root": {username: "success", currentUserNotRoot: true},
"Error when sessionID is empty": {sessionID: "-"},
"Error when there is no broker": {sessionID: "invalid-session"},

// broker errors
"Error when authenticating": {username: "IA_error"},
Expand Down Expand Up @@ -466,7 +465,14 @@ func TestIsAuthenticated(t *testing.T) {
cache.Z_ForTests_CreateDBFromYAML(t, filepath.Join(testutils.TestFamilyPath(t), tc.existingDB), cacheDir)
}

m, err := users.NewManager(users.DefaultConfig, cacheDir)
managerOpts := []users.Option{
users.WithIDGenerator(&idgenerator.IDGeneratorMock{
UIDsToGenerate: []uint32{1111},
GIDsToGenerate: []uint32{1111, 2222},
}),
}

m, err := users.NewManager(users.DefaultConfig, cacheDir, managerOpts...)
require.NoError(t, err, "Setup: could not create user manager")
t.Cleanup(func() { _ = m.Stop() })
pm := newPermissionManager(t, false) // Allow starting the session (current user considered root)
Expand Down Expand Up @@ -550,13 +556,19 @@ func TestIDGeneration(t *testing.T) {
username string
}{
"Generate ID": {username: "success"},
"Generates same ID if user has upper cases in username": {username: "SuCcEsS"},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
t.Parallel()

m, err := users.NewManager(users.DefaultConfig, t.TempDir())
managerOpts := []users.Option{
users.WithIDGenerator(&idgenerator.IDGeneratorMock{
UIDsToGenerate: []uint32{1111},
GIDsToGenerate: []uint32{1111, 2222},
}),
}

m, err := users.NewManager(users.DefaultConfig, t.TempDir(), managerOpts...)
require.NoError(t, err, "Setup: could not create user manager")
t.Cleanup(func() { _ = m.Stop() })
pm := newPermissionManager(t, false) // Allow starting the session (current user considered root)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
GroupByID:
"1648262143": '{"Name":"TestIDGeneration_separator_success","GID":1648262143,"UGID":"TestIDGeneration_separator_success"}'
"1946747284": '{"Name":"group-success","GID":1946747284,"UGID":"ugid-success"}'
"1111": '{"Name":"TestIDGeneration_separator_success","GID":1111,"UGID":"TestIDGeneration_separator_success"}'
"2222": '{"Name":"group-success","GID":2222,"UGID":"ugid-success"}'
GroupByName:
TestIDGeneration_separator_success: '{"Name":"TestIDGeneration_separator_success","GID":1648262143,"UGID":"TestIDGeneration_separator_success"}'
group-success: '{"Name":"group-success","GID":1946747284,"UGID":"ugid-success"}'
TestIDGeneration_separator_success: '{"Name":"TestIDGeneration_separator_success","GID":1111,"UGID":"TestIDGeneration_separator_success"}'
group-success: '{"Name":"group-success","GID":2222,"UGID":"ugid-success"}'
GroupByUGID:
TestIDGeneration_separator_success: '{"Name":"TestIDGeneration_separator_success","GID":1648262143,"UGID":"TestIDGeneration_separator_success"}'
ugid-success: '{"Name":"group-success","GID":1946747284,"UGID":"ugid-success"}'
TestIDGeneration_separator_success: '{"Name":"TestIDGeneration_separator_success","GID":1111,"UGID":"TestIDGeneration_separator_success"}'
ugid-success: '{"Name":"group-success","GID":2222,"UGID":"ugid-success"}'
GroupToUsers:
"1648262143": '{"GID":1648262143,"UIDs":[1648262143]}'
"1946747284": '{"GID":1946747284,"UIDs":[1648262143]}'
"1111": '{"GID":1111,"UIDs":[1111]}'
"2222": '{"GID":2222,"UIDs":[1111]}'
UserByID:
"1648262143": '{"Name":"TestIDGeneration_separator_success","UID":1648262143,"GID":1648262143,"Gecos":"gecos for success","Dir":"/home/success","Shell":"/bin/sh/success","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}'
"1111": '{"Name":"TestIDGeneration_separator_success","UID":1111,"GID":1111,"Gecos":"gecos for success","Dir":"/home/success","Shell":"/bin/sh/success","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}'
UserByName:
TestIDGeneration_separator_success: '{"Name":"TestIDGeneration_separator_success","UID":1648262143,"GID":1648262143,"Gecos":"gecos for success","Dir":"/home/success","Shell":"/bin/sh/success","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}'
TestIDGeneration_separator_success: '{"Name":"TestIDGeneration_separator_success","UID":1111,"GID":1111,"Gecos":"gecos for success","Dir":"/home/success","Shell":"/bin/sh/success","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}'
UserToBroker: {}
UserToGroups:
"1648262143": '{"UID":1648262143,"GIDs":[1648262143,1946747284]}'
"1111": '{"UID":1111,"GIDs":[1111,2222]}'
UserToLocalGroups:
"1648262143": "null"
"1111": "null"
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ UserByName: {}
UserToBroker: {}
UserToGroups: {}
UserToLocalGroups:
"1042855937": '["localgroup1","localgroup3"]'
"1111": '["localgroup1","localgroup3"]'
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FIRST CALL:
access:
msg:
err: can't check authentication: message is not JSON formatted: json: cannot unmarshal string into Go value of type users.UserInfo
err: can't check authentication: message is not JSON formatted: json: cannot unmarshal string into Go value of type types.UserInfo
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
GroupByID:
"1369382419": '{"Name":"group-IA_second_call","GID":1369382419,"UGID":"ugid-IA_second_call"}'
"1556535091": '{"Name":"TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call","GID":1556535091,"UGID":"TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call"}'
"1111": '{"Name":"TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call","GID":1111,"UGID":"TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call"}'
"2222": '{"Name":"group-IA_second_call","GID":2222,"UGID":"ugid-IA_second_call"}'
GroupByName:
TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call: '{"Name":"TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call","GID":1556535091,"UGID":"TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call"}'
group-IA_second_call: '{"Name":"group-IA_second_call","GID":1369382419,"UGID":"ugid-IA_second_call"}'
TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call: '{"Name":"TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call","GID":1111,"UGID":"TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call"}'
group-IA_second_call: '{"Name":"group-IA_second_call","GID":2222,"UGID":"ugid-IA_second_call"}'
GroupByUGID:
TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call: '{"Name":"TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call","GID":1556535091,"UGID":"TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call"}'
ugid-IA_second_call: '{"Name":"group-IA_second_call","GID":1369382419,"UGID":"ugid-IA_second_call"}'
TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call: '{"Name":"TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call","GID":1111,"UGID":"TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call"}'
ugid-IA_second_call: '{"Name":"group-IA_second_call","GID":2222,"UGID":"ugid-IA_second_call"}'
GroupToUsers:
"1369382419": '{"GID":1369382419,"UIDs":[1556535091]}'
"1556535091": '{"GID":1556535091,"UIDs":[1556535091]}'
"1111": '{"GID":1111,"UIDs":[1111]}'
"2222": '{"GID":2222,"UIDs":[1111]}'
UserByID:
"1556535091": '{"Name":"TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call","UID":1556535091,"GID":1556535091,"Gecos":"gecos for IA_second_call","Dir":"/home/IA_second_call","Shell":"/bin/sh/IA_second_call","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}'
"1111": '{"Name":"TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call","UID":1111,"GID":1111,"Gecos":"gecos for IA_second_call","Dir":"/home/IA_second_call","Shell":"/bin/sh/IA_second_call","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}'
UserByName:
TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call: '{"Name":"TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call","UID":1556535091,"GID":1556535091,"Gecos":"gecos for IA_second_call","Dir":"/home/IA_second_call","Shell":"/bin/sh/IA_second_call","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}'
TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call: '{"Name":"TestIsAuthenticated/Error_when_calling_second_time_without_cancelling_separator_IA_second_call","UID":1111,"GID":1111,"Gecos":"gecos for IA_second_call","Dir":"/home/IA_second_call","Shell":"/bin/sh/IA_second_call","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}'
UserToBroker: {}
UserToGroups:
"1556535091": '{"UID":1556535091,"GIDs":[1556535091,1369382419]}'
"1111": '{"UID":1111,"GIDs":[1111,2222]}'
UserToLocalGroups:
"1556535091": "null"
"1111": "null"
Loading

0 comments on commit 0eb85a5

Please sign in to comment.