Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various fixes to allow uid to be in a correct range #159

Merged
merged 2 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ require (
github.com/hashicorp/go-multierror v1.1.1
github.com/itchyny/gojq v0.12.15
github.com/joho/godotenv v1.5.1
github.com/mauromorales/xpasswd v0.3.1
github.com/mauromorales/xpasswd v0.4.0
github.com/moby/moby v24.0.9+incompatible
github.com/mudler/entities v0.0.0-20240611135956-f8f11ba52c2f
github.com/mudler/entities v0.0.0-20240625130751-3d7f84082f3a
github.com/onsi/ginkgo/v2 v2.17.1
github.com/onsi/gomega v1.33.0
github.com/pkg/errors v0.9.1
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ github.com/mattn/go-colorable v0.1.1/go.mod h1:FuOcm+DKB9mbwrcAfNl7/TZVBZ6rcncea
github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=
github.com/mattn/go-isatty v0.0.5/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s=
github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s=
github.com/mauromorales/xpasswd v0.3.1 h1:mVPGISfzN/WaCUjYRFiDgIREb2NMfwgPSj3LS6QMm0Q=
github.com/mauromorales/xpasswd v0.3.1/go.mod h1:Z3+aY19mhNfcGi3st0+RAVSz2vC+pyoju2S/FPN8kEg=
github.com/mauromorales/xpasswd v0.4.0 h1:Jf6mfA8lwQsYzwgfQADPDGV7l/liAvRrnG+nQTPy0j8=
github.com/mauromorales/xpasswd v0.4.0/go.mod h1:Z3+aY19mhNfcGi3st0+RAVSz2vC+pyoju2S/FPN8kEg=
github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE=
github.com/mitchellh/copystructure v1.0.0/go.mod h1:SNtv71yrdKgLRyLFxmLdkAbkKEFWgYaq1OVrnRcwhnw=
github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa15WveJJGw=
Expand All @@ -149,8 +149,8 @@ github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zx
github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
github.com/moby/moby v24.0.9+incompatible h1:Z/hFbZJqC5Fmuf6jesMLdHU71CMAgdiSJ1ZYey+bFmg=
github.com/moby/moby v24.0.9+incompatible/go.mod h1:fDXVQ6+S340veQPv35CzDahGBmHsiclFwfEygB/TWMc=
github.com/mudler/entities v0.0.0-20240611135956-f8f11ba52c2f h1:5x1OeWl4gSW3L9KzntvWphjrcHASXq3gI351BgBbXjs=
github.com/mudler/entities v0.0.0-20240611135956-f8f11ba52c2f/go.mod h1:TXMcB82+CBF3fEQhIch/gFSuWVcItz4BdTfomYGW1jg=
github.com/mudler/entities v0.0.0-20240625130751-3d7f84082f3a h1:IeKeUwMeqfGYblesrk7Gu72z1xsJrceiIDDES4Rj3+U=
github.com/mudler/entities v0.0.0-20240625130751-3d7f84082f3a/go.mod h1:7bNIR64mpm7ld4bnDQ+LjHBPhUDAqoBQuv4aK53UGSs=
github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A=
github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE=
github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU=
Expand Down
49 changes: 27 additions & 22 deletions pkg/plugins/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,30 +69,15 @@ func createUser(fs vfs.FS, u schema.User, console Console) error {
}

primaryGroup := u.Name
gid := 1000

gid := -1 // -1 instructs entities to find the next free id and assign it
if u.PrimaryGroup != "" {
gr, err := osuser.LookupGroup(u.PrimaryGroup)
if err != nil {
return errors.Wrap(err, "could not resolve primary group of user")
}
gid, _ = strconv.Atoi(gr.Gid)
primaryGroup = u.PrimaryGroup
} else {
Copy link
Collaborator Author

@jimmykarily jimmykarily Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not need anymore. That's what the updateGroup.Apply will do if the gid is -1.

// Create a new group after the user name
all, _ := entities.ParseGroup(etcgroup)
if len(all) != 0 {
usedGids := []int{}
for _, entry := range all {
usedGids = append(usedGids, *entry.Gid)
}
sort.Ints(usedGids)
if len(usedGids) == 0 {
return errors.New("no new guid found")
}
gid = usedGids[len(usedGids)-1]
gid++
}
}

updateGroup := entities.Group{
Expand All @@ -101,9 +86,22 @@ func createUser(fs vfs.FS, u schema.User, console Console) error {
Gid: &gid,
Users: u.Name,
}
updateGroup.Apply(etcgroup, false)
err = updateGroup.Apply(etcgroup, false)
if err != nil {
return errors.Wrap(err, "creating the user's group")
}

uid := 1000
// reload the group to get the generated GID
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wouldn't be needed if the ParseGroup had a pointer receiver. I don't want to change that here though to avoid breaking other things. We can consider changing it in the future to avoid this second parsing of groups.

groups, _ := entities.ParseGroup(etcgroup)
for name, group := range groups {
if name == updateGroup.Name {
updateGroup = group
gid = *group.Gid
break
}
}

uid := -1
if u.UID != "" {
// User defined-uid
uid, err = strconv.Atoi(u.UID)
Expand All @@ -123,9 +121,16 @@ func createUser(fs vfs.FS, u schema.User, console Console) error {
return errors.Wrap(err, "could not get user id")
}
} else {
uid = list.GenerateUID()
// https://systemd.io/UIDS-GIDS/#special-distribution-uid-ranges
uid, err = list.GenerateUIDInRange(entities.HumanIDMin, entities.HumanIDMax)
if err != nil {
return errors.Wrap(err, "no available uid")
}
}
}
if uid == -1 {
return errors.New("could not set uid for user")
}

if u.Homedir == "" {
u.Homedir = fmt.Sprintf("%s/%s", usrDefaults["HOME"], u.Name)
Expand Down Expand Up @@ -162,7 +167,7 @@ func createUser(fs vfs.FS, u schema.User, console Console) error {
os.Chown(homedir, uid, gid)
}

groups, _ := entities.ParseGroup(etcgroup)
groups, _ = entities.ParseGroup(etcgroup)
for name, group := range groups {
for _, w := range u.Groups {
if w == name {
Expand Down Expand Up @@ -205,11 +210,11 @@ func User(l logger.Interface, s schema.Stage, fs vfs.FS, console Console) error
for _, k := range users {
r := s.Users[k]
r.Name = k
if !s.Users[k].Exists() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exists() was never true because the Name on the s.Users[k] wasn't set. It's set on r above though.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch

if !r.Exists() {
if err := createUser(fs, r, console); err != nil {
errs = multierror.Append(errs, err)
}
} else if s.Users[k].PasswordHash != "" {
} else if r.PasswordHash != "" {
if err := setUserPass(fs, r.Name, r.PasswordHash); err != nil {
return err
}
Expand Down
Loading