Add min_uid and max_uid configuration values.

README.md

@@ -80,6 +80,13 @@ The [default template](https://github.com/ubuntu/aad-auth/blob/main/conf/aad.con
 #                    ; %d - domain
 # shell = /bin/bash ; default shell for the user
 
+## UID range configuration
+## WARNING! Changing these values may effect existing users.
+min_uid = 100000 ; minimum UID to assign (inclusive)
+max_uid = 2147483647 ; maximum UID to assign (exclusive)
+                     ; the default max_uid is 4294967295 (max uint32), however this causes issues with commonly used software, such as xdg-desktop-portal-gnome
+                     ; this value (max int32) works with most software, while still providing a large UID space.
+
 ### overriding values for a specific domain, every value inside a section is optional
 # [domain.com]
 # tenant_id = aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa

conf/aad.conf.template

@@ -16,6 +16,13 @@
 #                    ; %d - domain
 # shell = /bin/bash ; default shell for the user
 
+## UID range configuration
+## WARNING! Changing these values may effect existing users.
+min_uid = 100000 ; minimum UID to assign (inclusive)
+max_uid = 2147483647 ; maximum UID to assign (exclusive)
+                     ; the default max_uid is 4294967295 (max uint32), however this causes issues with commonly used software, such as xdg-desktop-portal-gnome
+                     ; this value (max int32) works with most software, while still providing a large UID space.
+
 ### overriding values for a specific domain, every value inside a section is optional
 # [domain.com]
 # tenant_id = aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa

internal/cache/cache.go

@@ -29,6 +29,8 @@ var (
 	ErrOfflineCredentialsExpired = errors.New("offline credentials expired")
 	// ErrOfflineAuthDisabled is returned when offline authentication is disabled by using a negative value in aad.conf.
 	ErrOfflineAuthDisabled = errors.New("offline authentication is disabled")
+	// ErrNoUIDAvailable is returned when there is no available UID within the specified range
+	ErrNoUIDAvailable = errors.New("no uid available")
 )
 
 const (
@@ -330,13 +332,13 @@ func (c *Cache) CanAuthenticate(ctx context.Context, username, password string)
 }
 
 // Update creates and update user nss cache when there has been an online verification.
-func (c *Cache) Update(ctx context.Context, username, password, homeDirPattern, shell string) (err error) {
+func (c *Cache) Update(ctx context.Context, username, password, homeDirPattern, shell string, minUID, maxUID uint32) (err error) {
 	defer decorate.OnError(&err, i18n.G("couldn't create/open cache for nss database"))
 
 	user, err := c.GetUserByName(ctx, username)
 	if errors.Is(err, ErrNoEnt) {
 		// Try creating the user
-		id, err := c.generateUIDForUser(ctx, username)
+		id, err := c.generateUIDForUser(ctx, username, minUID, maxUID)
 		if err != nil {
 			return err
 		}
@@ -405,25 +407,30 @@ func encryptPassword(ctx context.Context, username, password string) (string, er
 }
 
 // generateUIDForUser returns an unique uid for the user to create.
-func (c *Cache) generateUIDForUser(ctx context.Context, username string) (uid uint32, err error) {
+func (c *Cache) generateUIDForUser(ctx context.Context, username string, minUID, maxUID uint32) (uid uint32, err error) {
 	defer decorate.OnError(&err, i18n.G("failed to generate uid for user %q"), username)
 
-	logger.Debug(ctx, "generate user id for user %q", username)
+	logger.Debug(ctx, "generate user id for user %q (min %d, max %d)", username, minUID, maxUID)
 
 	// compute uid for user
-	var offset uint32 = 100000
 	uid = 1
 	for _, c := range username {
 		uid = (uid * uint32(c)) % math.MaxUint32
 	}
-	uid = uid%(math.MaxUint32-offset) + offset
+	uid = uid%(maxUID-minUID) + minUID
 
 	// check collision or increment
+	initialUid := uid
 	for {
 		if exists, err := uidOrGidExists(c.db, uid, username); err != nil {
 			return 0, err
 		} else if exists {
-			uid++
+			// Loop round between offset and modulus until a uid is found
+			uid = (uid-minUID+1)%(maxUID-minUID) + minUID
+			// If we've cycled all the way round, we should error
+			if uid == initialUid {
+				return 0, ErrNoUIDAvailable
+			}
 			continue
 		}
 

internal/cache/cache_test.go

@@ -5,6 +5,7 @@ import (
 	"flag"
 	"fmt"
 	"io/fs"
+	"math"
 	"os"
 	"path/filepath"
 	"testing"
@@ -293,7 +294,7 @@ func TestUpdate(t *testing.T) {
 			var lastUID int64
 			for _, n := range tc.userNames {
 				start := time.Now()
-				err := c.Update(context.Background(), n, "my password", "/home/%f", "/bin/bash")
+				err := c.Update(context.Background(), n, "my password", "/home/%f", "/bin/bash", 100000, math.MaxUint32)
 				end := time.Now()
 				if tc.wantErr {
 					require.Error(t, err, "Update should have returned an error but hasn't")
@@ -327,7 +328,7 @@ func TestUpdate(t *testing.T) {
 				// we need one second as we are storing an unix timestamp for last online auth
 				time.Sleep(time.Second)
 
-				err = c.Update(context.Background(), n, "other password", "/home/%f", "/bin/bash")
+				err = c.Update(context.Background(), n, "other password", "/home/%f", "/bin/bash", 100000, math.MaxUint32)
 				if tc.wantErrRefresh {
 					require.Error(t, err, "Second update should have returned an error but hasn't")
 					return
@@ -542,6 +543,122 @@ func TestQueryPasswdAttribute(t *testing.T) {
 	}
 }
 
+func TestGenerateUIDForUser(t *testing.T) {
+	t.Parallel()
+
+	cacheDir := t.TempDir()
+	cacheDB := "users_in_db"
+	testutils.PrepareDBsForTests(t, cacheDir, cacheDB)
+	c := testutils.NewCacheForTests(t, cacheDir)
+
+	tests := []struct {
+		username string
+		offset   uint32
+		modulus  uint32
+		expected uint32
+	}{
+		// Default offset and modulus
+		{"[email protected]", 100000, math.MaxUint32, 1982826144},
+		{"[email protected]", 100000, math.MaxUint32, 155813536},
+		// An example of a collision
+		{"[email protected]", 100000, math.MaxUint32, 155813536},
+
+		// Test a smaller UID range
+		{"[email protected]", 2000, 3000, 0},
+		{"[email protected]", 2000, 3000, 0},
+		{"[email protected]", 2000, 3000, 0},
+		{"a", 2000, 30000, 0},
+		{"aa", 2000, 30000, 0},
+		{"aaa", 2000, 30000, 0},
+		{"aaaa", 2000, 30000, 0},
+		{"aaaaa", 2000, 30000, 0},
+		{"aaaaaa", 2000, 30000, 0},
+		{"aaaaaaa", 2000, 30000, 0},
+		{"aaaaaaaa", 2000, 30000, 0},
+	}
+	for _, tc := range tests {
+		tc := tc
+		t.Run(fmt.Sprint("generateUidFor ", tc.username, " from ", tc.offset, " to ", tc.modulus), func(t *testing.T) {
+			uid, err := c.GenerateUIDForUser(context.Background(), tc.username, tc.offset, tc.modulus)
+			require.NoError(t, err)
+			if tc.expected != 0 {
+				require.Equal(t, tc.expected, uid)
+			}
+			require.LessOrEqual(t, tc.offset, uid)
+			require.Greater(t, tc.modulus, uid)
+		})
+	}
+}
+
+func TestUpdateUIDs(t *testing.T) {
+	t.Parallel()
+
+	cacheDir := t.TempDir()
+	cacheDB := "users_in_db"
+	testutils.PrepareDBsForTests(t, cacheDir, cacheDB)
+	c := testutils.NewCacheForTests(t, cacheDir)
+
+	tests := []struct {
+		username string
+		offset   uint32
+		modulus  uint32
+		expected uint32
+		err      error
+	}{
+		// Default offset and modulus
+		{"[email protected]", 100000, math.MaxUint32, 1982826144, nil},
+		{"[email protected]", 100000, math.MaxUint32, 155813536, nil},
+		// A collision example, since it's using the database, it is incremented
+		{"[email protected]", 100000, math.MaxUint32, 155813537, nil},
+
+		// Test a smaller UID range
+		{"[email protected]", 2000, 30000, 0, nil},
+		{"[email protected]", 2000, 30000, 0, nil},
+		{"[email protected]", 2000, 30000, 0, nil},
+		{"a", 2000, 30000, 0, nil},
+		{"aa", 2000, 30000, 0, nil},
+		{"aaa", 2000, 30000, 0, nil},
+		{"aaaa", 2000, 30000, 0, nil},
+		{"aaaaa", 2000, 30000, 0, nil},
+		{"aaaaaa", 2000, 30000, 0, nil},
+		{"aaaaaaa", 2000, 30000, 0, nil},
+		{"aaaaaaaa", 2000, 30000, 0, nil},
+
+		// A short UID range causes an overflow
+		{"b", 1000, 1002, 1000, nil},
+		{"bb", 1000, 1002, 1001, nil},
+		{"bbb", 1000, 1002, 1002, cache.ErrNoUIDAvailable},
+
+		// We should also be able to handle overflow at the top of the uint32 space, and certainly not overflow to be root!!!
+		{"c", math.MaxUint32 - 2, math.MaxUint32, math.MaxUint32 - 1, nil},
+		{"cc", math.MaxUint32 - 2, math.MaxUint32, math.MaxUint32 - 2, nil},
+		{"ccc", math.MaxUint32 - 2, math.MaxUint32, 0, cache.ErrNoUIDAvailable},
+		{"cccc", math.MaxUint32 - 2, math.MaxUint32, 1, cache.ErrNoUIDAvailable},
+	}
+	for _, tc := range tests {
+		tc := tc
+		t.Run(fmt.Sprint("Update UID test for ", tc.username, " from ", tc.offset, " to ", tc.modulus), func(t *testing.T) {
+			// Update the user
+			err := c.Update(context.Background(), tc.username, "", "/home/%U", "/bin/sh", tc.offset, tc.modulus)
+			if tc.err == nil {
+				require.NoError(t, err, "Update should not error")
+			} else {
+				require.ErrorAs(t, err, &tc.err)
+				return
+			}
+			// Check the UID which was generated
+			uidInterface, err := c.QueryPasswdAttribute(context.Background(), tc.username, "uid")
+			uid := uint32(uidInterface.(int64))
+			require.NoError(t, err, "QueryPasswdAttribute should not error")
+			if tc.expected != 0 {
+				require.Equal(t, tc.expected, uid)
+			}
+			require.LessOrEqual(t, tc.offset, uid)
+			require.Greater(t, tc.modulus, uid)
+		})
+	}
+}
+
 func TestMain(m *testing.M) {
 	testutils.InstallUpdateFlag()
 	flag.Parse()

internal/cache/cache_uid_test.go

@@ -0,0 +1,9 @@
+package cache
+
+import (
+	"context"
+)
+
+func (c *Cache) GenerateUIDForUser(ctx context.Context, username string, minUID, maxUID uint32) (uint32, error) {
+	return c.generateUIDForUser(ctx, username, minUID, maxUID)
+}

internal/config/config.go

@@ -4,6 +4,7 @@ package config
 import (
 	"context"
 	"fmt"
+	"math"
 	"path/filepath"
 
 	"github.com/go-ini/ini"
@@ -26,6 +27,10 @@ type AAD struct {
 	OfflineCredentialsExpiration *int   `ini:"offline_credentials_expiration"`
 	HomeDirPattern               string `ini:"homedir"`
 	Shell                        string `ini:"shell"`
+	// MinUID is the minimum allowed UID to be assigned
+	MinUID uint32 `ini:"min_uid"`
+	// MaxUID is one above the maximum UID to be assigned
+	MaxUID uint32 `ini:"max_uid"`
 }
 
 // ToIni reflects the configuration values to an ini.File representation.
@@ -64,6 +69,8 @@ func Load(ctx context.Context, p, domain string, opts ...Option) (config AAD, er
 	config = AAD{
 		HomeDirPattern: defaultHomePattern,
 		Shell:          defaultShell,
+		MinUID:         100000,
+		MaxUID:         math.MaxUint32,
 	}
 
 	// Tries to load the defaults from the adduser.conf
@@ -93,6 +100,9 @@ func Load(ctx context.Context, p, domain string, opts ...Option) (config AAD, er
 	if config.AppID == "" {
 		return AAD{}, fmt.Errorf("missing required 'app_id' entry in configuration file")
 	}
+	if config.MinUID >= config.MaxUID {
+		return AAD{}, fmt.Errorf("'min_uid' must be less than 'max_uid' in configuration file")
+	}
 
 	return config, nil
 }

internal/pam/pam.go

@@ -107,7 +107,7 @@ func Authenticate(ctx context.Context, username, password, conf string, opts ...
 	}
 
 	// Successful online login, update cache.
-	if err := c.Update(ctx, username, password, cfg.HomeDirPattern, cfg.Shell); err != nil {
+	if err := c.Update(ctx, username, password, cfg.HomeDirPattern, cfg.Shell, cfg.MinUID, cfg.MaxUID); err != nil {
 		logError(ctx, i18n.G("%w. Denying access."), err)
 		return ErrPamAuth
 	}