diff --git a/README.md b/README.md index 6ecee87..5ea1453 100644 --- a/README.md +++ b/README.md @@ -46,20 +46,54 @@ bt cloud-config.yaml > butane.yaml is either converted completely or rejected; unsupported fields are never silently discarded. -| cloud-config field | Butane field | -| ----------------------- | --------------------- | -| `name` | `name` | -| `passwd` | `password_hash` | -| `gecos` | `gecos` | -| `homedir` | `home_dir` | -| `shell` | `shell` | -| `ssh_authorized_keys` | `ssh_authorized_keys` | - -Password hashes remain locked, matching cloud-init's default -`lock_passwd: true`. Supplementary and primary groups, sudo configuration, -password unlocking, and every non-`users` stanza are not yet supported. -Jinja templates are not evaluated and are rejected; render them before passing -the resulting cloud-config to `bt`. +| cloud-config stanza | Status | Butane output | +| ------------------- | ------ | ------------- | +| `users` | Supported Cluster API subset | `passwd`, plus generated `storage.files` | +| `write_files` | Unsupported | Planned `storage.files` | +| `runcmd` | Unsupported | Planned scripts and systemd units | +| `bootcmd` | Unsupported | Planned scripts and systemd units | +| `ntp` | Unsupported | Planned time-sync configuration | +| `disk_setup` | Unsupported | Planned storage configuration | +| `fs_setup` | Unsupported | Planned `storage.filesystems` | +| `mounts` | Unsupported | Planned filesystems or systemd mount units | + +| cloud-config field | Butane output | +| --------------------- | ------------- | +| `name` | `passwd.users[].name` | +| `passwd` | `passwd.users[].password_hash` | +| `gecos` | `passwd.users[].gecos` | +| `homedir` | `passwd.users[].home_dir` | +| `shell` | `passwd.users[].shell` | +| `ssh_authorized_keys` | `passwd.users[].ssh_authorized_keys` | +| `groups` | `passwd.users[].groups` and synthesized `passwd.groups` | +| `primary_group` | `passwd.users[].primary_group` and synthesized `passwd.groups` | +| `inactive` | `false` is a no-op; `true` is rejected | +| `lock_passwd` | Password locking and a generated sshd drop-in when `false` | +| `sudo` | A generated `/etc/sudoers.d/` file | + +The new fields accept only the shapes emitted by Cluster API: `groups` is a +comma-separated string, `primary_group` and `sudo` are non-empty strings, and +`inactive` and `lock_passwd` are booleans. Referenced groups are created before +users. Empty or duplicate supplementary groups are rejected, and a primary +group must not also be listed as a supplementary group. If a referenced group +has the same name as a configured user, that user must set `primary_group` +explicitly to avoid colliding with Linux's implicit private-group creation. + +Password hashes are locked when `lock_passwd` is omitted or `true`. When it is +`false`, the hash remains unlocked and `bt` enables SSH password authentication +for that user with a Flatcar sshd drop-in. An already-locked hash is rejected +when `lock_passwd` is `false`. Users that request password login or sudo must +have names matching `^[a-z_][a-z0-9_-]*$` so names cannot become SSH, sudoers, +or path syntax. + +Generated sshd and sudoers files are root-owned, mode 0600, and replace any +existing file at the generated path. Sudo rules are preserved verbatim and are +not checked for valid sudoers syntax. No sshd restart is needed because +Ignition writes the drop-in before sshd starts. + +Every non-`users` stanza remains unsupported. Jinja templates are not evaluated +and are rejected; render them before passing the resulting cloud-config to +`bt`. ### A Flatcar Container Linux project diff --git a/testcases/README.md b/testcases/README.md index e30ed8e..0246b70 100644 --- a/testcases/README.md +++ b/testcases/README.md @@ -19,6 +19,6 @@ boundary. | Fixture | Purpose | Current result | | --- | --- | --- | -| `cluster-api-supported-user.yaml` | Every currently supported user field | Success | -| `cluster-api-groups.yaml` | Cluster API's comma-separated groups and primary group | Rejected until group support lands | -| `cluster-api-deferred-fields.yaml` | Remaining rendered account-policy fields | Rejected as unsupported | +| `cluster-api-supported-user.yaml` | Cluster API's direct user-field mappings | Success | +| `cluster-api-groups.yaml` | Same group used as supplementary and primary | Rejected as contradictory | +| `cluster-api-deferred-fields.yaml` | Cluster API account-policy value shapes | Rejected because `inactive: true` cannot be represented | diff --git a/testcases/cluster-api-groups.yaml b/testcases/cluster-api-groups.yaml index 649015c..c255d21 100644 --- a/testcases/cluster-api-groups.yaml +++ b/testcases/cluster-api-groups.yaml @@ -1,5 +1,5 @@ #cloud-config -# Cluster API renders additional groups as one comma-separated scalar. +# This deliberately repeats the primary group as a supplementary group. users: - name: foo groups: "foo, bar" diff --git a/transpile.go b/transpile.go index ee8f831..9ebe643 100644 --- a/transpile.go +++ b/transpile.go @@ -82,9 +82,9 @@ func Transpile(input []byte) ([]byte, error) { } config := schema.Config{Variant: variant, Version: version} - if len(users) > 0 { - config.Passwd.Users = users - } + config.Passwd.Groups = users.Groups + config.Passwd.Users = users.Users + config.Storage.Files = users.Files out, err := yaml.MarshalWithOptions(config, yaml.OmitZero(), yaml.Indent(2), yaml.IndentSequence(true)) if err != nil { return nil, fmt.Errorf("encode Butane config: %w", err) diff --git a/users.go b/users.go index 92319b5..8926bb3 100644 --- a/users.go +++ b/users.go @@ -2,26 +2,48 @@ package transpile import ( "fmt" + "regexp" "strings" base "github.com/coreos/ignition/v2/butane/base/v0_5" "github.com/goccy/go-yaml/ast" ) -func parseUsers(document map[string]any, file *ast.File) ([]base.PasswdUser, ValidationErrors) { +var generatedConfigUsername = regexp.MustCompile(`^[a-z_][a-z0-9_-]*$`) + +type usersConfig struct { + Files []base.File + Groups []base.PasswdGroup + Users []base.PasswdUser +} + +type parsedUser struct { + PasswordLogin bool + Sudo *string + User base.PasswdUser +} + +type userDeclaration struct { + HasPrimaryGroup bool + Path string +} + +func parseUsers(document map[string]any, file *ast.File) (usersConfig, ValidationErrors) { + var config usersConfig var problems ValidationErrors rawUsers, exists := document["users"] if !exists { - return nil, problems + return config, problems } items, ok := rawUsers.([]any) if !ok { problems = append(problems, problem(file, "users", "must be a list")) - return nil, problems + return config, problems } - users := make([]base.PasswdUser, 0, len(items)) - seenUsers := map[string]struct{}{} + seenGroups := map[string]struct{}{} + declaredUsers := map[string]userDeclaration{} + var passwordLoginUsers []string for i, item := range items { path := fmt.Sprintf("users[%d]", i) fields, ok := item.(map[string]any) @@ -30,24 +52,61 @@ func parseUsers(document map[string]any, file *ast.File) ([]base.PasswdUser, Val continue } - user, userProblems := parseUser(fields, path, file) + parsed, userProblems := parseUser(fields, path, file) problems = append(problems, userProblems...) + user := parsed.User if user.Name != "" { - if _, duplicate := seenUsers[user.Name]; duplicate { + if _, duplicate := declaredUsers[user.Name]; duplicate { problems = append(problems, problem(file, path+".name", fmt.Sprintf("duplicate user %q", user.Name))) } else { - seenUsers[user.Name] = struct{}{} + declaredUsers[user.Name] = userDeclaration{ + HasPrimaryGroup: user.PrimaryGroup != nil, + Path: path, + } } } - users = append(users, user) + for _, group := range user.Groups { + config.Groups = appendGroup(config.Groups, seenGroups, string(group)) + } + if user.PrimaryGroup != nil { + config.Groups = appendGroup(config.Groups, seenGroups, *user.PrimaryGroup) + } + if parsed.PasswordLogin { + passwordLoginUsers = append(passwordLoginUsers, user.Name) + } + if parsed.Sudo != nil && user.Name != "" { + contents := fmt.Sprintf("%s %s\n", user.Name, *parsed.Sudo) + config.Files = append(config.Files, generatedFile("/etc/sudoers.d/"+user.Name, contents)) + } + config.Users = append(config.Users, user) + } + for _, group := range config.Groups { + user, matchesUser := declaredUsers[group.Name] + if matchesUser && !user.HasPrimaryGroup { + message := fmt.Sprintf("is required because group %q is explicitly referenced", group.Name) + problems = append(problems, problem(file, user.Path+".primary_group", message)) + } + } + if len(passwordLoginUsers) > 0 { + contents := fmt.Sprintf("Match User %s\n PasswordAuthentication yes\nMatch all\n", strings.Join(passwordLoginUsers, ",")) + config.Files = append(config.Files, generatedFile("/etc/ssh/sshd_config.d/20-butane-init-password-auth.conf", contents)) } - return users, problems + return config, problems } -func parseUser(fields map[string]any, path string, file *ast.File) (base.PasswdUser, ValidationErrors) { +func parseUser(fields map[string]any, path string, file *ast.File) (parsedUser, ValidationErrors) { allowed := map[string]bool{ - "name": true, "passwd": true, "gecos": true, "homedir": true, - "shell": true, "ssh_authorized_keys": true, + "name": true, + "passwd": true, + "gecos": true, + "homedir": true, + "shell": true, + "ssh_authorized_keys": true, + "groups": true, + "primary_group": true, + "inactive": true, + "lock_passwd": true, + "sudo": true, } var problems ValidationErrors for _, key := range sortedKeys(fields) { @@ -56,17 +115,93 @@ func parseUser(fields map[string]any, path string, file *ast.File) (base.PasswdU } } - var user base.PasswdUser + var result parsedUser + user := &result.User user.Name = requiredString(fields, "name", path, file, &problems) user.Gecos = optionalString(fields, "gecos", path, file, &problems) + user.Groups = supplementaryGroups(fields, path, file, &problems) user.HomeDir = optionalString(fields, "homedir", path, file, &problems) + user.PrimaryGroup = optionalString(fields, "primary_group", path, file, &problems) user.Shell = optionalString(fields, "shell", path, file, &problems) + result.Sudo = optionalString(fields, "sudo", path, file, &problems) + if inactive := optionalBool(fields, "inactive", path, file, &problems); inactive != nil && *inactive { + problems = append(problems, problem(file, path+".inactive", "true is unsupported")) + } + if user.PrimaryGroup != nil && containsGroup(user.Groups, *user.PrimaryGroup) { + problems = append(problems, problem(file, path+".primary_group", "must not also be a supplementary group")) + } + if lockPasswd := optionalBool(fields, "lock_passwd", path, file, &problems); lockPasswd != nil { + result.PasswordLogin = !*lockPasswd + } if passwd := optionalString(fields, "passwd", path, file, &problems); passwd != nil { - locked := lockPassword(*passwd) - user.PasswordHash = &locked + if result.PasswordLogin { + if passwordLocked(*passwd) { + problems = append(problems, problem(file, path+".passwd", "must not be locked when lock_passwd is false")) + } else { + user.PasswordHash = passwd + } + } else { + locked := lockPassword(*passwd) + user.PasswordHash = &locked + } } user.SSHAuthorizedKeys = sshKeys(fields, path, file, &problems) - return user, problems + if (result.PasswordLogin || result.Sudo != nil) && user.Name != "" && !generatedConfigUsername.MatchString(user.Name) { + problems = append(problems, problem(file, path+".name", "is unsafe for generated configuration")) + } + return result, problems +} + +func generatedFile(path, contents string) base.File { + mode := 0o600 + overwrite := true + return base.File{ + Overwrite: &overwrite, + Path: path, + Contents: base.Resource{Inline: &contents}, + Mode: &mode, + } +} + +func supplementaryGroups(fields map[string]any, path string, file *ast.File, problems *ValidationErrors) []base.Group { + raw := optionalString(fields, "groups", path, file, problems) + if raw == nil { + return nil + } + + var groups []base.Group + seen := map[string]struct{}{} + for _, item := range strings.Split(*raw, ",") { + name := strings.TrimSpace(item) + if name == "" { + *problems = append(*problems, problem(file, path+".groups", "must not contain an empty group name")) + continue + } + if _, duplicate := seen[name]; duplicate { + *problems = append(*problems, problem(file, path+".groups", fmt.Sprintf("duplicate group %q", name))) + continue + } + seen[name] = struct{}{} + groups = append(groups, base.Group(name)) + } + return groups +} + +func containsGroup(groups []base.Group, name string) bool { + for _, group := range groups { + if string(group) == name { + return true + } + } + return false +} + +func appendGroup(groups []base.PasswdGroup, seen map[string]struct{}, name string) []base.PasswdGroup { + if _, exists := seen[name]; exists { + return groups + } + seen[name] = struct{}{} + return append(groups, base.PasswdGroup{Name: name}) } func requiredString(fields map[string]any, key, path string, file *ast.File, problems *ValidationErrors) string { @@ -97,6 +232,19 @@ func optionalString(fields map[string]any, key, path string, file *ast.File, pro return &value } +func optionalBool(fields map[string]any, key, path string, file *ast.File, problems *ValidationErrors) *bool { + raw, exists := fields[key] + if !exists { + return nil + } + value, ok := raw.(bool) + if !ok { + *problems = append(*problems, problem(file, path+"."+key, "must be a boolean")) + return nil + } + return &value +} + func sshKeys(fields map[string]any, path string, file *ast.File, problems *ValidationErrors) []base.SSHAuthorizedKey { raw, exists := fields["ssh_authorized_keys"] if !exists { @@ -132,8 +280,12 @@ func sshKeys(fields map[string]any, path string, file *ast.File, problems *Valid } func lockPassword(hash string) string { - if strings.HasPrefix(hash, "!") || strings.HasPrefix(hash, "*") { + if passwordLocked(hash) { return hash } return "!" + hash } + +func passwordLocked(hash string) bool { + return strings.HasPrefix(hash, "!") || strings.HasPrefix(hash, "*") +} diff --git a/users_test.go b/users_test.go index a1990f6..49acf70 100644 --- a/users_test.go +++ b/users_test.go @@ -32,6 +32,202 @@ passwd: } } +func TestTranspileCreatesReferencedGroups(t *testing.T) { + input := `#cloud-config +users: + - name: alice + groups: "docker, developers" + primary_group: staff + - name: bob + groups: "developers, monitoring" + primary_group: docker +` + want := `version: 1.1.0 +variant: flatcar +passwd: + groups: + - name: docker + - name: developers + - name: staff + - name: monitoring + users: + - groups: + - docker + - developers + name: alice + primary_group: staff + - groups: + - developers + - monitoring + name: bob + primary_group: docker +` + + got, err := transpile.Transpile([]byte(input)) + if err != nil { + t.Fatalf("Transpile() error = %v", err) + } + if string(got) != want { + t.Fatalf("Transpile() output mismatch\nwant:\n%s\ngot:\n%s", want, got) + } +} + +func TestTranspileRejectsGroupCollidingWithImplicitUserGroup(t *testing.T) { + inputs := []string{ + "#cloud-config\nusers:\n - name: alice\n groups: bob\n - name: bob\n", + "#cloud-config\nusers:\n - name: bob\n - name: alice\n groups: bob\n", + } + + for _, input := range inputs { + got, err := transpile.Transpile([]byte(input)) + if err == nil { + t.Fatal("Transpile() error = nil, want an implicit user-group collision error") + } + if got != nil { + t.Fatalf("Transpile() output = %q, want nil", got) + } + if !strings.Contains(err.Error(), "primary_group: is required because group \"bob\" is explicitly referenced") { + t.Fatalf("Transpile() error = %q, want implicit user-group collision error", err) + } + } +} + +func TestTranspileAllowsExplicitSameNamePrimaryGroup(t *testing.T) { + input := `#cloud-config +users: + - name: bob + primary_group: bob + - name: alice + groups: bob +` + want := `version: 1.1.0 +variant: flatcar +passwd: + groups: + - name: bob + users: + - name: bob + primary_group: bob + - groups: + - bob + name: alice +` + + got, err := transpile.Transpile([]byte(input)) + if err != nil { + t.Fatalf("Transpile() error = %v", err) + } + if string(got) != want { + t.Fatalf("Transpile() output mismatch\nwant:\n%s\ngot:\n%s", want, got) + } +} + +func TestTranspileHandlesInactive(t *testing.T) { + falseInput := "#cloud-config\nusers:\n - name: alice\n inactive: false\n" + want := "version: 1.1.0\nvariant: flatcar\npasswd:\n users:\n - name: alice\n" + + got, err := transpile.Transpile([]byte(falseInput)) + if err != nil { + t.Fatalf("Transpile(inactive: false) error = %v", err) + } + if string(got) != want { + t.Fatalf("Transpile(inactive: false) output mismatch\nwant:\n%s\ngot:\n%s", want, got) + } + + trueInput := "#cloud-config\nusers:\n - name: alice\n inactive: true\n" + got, err = transpile.Transpile([]byte(trueInput)) + if err == nil { + t.Fatal("Transpile(inactive: true) error = nil, want an error") + } + if got != nil { + t.Fatalf("Transpile(inactive: true) output = %q, want nil", got) + } + if !strings.Contains(err.Error(), "users[0].inactive: true is unsupported") { + t.Fatalf("Transpile(inactive: true) error = %q, want unsupported inactivity error", err) + } +} + +func TestTranspileConfiguresPasswordLogin(t *testing.T) { + input := `#cloud-config +users: + - name: alice + passwd: "$6$ALICE" + lock_passwd: false + - name: bob + lock_passwd: false + - name: carol + passwd: "$6$CAROL" + lock_passwd: true +` + want := `version: 1.1.0 +variant: flatcar +passwd: + users: + - name: alice + password_hash: $6$ALICE + - name: bob + - name: carol + password_hash: "!$6$CAROL" +storage: + files: + - overwrite: true + path: /etc/ssh/sshd_config.d/20-butane-init-password-auth.conf + contents: + inline: | + Match User alice,bob + PasswordAuthentication yes + Match all + mode: 384 +` + + got, err := transpile.Transpile([]byte(input)) + if err != nil { + t.Fatalf("Transpile() error = %v", err) + } + if string(got) != want { + t.Fatalf("Transpile() output mismatch\nwant:\n%s\ngot:\n%s", want, got) + } +} + +func TestTranspileCreatesSudoersFiles(t *testing.T) { + input := `#cloud-config +users: + - name: alice + sudo: "ALL=(ALL) NOPASSWD:ALL" + - name: bob + sudo: "ALL=(root) /usr/bin/systemctl status kubelet" +` + want := `version: 1.1.0 +variant: flatcar +passwd: + users: + - name: alice + - name: bob +storage: + files: + - overwrite: true + path: /etc/sudoers.d/alice + contents: + inline: | + alice ALL=(ALL) NOPASSWD:ALL + mode: 384 + - overwrite: true + path: /etc/sudoers.d/bob + contents: + inline: | + bob ALL=(root) /usr/bin/systemctl status kubelet + mode: 384 +` + + got, err := transpile.Transpile([]byte(input)) + if err != nil { + t.Fatalf("Transpile() error = %v", err) + } + if string(got) != want { + t.Fatalf("Transpile() output mismatch\nwant:\n%s\ngot:\n%s", want, got) + } +} + func TestTranspileRejectsInvalidUsers(t *testing.T) { tests := map[string]struct { input string @@ -39,12 +235,68 @@ func TestTranspileRejectsInvalidUsers(t *testing.T) { }{ "Cluster API groups fixture": { input: readFixture(t, "cluster-api-groups.yaml"), - wantErr: "groups", + wantErr: "primary_group: must not also be a supplementary group", }, "Cluster API deferred fields fixture": { input: readFixture(t, "cluster-api-deferred-fields.yaml"), wantErr: "inactive", }, + "empty supplementary group": { + input: "#cloud-config\nusers:\n - name: alice\n groups: \"docker,,wheel\"\n", + wantErr: "empty group name", + }, + "duplicate supplementary group": { + input: "#cloud-config\nusers:\n - name: alice\n groups: \"docker, docker\"\n", + wantErr: "duplicate group", + }, + "groups must be a scalar": { + input: "#cloud-config\nusers:\n - name: alice\n groups: [docker]\n", + wantErr: "users[0].groups: must be a string", + }, + "inactive must be a boolean": { + input: "#cloud-config\nusers:\n - name: alice\n inactive: \"false\"\n", + wantErr: "users[0].inactive: must be a boolean", + }, + "lock_passwd must be a boolean": { + input: "#cloud-config\nusers:\n - name: alice\n lock_passwd: \"false\"\n", + wantErr: "users[0].lock_passwd: must be a boolean", + }, + "sudo must be a scalar": { + input: "#cloud-config\nusers:\n - name: alice\n sudo: [\"ALL=(ALL) ALL\"]\n", + wantErr: "users[0].sudo: must be a string", + }, + "locked password with password login": { + input: "#cloud-config\nusers:\n - name: alice\n passwd: \"!$6$HASH\"\n lock_passwd: false\n", + wantErr: "must not be locked when lock_passwd is false", + }, + "star-locked password with password login": { + input: "#cloud-config\nusers:\n - name: alice\n passwd: \"*LK*\"\n lock_passwd: false\n", + wantErr: "users[0].passwd: must not be locked when lock_passwd is false", + }, + "unsafe sudoers username": { + input: "#cloud-config\nusers:\n - name: ALL\n sudo: \"ALL=(ALL) NOPASSWD:ALL\"\n", + wantErr: "users[0].name: is unsafe for generated configuration", + }, + "ignored sudoers filename": { + input: "#cloud-config\nusers:\n - name: alice.admin\n sudo: \"ALL=(ALL) NOPASSWD:ALL\"\n", + wantErr: "users[0].name: is unsafe for generated configuration", + }, + "sudoers path traversal": { + input: "#cloud-config\nusers:\n - name: ../alice\n sudo: \"ALL=(ALL) NOPASSWD:ALL\"\n", + wantErr: "users[0].name: is unsafe for generated configuration", + }, + "SSH username pattern": { + input: "#cloud-config\nusers:\n - name: alice,bob\n lock_passwd: false\n", + wantErr: "users[0].name: is unsafe for generated configuration", + }, + "SSH username negation": { + input: "#cloud-config\nusers:\n - name: '!alice'\n lock_passwd: false\n", + wantErr: "users[0].name: is unsafe for generated configuration", + }, + "SSH username control character": { + input: "#cloud-config\nusers:\n - name: \"alice\\nbob\"\n lock_passwd: false\n", + wantErr: "users[0].name: is unsafe for generated configuration", + }, "empty optional field": { input: "#cloud-config\nusers:\n - name: alice\n shell: \"\"\n", wantErr: "users[0].shell",