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 Dec 20, 2024
1 parent a200b50 commit 12c61bb
Show file tree
Hide file tree
Showing 111 changed files with 2,351 additions and 427 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
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
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 12c61bb

Please sign in to comment.