From 44123f16b7bb4efdd210049d1458181331560778 Mon Sep 17 00:00:00 2001 From: Saqib Ali Date: Mon, 11 Apr 2022 12:47:55 -0400 Subject: [PATCH 1/4] v1_5_exp: Add sugar to create user.cfg We want to add sugar to butane which will allow users to indirectly modify the GRUB configuration. The sugar added here will abstract the mounting of /boot and will allow users to create /boot/grub2/user.cfg which is sourced by grub.cfg. related: https://github.com/coreos/fedora-coreos-tracker/issues/134 --- config/common/errors.go | 4 + config/fcos/v1_5_exp/schema.go | 10 ++ config/fcos/v1_5_exp/translate.go | 66 +++++++++++++ config/fcos/v1_5_exp/translate_test.go | 130 +++++++++++++++++++++++++ config/fcos/v1_5_exp/validate.go | 12 +++ config/fcos/v1_5_exp/validate_test.go | 44 +++++++++ 6 files changed, 266 insertions(+) diff --git a/config/common/errors.go b/config/common/errors.go index cefbfb27..1961f007 100644 --- a/config/common/errors.go +++ b/config/common/errors.go @@ -76,4 +76,8 @@ var ( // Extensions ErrExtensionNameRequired = errors.New("field \"name\" is required") + + // Grub + ErrGrubUserNameNotSpecified = errors.New("field \"name\" is required") + ErrGrubPasswordNotSpecified = errors.New("field \"password_hash\" is required") ) diff --git a/config/fcos/v1_5_exp/schema.go b/config/fcos/v1_5_exp/schema.go index fc174dac..d985413d 100644 --- a/config/fcos/v1_5_exp/schema.go +++ b/config/fcos/v1_5_exp/schema.go @@ -22,6 +22,7 @@ type Config struct { base.Config `yaml:",inline"` BootDevice BootDevice `yaml:"boot_device"` Extensions []Extension `yaml:"extensions"` + Grub Grub `yaml:"grub"` } type BootDevice struct { @@ -43,3 +44,12 @@ type BootDeviceMirror struct { type Extension struct { Name string `yaml:"name"` } + +type Grub struct { + Users []GrubUser `yaml:"users"` +} + +type GrubUser struct { + Name string `yaml:"name"` + PasswordHash *string `yaml:"password_hash"` +} diff --git a/config/fcos/v1_5_exp/translate.go b/config/fcos/v1_5_exp/translate.go index e3fdbbae..f2eec2fd 100644 --- a/config/fcos/v1_5_exp/translate.go +++ b/config/fcos/v1_5_exp/translate.go @@ -18,6 +18,7 @@ import ( "crypto/sha256" "encoding/hex" "fmt" + "strings" baseutil "github.com/coreos/butane/base/util" "github.com/coreos/butane/config/common" @@ -86,6 +87,11 @@ func (c Config) ToIgn3_4Unvalidated(options common.TranslateOptions) (types.Conf retConfig, ts := baseutil.MergeTranslatedConfigs(retp, tsp, ret, ts) ret = retConfig.(types.Config) r.Merge(rp) + + retp, tsp, rp = c.handleUserGrubCfg(options) + retConfig, ts = baseutil.MergeTranslatedConfigs(retp, tsp, ret, ts) + ret = retConfig.(types.Config) + r.Merge(rp) return ret, ts, r } @@ -348,3 +354,63 @@ func (c Config) processPackages(options common.TranslateOptions) (types.Config, ts.AddFromCommonSource(yamlPath, path.New("json", "storage"), ret.Storage) return ret, ts, r } + +func (c Config) handleUserGrubCfg(options common.TranslateOptions) (types.Config, translate.TranslationSet, report.Report) { + rendered := types.Config{} + ts := translate.NewTranslationSet("yaml", "json") + var r report.Report + yamlPath := path.New("yaml", "grub", "users") + if len(c.Grub.Users) == 0 { + // No users + return rendered, ts, r + } + + // create boot filesystem + rendered.Storage.Filesystems = append(rendered.Storage.Filesystems, + types.Filesystem{ + Device: "/dev/disk/by-label/boot", + Format: util.StrToPtr("ext4"), + Path: util.StrToPtr("/boot"), + }) + + userCfgContent := []byte(buildGrubConfig(c.Grub)) + src, compression, err := baseutil.MakeDataURL(userCfgContent, nil, !options.NoResourceAutoCompression) + if err != nil { + r.AddOnError(yamlPath, err) + return rendered, ts, r + } + + // Create user.cfg file and add it to rendered config + rendered.Storage.Files = append(rendered.Storage.Files, + types.File{ + Node: types.Node{ + Path: "/boot/grub2/user.cfg", + }, + FileEmbedded1: types.FileEmbedded1{ + Append: []types.Resource{ + { + Source: util.StrToPtr(src), + Compression: compression, + }, + }, + }, + }) + + ts.AddFromCommonSource(yamlPath, path.New("json", "storage"), rendered.Storage) + return rendered, ts, r +} + +func buildGrubConfig(gb Grub) string { + // Process super users and corresponding passwords + allUsers := []string{} + cmds := []string{} + + for _, user := range gb.Users { + // We have already validated that user.Name and user.PasswordHash are non-empty + allUsers = append(allUsers, user.Name) + // Command for setting users password + cmds = append(cmds, fmt.Sprintf("password_pbkdf2 %s %s", user.Name, *user.PasswordHash)) + } + superUserCmd := fmt.Sprintf("set superusers=\"%s\"\n", strings.Join(allUsers, " ")) + return "# Generated by Butane\n\n" + superUserCmd + strings.Join(cmds, "\n") + "\n" +} diff --git a/config/fcos/v1_5_exp/translate_test.go b/config/fcos/v1_5_exp/translate_test.go index fa0f2577..fbb3f479 100644 --- a/config/fcos/v1_5_exp/translate_test.go +++ b/config/fcos/v1_5_exp/translate_test.go @@ -1487,3 +1487,133 @@ func TestTranslateExtensions(t *testing.T) { }) } } + +// TestTranslateGrub tests translating the Butane config Grub section. +func TestTranslateGrub(t *testing.T) { + // Some tests below have the same translations + translations := []translate.Translation{ + {path.New("yaml", "version"), path.New("json", "ignition", "version")}, + {path.New("yaml", "grub", "users"), path.New("json", "storage")}, + {path.New("yaml", "grub", "users"), path.New("json", "storage", "filesystems")}, + {path.New("yaml", "grub", "users"), path.New("json", "storage", "filesystems", 0)}, + {path.New("yaml", "grub", "users"), path.New("json", "storage", "filesystems", 0, "path")}, + {path.New("yaml", "grub", "users"), path.New("json", "storage", "filesystems", 0, "device")}, + {path.New("yaml", "grub", "users"), path.New("json", "storage", "filesystems", 0, "format")}, + {path.New("yaml", "grub", "users"), path.New("json", "storage", "files")}, + {path.New("yaml", "grub", "users"), path.New("json", "storage", "files", 0)}, + {path.New("yaml", "grub", "users"), path.New("json", "storage", "files", 0, "path")}, + {path.New("yaml", "grub", "users"), path.New("json", "storage", "files", 0, "append")}, + {path.New("yaml", "grub", "users"), path.New("json", "storage", "files", 0, "append", 0)}, + {path.New("yaml", "grub", "users"), path.New("json", "storage", "files", 0, "append", 0, "source")}, + {path.New("yaml", "grub", "users"), path.New("json", "storage", "files", 0, "append", 0, "compression")}, + } + tests := []struct { + in Config + out types.Config + exceptions []translate.Translation + report report.Report + }{ + // config with 1 user + { + Config{ + Grub: Grub{ + Users: []GrubUser{ + { + Name: "root", + PasswordHash: util.StrToPtr("grub.pbkdf2.sha512.10000.874A958E526409..."), + }, + }, + }, + }, + types.Config{ + Ignition: types.Ignition{ + Version: "3.4.0-experimental", + }, + Storage: types.Storage{ + Filesystems: []types.Filesystem{ + { + Device: "/dev/disk/by-label/boot", + Format: util.StrToPtr("ext4"), + Path: util.StrToPtr("/boot"), + }, + }, + Files: []types.File{ + { + Node: types.Node{ + Path: "/boot/grub2/user.cfg", + }, + FileEmbedded1: types.FileEmbedded1{ + Append: []types.Resource{ + { + Source: util.StrToPtr("data:,%23%20Generated%20by%20Butane%0A%0Aset%20superusers%3D%22root%22%0Apassword_pbkdf2%20root%20grub.pbkdf2.sha512.10000.874A958E526409...%0A"), + Compression: util.StrToPtr(""), + }, + }, + }, + }, + }, + }, + }, + translations, + report.Report{}, + }, + // config with 2 users (and 2 different hashes) + { + Config{ + Grub: Grub{ + Users: []GrubUser{ + { + Name: "root1", + PasswordHash: util.StrToPtr("grub.pbkdf2.sha512.10000.874A958E526409..."), + }, + { + Name: "root2", + PasswordHash: util.StrToPtr("grub.pbkdf2.sha512.10000.874B829D126209..."), + }, + }, + }, + }, + types.Config{ + Ignition: types.Ignition{ + Version: "3.4.0-experimental", + }, + Storage: types.Storage{ + Filesystems: []types.Filesystem{ + { + Device: "/dev/disk/by-label/boot", + Format: util.StrToPtr("ext4"), + Path: util.StrToPtr("/boot"), + }, + }, + Files: []types.File{ + { + Node: types.Node{ + Path: "/boot/grub2/user.cfg", + }, + FileEmbedded1: types.FileEmbedded1{ + Append: []types.Resource{ + { + Source: util.StrToPtr("data:;base64,H4sIAAAAAAAC/3zMsQrCMBDG8b1PcdT9SI62JoODRfExJCGngtCEuwTx7UWyiss3fH/47eDCG0uonCC+YW01bDwMyhW0FZamLHoYJedq4bs0DiWovrKka4nPdCPo8S4tYn9QH2G2hNYYY9Dtp6Of3XmmZTIeEX8C9BdYHfmTpYU68AkAAP//Mp8bt7YAAAA="), + Compression: util.StrToPtr("gzip"), + }, + }, + }, + }, + }, + }, + }, + translations, + report.Report{}, + }, + } + + for i, test := range tests { + t.Run(fmt.Sprintf("translate %d", i), func(t *testing.T) { + actual, translations, r := test.in.ToIgn3_4Unvalidated(common.TranslateOptions{}) + assert.Equal(t, test.out, actual, "translation mismatch") + assert.Equal(t, test.report, r, "report mismatch") + baseutil.VerifyTranslations(t, translations, test.exceptions) + assert.NoError(t, translations.DebugVerifyCoverage(actual), "incomplete TranslationSet coverage") + }) + } +} diff --git a/config/fcos/v1_5_exp/validate.go b/config/fcos/v1_5_exp/validate.go index 97b2907a..61cf290d 100644 --- a/config/fcos/v1_5_exp/validate.go +++ b/config/fcos/v1_5_exp/validate.go @@ -16,6 +16,7 @@ package v1_5_exp import ( "github.com/coreos/butane/config/common" + "github.com/coreos/ignition/v2/config/util" "github.com/coreos/vcontext/path" "github.com/coreos/vcontext/report" @@ -46,3 +47,14 @@ func (e Extension) Validate(c path.ContextPath) (r report.Report) { } return } + +func (user GrubUser) Validate(c path.ContextPath) (r report.Report) { + if user.Name == "" { + r.AddOnError(c.Append("name"), common.ErrGrubUserNameNotSpecified) + } + + if !util.NotEmpty(user.PasswordHash) { + r.AddOnError(c.Append("password_hash"), common.ErrGrubPasswordNotSpecified) + } + return +} diff --git a/config/fcos/v1_5_exp/validate_test.go b/config/fcos/v1_5_exp/validate_test.go index 413ff202..b2fa3971 100644 --- a/config/fcos/v1_5_exp/validate_test.go +++ b/config/fcos/v1_5_exp/validate_test.go @@ -228,3 +228,47 @@ func TestValidateExtension(t *testing.T) { }) } } + +func TestValidateGrubUser(t *testing.T) { + tests := []struct { + in GrubUser + out error + errPath path.ContextPath + }{ + // valid user + { + in: GrubUser{ + Name: "name", + PasswordHash: util.StrToPtr("pkcs5-pass"), + }, + out: nil, + errPath: path.New("yaml"), + }, + // username is not specified + { + in: GrubUser{ + Name: "", + PasswordHash: util.StrToPtr("pkcs5-pass"), + }, + out: common.ErrGrubUserNameNotSpecified, + errPath: path.New("yaml", "name"), + }, + // password is not specified + { + in: GrubUser{ + Name: "name", + }, + out: common.ErrGrubPasswordNotSpecified, + errPath: path.New("yaml", "password_hash"), + }, + } + + for i, test := range tests { + t.Run(fmt.Sprintf("validate %d", i), func(t *testing.T) { + actual := test.in.Validate(path.New("yaml")) + expected := report.Report{} + expected.AddOnError(test.errPath, test.out) + assert.Equal(t, expected, actual, "bad report") + }) + } +} From 8d3b20f0b5b41a964e87e1cc8f82b7b4d0ded740 Mon Sep 17 00:00:00 2001 From: Saqib Ali Date: Thu, 12 May 2022 13:05:22 -0400 Subject: [PATCH 2/4] docs: document new 'grub' field Let's document the newly added GRUB functionality and add an example config showing users how to use it. --- docs/config-fcos-v1_5-exp.md | 4 ++++ docs/examples.md | 14 ++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/docs/config-fcos-v1_5-exp.md b/docs/config-fcos-v1_5-exp.md index b4a0068a..338fe01a 100644 --- a/docs/config-fcos-v1_5-exp.md +++ b/docs/config-fcos-v1_5-exp.md @@ -208,6 +208,10 @@ The Fedora CoreOS configuration is a YAML document conforming to the following s * **_devices_** (list of strings): the list of whole-disk devices (not partitions) to include in the disk array, referenced by their absolute path. At least two devices must be specified. * **_extensions_** (list of objects): the list of additional packages to be installed. * **name** (string): the name of the package. +* **_grub_** (object): describes the desired GRUB bootloader configuration. + * **_users_** (list of objects): the list of GRUB superusers. + * **name** (string): the user name. + * **password_hash** (string): the PBKDF2 password hash. [part-types]: http://en.wikipedia.org/wiki/GUID_Partition_Table#Partition_type_GUIDs [rfc2397]: https://tools.ietf.org/html/rfc2397 diff --git a/docs/examples.md b/docs/examples.md index b7ee80cb..58be0ef8 100644 --- a/docs/examples.md +++ b/docs/examples.md @@ -372,6 +372,20 @@ systemd: [Install] WantedBy=multi-user.target ``` +## GRUB password + +This example adds a superuser to GRUB and sets a password. Users without the given username +and password will not be able to access GRUB command line, modify kernel command-line arguments, or boot non-default OSTree deployments. + + +```yaml +variant: fcos +version: 1.5.0-experimental +grub: + users: + - name: root + password_hash: grub.pbkdf2.sha512.10000.874A958E5264... +``` [spec]: specs.md [dropins]: https://www.freedesktop.org/software/systemd/man/systemd.unit.html#Description From 72825ae54b15c33a231f23417a375999c31c5839 Mon Sep 17 00:00:00 2001 From: Saqib Ali Date: Mon, 30 May 2022 10:23:45 -0400 Subject: [PATCH 3/4] translate/set.go: Add AddFromCommonObject helper This is a useful function for copying all the fields of one object to the identically named fields of another object. We add translations prefixed by fromPrefix and toPrefix, retrieving the subpaths from to. --- translate/set.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/translate/set.go b/translate/set.go index a48a80d7..49afefd4 100644 --- a/translate/set.go +++ b/translate/set.go @@ -105,6 +105,19 @@ func (ts TranslationSet) AddFromCommonSource(common path.ContextPath, toPrefix p ts.AddTranslation(common, toPrefix) } +// AddFromCommonObject adds translations for all of the paths in to. The paths being translated +// are prefixed by fromPrefix and the translated paths are prefixed by toPrefix. +// This is useful when we want to copy all the fields of an object to another with the same field names. +func (ts TranslationSet) AddFromCommonObject(fromPrefix path.ContextPath, toPrefix path.ContextPath, to interface{}) { + vTo := reflect.ValueOf(to) + vPaths := getAllPaths(vTo, ts.ToTag, true) + + for _, path := range vPaths { + ts.AddTranslation(prefixPath(path, fromPrefix.Path...), prefixPath(path, toPrefix.Path...)) + } + ts.AddTranslation(fromPrefix, toPrefix) +} + // Merge adds all the entries to the set. It mutates the Set in place. func (ts TranslationSet) Merge(from TranslationSet) { for _, t := range from.Set { From 460bee48ede7bd8811a0d4a7fc6c48a78024bf4a Mon Sep 17 00:00:00 2001 From: Saqib Ali Date: Thu, 19 May 2022 16:50:39 -0400 Subject: [PATCH 4/4] v_4_12_exp/translate.go: add special handling for GRUB user.cfg Sugar in FCOS V1.5.0-exp creates a user.cfg file using append; however, append is forbidden for MCO. Let's add special handling that converts user.cfg's append to contents for Openshift V4.12.0-exp. --- config/openshift/v4_12_exp/translate.go | 24 ++++++ config/openshift/v4_12_exp/translate_test.go | 83 ++++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/config/openshift/v4_12_exp/translate.go b/config/openshift/v4_12_exp/translate.go index c8126138..39b7441c 100644 --- a/config/openshift/v4_12_exp/translate.go +++ b/config/openshift/v4_12_exp/translate.go @@ -46,6 +46,7 @@ func (c Config) ToMachineConfig4_12Unvalidated(options common.TranslateOptions) if r.IsFatal() { return result.MachineConfig{}, ts, r } + ts = translateUserGrubCfg(&cfg, &ts) // wrap ts = ts.PrefixPaths(path.New("yaml"), path.New("json", "spec", "config")) @@ -295,3 +296,26 @@ func validateMCOSupport(mc result.MachineConfig, ts translate.TranslationSet) re } return cutil.TranslateReportPaths(r, ts) } + +// fcos config generates a user.cfg file using append; however, OpenShift config +// does not support append (since MCO does not support it). Let change the file to use contents +func translateUserGrubCfg(config *types.Config, ts *translate.TranslationSet) translate.TranslationSet { + newMappings := translate.NewTranslationSet("json", "json") + for i, file := range config.Storage.Files { + if file.Path == "/boot/grub2/user.cfg" { + if len(file.Append) != 1 { + // The number of append objects was different from expected, this file + // was created by the user and not via butane GRUB sugar + return *ts + } + fromPath := path.New("json", "storage", "files", i, "append", 0) + translatedPath := path.New("json", "storage", "files", i, "contents") + config.Storage.Files[i].FileEmbedded1.Contents = file.Append[0] + config.Storage.Files[i].FileEmbedded1.Append = nil + newMappings.AddFromCommonObject(fromPath, translatedPath, config.Storage.Files[i].FileEmbedded1.Contents) + + return ts.Map(newMappings) + } + } + return *ts +} diff --git a/config/openshift/v4_12_exp/translate_test.go b/config/openshift/v4_12_exp/translate_test.go index 70963877..c45b0217 100644 --- a/config/openshift/v4_12_exp/translate_test.go +++ b/config/openshift/v4_12_exp/translate_test.go @@ -271,6 +271,89 @@ func TestTranslateConfig(t *testing.T) { {path.New("yaml", "openshift", "fips"), path.New("json", "spec", "fips")}, }, }, + // Test Grub config + { + Config{ + Metadata: Metadata{ + Name: "z", + Labels: map[string]string{ + ROLE_LABEL_KEY: "z", + }, + }, + Config: fcos.Config{ + Grub: fcos.Grub{ + Users: []fcos.GrubUser{ + { + Name: "root", + PasswordHash: util.StrToPtr("grub.pbkdf2.sha512.10000.874A958E526409..."), + }, + }, + }, + }, + }, + result.MachineConfig{ + ApiVersion: result.MC_API_VERSION, + Kind: result.MC_KIND, + Metadata: result.Metadata{ + Name: "z", + Labels: map[string]string{ + ROLE_LABEL_KEY: "z", + }, + }, + Spec: result.Spec{ + Config: types.Config{ + Ignition: types.Ignition{ + Version: "3.4.0-experimental", + }, + Storage: types.Storage{ + Filesystems: []types.Filesystem{ + { + Device: "/dev/disk/by-label/boot", + Format: util.StrToPtr("ext4"), + Path: util.StrToPtr("/boot"), + }, + }, + Files: []types.File{ + { + Node: types.Node{ + Path: "/boot/grub2/user.cfg", + }, + FileEmbedded1: types.FileEmbedded1{ + Contents: types.Resource{ + Source: util.StrToPtr("data:,%23%20Generated%20by%20Butane%0A%0Aset%20superusers%3D%22root%22%0Apassword_pbkdf2%20root%20grub.pbkdf2.sha512.10000.874A958E526409...%0A"), + Compression: util.StrToPtr(""), + }, + }, + }, + }, + }, + }, + }, + }, + []translate.Translation{ + {path.New("yaml", "version"), path.New("json", "apiVersion")}, + {path.New("yaml", "version"), path.New("json", "kind")}, + {path.New("yaml", "version"), path.New("json", "spec")}, + {path.New("yaml"), path.New("json", "spec", "config")}, + {path.New("yaml", "ignition"), path.New("json", "spec", "config", "ignition")}, + {path.New("yaml", "version"), path.New("json", "spec", "config", "ignition", "version")}, + {path.New("yaml", "grub", "users"), path.New("json", "spec", "config", "storage")}, + {path.New("yaml", "grub", "users"), path.New("json", "spec", "config", "storage", "filesystems")}, + {path.New("yaml", "grub", "users"), path.New("json", "spec", "config", "storage", "filesystems", 0)}, + {path.New("yaml", "grub", "users"), path.New("json", "spec", "config", "storage", "filesystems", 0, "path")}, + {path.New("yaml", "grub", "users"), path.New("json", "spec", "config", "storage", "filesystems", 0, "device")}, + {path.New("yaml", "grub", "users"), path.New("json", "spec", "config", "storage", "filesystems", 0, "format")}, + {path.New("yaml", "grub", "users"), path.New("json", "spec", "config", "storage", "files")}, + {path.New("yaml", "grub", "users"), path.New("json", "spec", "config", "storage", "files", 0)}, + {path.New("yaml", "grub", "users"), path.New("json", "spec", "config", "storage", "files", 0, "path")}, + // "append" field is a remnant of translations performed in fcos config + // TODO: add a delete function to translation.TranslationSet and delete "append" translation + {path.New("yaml", "grub", "users"), path.New("json", "spec", "config", "storage", "files", 0, "append")}, + {path.New("yaml", "grub", "users"), path.New("json", "spec", "config", "storage", "files", 0, "contents")}, + {path.New("yaml", "grub", "users"), path.New("json", "spec", "config", "storage", "files", 0, "contents", "source")}, + {path.New("yaml", "grub", "users"), path.New("json", "spec", "config", "storage", "files", 0, "contents", "compression")}, + }, + }, } for i, test := range tests {