From 7cc2db4b8ef4514fb2bdc337dca2bc2231a183c6 Mon Sep 17 00:00:00 2001 From: Gianluca Zuccarelli Date: Mon, 29 Jul 2024 13:36:41 +0100 Subject: [PATCH] pkg/oscap: refactor the oscap configs The oscap tailoring and remediation configs were too tightly coupled with the osbuild stage implementations. These configs can be refactored further by making the `TailoringConfig` a pointer on the `RemediationConfig`, reversing the dependency of the configs. These changes should reduce the complexity of the OpenSCAP stage options and hopefully reduce some of the confusion too. --- pkg/customizations/oscap/oscap.go | 56 +++++++++----------------- pkg/customizations/oscap/oscap_test.go | 51 +++++++++-------------- pkg/distro/fedora/images.go | 3 +- pkg/distro/rhel/images.go | 3 +- pkg/manifest/os.go | 22 ++++------ pkg/osbuild/oscap_autotailor_stage.go | 31 ++++++++------ pkg/osbuild/oscap_remediation_stage.go | 18 ++++++--- 7 files changed, 77 insertions(+), 107 deletions(-) diff --git a/pkg/customizations/oscap/oscap.go b/pkg/customizations/oscap/oscap.go index 43dbc47e29..f9d39c1ff8 100644 --- a/pkg/customizations/oscap/oscap.go +++ b/pkg/customizations/oscap/oscap.go @@ -50,70 +50,52 @@ const ( type RemediationConfig struct { Datastream string ProfileID string - TailoringPath string CompressionEnabled bool + TailoringConfig *TailoringConfig } type TailoringConfig struct { - RemediationConfig TailoredProfileID string JSONFilepath string + TailoringPath string Selected []string Unselected []string } -func newTailoringConfigs(profileID string, tc blueprint.OpenSCAPTailoringCustomizations, remediationConfig RemediationConfig) (*RemediationConfig, *TailoringConfig, error) { - // the tailoring config needs to know about the original base profile id, - // but the remediation config needs to know the updated profile id. - remediationConfig.ProfileID = fmt.Sprintf("%s_osbuild_tailoring", remediationConfig.ProfileID) - remediationConfig.TailoringPath = filepath.Join(DataDir, "tailoring.xml") - - tailoringConfig := &TailoringConfig{ - RemediationConfig: RemediationConfig{ - ProfileID: profileID, - Datastream: remediationConfig.Datastream, - TailoringPath: remediationConfig.TailoringPath, - }, +func (rc *RemediationConfig) addTailoringConfigs(tc blueprint.OpenSCAPTailoringCustomizations) (*RemediationConfig, error) { + rc.TailoringConfig = &TailoringConfig{ + TailoredProfileID: fmt.Sprintf("%s_osbuild_tailoring", rc.ProfileID), Selected: tc.Selected, Unselected: tc.Unselected, - TailoredProfileID: remediationConfig.ProfileID, + TailoringPath: filepath.Join(DataDir, "tailoring.xml"), } - return &remediationConfig, tailoringConfig, nil + return rc, nil } -func newJsonConfigs(profileID string, json blueprint.OpenSCAPJSONTailoringCustomizations, remediationConfig RemediationConfig) (*RemediationConfig, *TailoringConfig, error) { +func (rc *RemediationConfig) addJsonConfigs(json blueprint.OpenSCAPJSONTailoringCustomizations) (*RemediationConfig, error) { if json.Filepath == "" { - return nil, nil, fmt.Errorf("Filepath to an JSON tailoring file is required") + return nil, fmt.Errorf("Filepath to an JSON tailoring file is required") } if json.ProfileID == "" { - return nil, nil, fmt.Errorf("Tailoring profile ID is required for an JSON tailoring file") + return nil, fmt.Errorf("Tailoring profile ID is required for an JSON tailoring file") } - // the tailoring config needs to know about the original base profile id, - // but the remediation config needs to know the updated profile id. - remediationConfig.ProfileID = json.ProfileID - remediationConfig.TailoringPath = filepath.Join(DataDir, "tailoring.xml") - - tailoringConfig := &TailoringConfig{ - RemediationConfig: RemediationConfig{ - ProfileID: profileID, - Datastream: remediationConfig.Datastream, - TailoringPath: remediationConfig.TailoringPath, - }, + rc.TailoringConfig = &TailoringConfig{ JSONFilepath: json.Filepath, TailoredProfileID: json.ProfileID, + TailoringPath: filepath.Join(DataDir, "tailoring.xml"), } - return &remediationConfig, tailoringConfig, nil + return rc, nil } -func NewConfigs(oscapConfig blueprint.OpenSCAPCustomization, defaultDatastream *string) (*RemediationConfig, *TailoringConfig, error) { +func NewConfigs(oscapConfig blueprint.OpenSCAPCustomization, defaultDatastream *string) (*RemediationConfig, error) { var datastream = oscapConfig.DataStream if datastream == "" { if defaultDatastream == nil { - return nil, nil, fmt.Errorf("No OSCAP datastream specified and the distro does not have any default set") + return nil, fmt.Errorf("No OSCAP datastream specified and the distro does not have any default set") } datastream = *defaultDatastream } @@ -129,13 +111,13 @@ func NewConfigs(oscapConfig blueprint.OpenSCAPCustomization, defaultDatastream * switch { case tc != nil && json != nil: - return nil, nil, fmt.Errorf("Multiple tailoring types set, only one type can be chosen (JSON/Override rules)") + return nil, fmt.Errorf("Multiple tailoring types set, only one type can be chosen (JSON/Override rules)") case tc != nil: - return newTailoringConfigs(oscapConfig.ProfileID, *tc, remediationConfig) + return remediationConfig.addTailoringConfigs(*tc) case json != nil: - return newJsonConfigs(oscapConfig.ProfileID, *json, remediationConfig) + return remediationConfig.addJsonConfigs(*json) default: - return &remediationConfig, nil, nil + return &remediationConfig, nil } } diff --git a/pkg/customizations/oscap/oscap_test.go b/pkg/customizations/oscap/oscap_test.go index 39c68dc1bf..f33bbfd7aa 100644 --- a/pkg/customizations/oscap/oscap_test.go +++ b/pkg/customizations/oscap/oscap_test.go @@ -11,11 +11,10 @@ import ( func TestOscapConfigGeneration(t *testing.T) { tests := []struct { - name string - options blueprint.OpenSCAPCustomization - expectedRemediation *RemediationConfig - expectedTailoring *TailoringConfig - err error + name string + options blueprint.OpenSCAPCustomization + expected *RemediationConfig + err error }{ { name: "no-datastream", @@ -59,20 +58,15 @@ func TestOscapConfigGeneration(t *testing.T) { ProfileID: "some-tailored-id", }, }, - expectedRemediation: &RemediationConfig{ + expected: &RemediationConfig{ Datastream: "datastream", - ProfileID: "some-tailored-id", - TailoringPath: filepath.Join(DataDir, "tailoring.xml"), + ProfileID: "some-profile-id", CompressionEnabled: true, - }, - expectedTailoring: &TailoringConfig{ - RemediationConfig: RemediationConfig{ - Datastream: "datastream", - ProfileID: "some-profile-id", - TailoringPath: filepath.Join(DataDir, "tailoring.xml"), + TailoringConfig: &TailoringConfig{ + TailoringPath: filepath.Join(DataDir, "tailoring.xml"), + JSONFilepath: "/some/filepath.json", + TailoredProfileID: "some-tailored-id", }, - JSONFilepath: "/some/filepath.json", - TailoredProfileID: "some-tailored-id", }, err: nil, }, @@ -86,37 +80,30 @@ func TestOscapConfigGeneration(t *testing.T) { Unselected: []string{"two", "four"}, }, }, - expectedRemediation: &RemediationConfig{ + expected: &RemediationConfig{ Datastream: "datastream", - ProfileID: "some-profile-id_osbuild_tailoring", - TailoringPath: filepath.Join(DataDir, "tailoring.xml"), + ProfileID: "some-profile-id", CompressionEnabled: true, - }, - expectedTailoring: &TailoringConfig{ - RemediationConfig: RemediationConfig{ - Datastream: "datastream", - ProfileID: "some-profile-id", - TailoringPath: filepath.Join(DataDir, "tailoring.xml"), + TailoringConfig: &TailoringConfig{ + TailoringPath: filepath.Join(DataDir, "tailoring.xml"), + TailoredProfileID: "some-profile-id_osbuild_tailoring", + Selected: []string{"one", "three"}, + Unselected: []string{"two", "four"}, }, - TailoredProfileID: "some-profile-id_osbuild_tailoring", - Selected: []string{"one", "three"}, - Unselected: []string{"two", "four"}, }, err: nil, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - remediationConfig, tailoringConfig, err := NewConfigs(tt.options, nil) + remediationConfig, err := NewConfigs(tt.options, nil) if tt.err != nil { assert.NotNil(t, err) assert.EqualValues(t, tt.err, err) assert.Nil(t, remediationConfig) - assert.Nil(t, tailoringConfig) } else { assert.NoError(t, err) - assert.EqualValues(t, tt.expectedRemediation, remediationConfig) - assert.EqualValues(t, tt.expectedTailoring, tailoringConfig) + assert.EqualValues(t, tt.expected, remediationConfig) } }) } diff --git a/pkg/distro/fedora/images.go b/pkg/distro/fedora/images.go index 3c6c2f8214..c40c42e943 100644 --- a/pkg/distro/fedora/images.go +++ b/pkg/distro/fedora/images.go @@ -194,12 +194,11 @@ func osCustomizations( } osc.Directories = append(osc.Directories, oscapDataNode) - remediationConfig, tailoringConfig, err := oscap.NewConfigs(*oscapConfig, imageConfig.DefaultOSCAPDatastream) + remediationConfig, err := oscap.NewConfigs(*oscapConfig, imageConfig.DefaultOSCAPDatastream) if err != nil { panic(fmt.Errorf("error creating OpenSCAP configs: %w", err)) } - osc.OpenSCAPTailorConfig = tailoringConfig osc.OpenSCAPRemediationConfig = remediationConfig } diff --git a/pkg/distro/rhel/images.go b/pkg/distro/rhel/images.go index 9cfa8d83a6..5fa8af35cd 100644 --- a/pkg/distro/rhel/images.go +++ b/pkg/distro/rhel/images.go @@ -215,12 +215,11 @@ func osCustomizations( } osc.Directories = append(osc.Directories, oscapDataNode) - remediationConfig, tailoringConfig, err := oscap.NewConfigs(*oscapConfig, imageConfig.DefaultOSCAPDatastream) + remediationConfig, err := oscap.NewConfigs(*oscapConfig, imageConfig.DefaultOSCAPDatastream) if err != nil { panic(fmt.Errorf("error creating OpenSCAP configs: %w", err)) } - osc.OpenSCAPTailorConfig = tailoringConfig osc.OpenSCAPRemediationConfig = remediationConfig } diff --git a/pkg/manifest/os.go b/pkg/manifest/os.go index 3cd176ce55..811d98f0cd 100644 --- a/pkg/manifest/os.go +++ b/pkg/manifest/os.go @@ -129,7 +129,6 @@ type OSCustomizations struct { ContainersStorage *string // OpenSCAP config - OpenSCAPTailorConfig *oscap.TailoringConfig OpenSCAPRemediationConfig *oscap.RemediationConfig Subscription *subscription.ImageOptions @@ -324,7 +323,7 @@ func (p *OS) getBuildPackages(distro Distro) []string { packages = append(packages, "skopeo") } - if p.OpenSCAPTailorConfig != nil { + if p.OpenSCAPRemediationConfig != nil && p.OpenSCAPRemediationConfig.TailoringConfig != nil { packages = append(packages, "openscap-utils") } @@ -807,22 +806,15 @@ func (p *OS) serialize() osbuild.Pipeline { } } - if p.OpenSCAPTailorConfig != nil { - if p.OpenSCAPRemediationConfig == nil { - // This is a programming error, since it doesn't make sense - // to have tailoring configs without openscap config. - panic(fmt.Errorf("OpenSCAP autotailoring cannot be set if no OpenSCAP config has been provided")) - } - - tailoringStageOpts := osbuild.NewOscapAutotailorStageOptions(p.OpenSCAPTailorConfig) - pipeline.AddStage(osbuild.NewOscapAutotailorStage(tailoringStageOpts)) - } - // NOTE: We need to run the OpenSCAP stages as the last stage before SELinux // since the remediation may change file permissions and other aspects of the // hardened image - if p.OpenSCAPRemediationConfig != nil { - remediationStageOpts := osbuild.NewOscapRemediationStageOptions(oscap.DataDir, p.OpenSCAPRemediationConfig) + if remediationConfig := p.OpenSCAPRemediationConfig; remediationConfig != nil { + if remediationConfig.TailoringConfig != nil { + tailoringStageOpts := osbuild.NewOscapAutotailorStageOptions(remediationConfig) + pipeline.AddStage(osbuild.NewOscapAutotailorStage(tailoringStageOpts)) + } + remediationStageOpts := osbuild.NewOscapRemediationStageOptions(oscap.DataDir, remediationConfig) pipeline.AddStage(osbuild.NewOscapRemediationStage(remediationStageOpts)) } diff --git a/pkg/osbuild/oscap_autotailor_stage.go b/pkg/osbuild/oscap_autotailor_stage.go index 7a7294a29f..cf75e3be84 100644 --- a/pkg/osbuild/oscap_autotailor_stage.go +++ b/pkg/osbuild/oscap_autotailor_stage.go @@ -73,36 +73,41 @@ func NewOscapAutotailorStage(options *OscapAutotailorStageOptions) *Stage { } } -func NewOscapAutotailorStageOptions(options *oscap.TailoringConfig) *OscapAutotailorStageOptions { +func NewOscapAutotailorStageOptions(options *oscap.RemediationConfig) *OscapAutotailorStageOptions { if options == nil { return nil } + tailoringConfig := options.TailoringConfig + if tailoringConfig == nil { + return nil + } + // TODO: don't panic! unfortunately this would involve quite // a big refactor and we still need to be a bit defensive here - if options.RemediationConfig.TailoringPath == "" { + if tailoringConfig.TailoringPath == "" { panic(fmt.Errorf("The tailoring path for the OpenSCAP remediation config cannot be empty, this is a programming error")) } - if options.JSONFilepath != "" { + if tailoringConfig.JSONFilepath != "" { return &OscapAutotailorStageOptions{ - Filepath: options.RemediationConfig.TailoringPath, + Filepath: tailoringConfig.TailoringPath, Config: AutotailorJSONConfig{ - TailoredProfileID: options.TailoredProfileID, - Datastream: options.RemediationConfig.Datastream, - TailoringFile: options.JSONFilepath, + Datastream: options.Datastream, + TailoredProfileID: tailoringConfig.TailoredProfileID, + TailoringFile: tailoringConfig.JSONFilepath, }, } } return &OscapAutotailorStageOptions{ - Filepath: options.RemediationConfig.TailoringPath, + Filepath: tailoringConfig.TailoringPath, Config: AutotailorKeyValueConfig{ - NewProfile: options.TailoredProfileID, - Datastream: options.RemediationConfig.Datastream, - ProfileID: options.RemediationConfig.ProfileID, - Selected: options.Selected, - Unselected: options.Unselected, + Datastream: options.Datastream, + ProfileID: options.ProfileID, + NewProfile: tailoringConfig.TailoredProfileID, + Selected: tailoringConfig.Selected, + Unselected: tailoringConfig.Unselected, }, } } diff --git a/pkg/osbuild/oscap_remediation_stage.go b/pkg/osbuild/oscap_remediation_stage.go index fb51094fbc..04112613a1 100644 --- a/pkg/osbuild/oscap_remediation_stage.go +++ b/pkg/osbuild/oscap_remediation_stage.go @@ -81,13 +81,19 @@ func NewOscapRemediationStageOptions(dataDir string, options *oscap.RemediationC return nil } + config := OscapConfig{ + ProfileID: options.ProfileID, + Datastream: options.Datastream, + Compression: options.CompressionEnabled, + } + + if tc := options.TailoringConfig; tc != nil { + config.ProfileID = tc.TailoredProfileID + config.Tailoring = tc.TailoringPath + } + return &OscapRemediationStageOptions{ DataDir: dataDir, - Config: OscapConfig{ - ProfileID: options.ProfileID, - Datastream: options.Datastream, - Tailoring: options.TailoringPath, - Compression: options.CompressionEnabled, - }, + Config: config, } }