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.

Sign up for GitHub

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

[ASCII-2691] Change wording on FIPS Mode status indication #33507

Merged
merged 9 commits into from
Jan 29, 2025
Merged

[ASCII-2691] Change wording on FIPS Mode status indication #33507

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
624f9d9
Fix wording of the status indicator of FIPS Mode
jeremy-hanna Jan 28, 2025
582a1dd
Merge branch 'main' into jeremy.hanna/change-wording-on-fips-status
jeremy-hanna Jan 28, 2025
3b59004
Include a FIPS Mode status indication for the regular agent builds
jeremy-hanna Jan 28, 2025
86d85ed
Add e2e tests for checking the FIPS Mode displays correctly for regul…
jeremy-hanna Jan 28, 2025
9a9f740
Merge branch 'main' into jeremy.hanna/change-wording-on-fips-status
jeremy-hanna Jan 28, 2025
f876802
Fix linter errors
jeremy-hanna Jan 28, 2025
da11535
Fix failing unit tests with normal agent status display
jeremy-hanna Jan 29, 2025
d5a5ebd
Fix compliance test with new wording
jeremy-hanna Jan 29, 2025
698c4db
Merge branch 'main' into jeremy.hanna/change-wording-on-fips-status
jeremy-hanna Jan 29, 2025
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
10 changes: 9 additions & 1 deletion comp/core/status/statusimpl/common_header_provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ func (h *headerProvider) HTML(_ bool, buffer io.Writer) error {
func (h *headerProvider) data() map[string]interface{} {
data := maps.Clone(h.constdata)
data["time_nano"] = nowFunc().UnixNano()
data["fips_status"] = fips.Status()
data["config"] = populateConfig(h.config)
data["fips_status"] = populateFIPSStatus(h.config)
return data
}

Expand Down Expand Up @@ -115,3 +115,11 @@ func populateConfig(config config.Component) map[string]string {

return conf
}

func populateFIPSStatus(config config.Component) string {
fipsStatus := fips.Status()
if fipsStatus == "not available" && config.GetString("fips.enabled") == "true" {
return "proxy"
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ question: Does the Agent use necessarily FIPS proxy if fips.enabled: true? Can it be bypass by anything else ?

}
return fipsStatus
}
4 changes: 4 additions & 0 deletions comp/core/status/statusimpl/common_header_provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ func TestCommonHeaderProviderText(t *testing.T) {
Python Version: n/a
Build arch: %s
Agent flavor: %s
FIPS Mode: not available
Log Level: info

Paths
Expand Down Expand Up @@ -188,6 +189,7 @@ func TestCommonHeaderProviderTextWithFipsInformation(t *testing.T) {
Python Version: n/a
Build arch: %s
Agent flavor: %s
FIPS Mode: proxy
Log Level: info

Paths
Expand Down Expand Up @@ -241,6 +243,7 @@ func TestCommonHeaderProviderHTML(t *testing.T) {
Flavor: %s<br>
PID: %d<br>
Agent start: 2018-01-05 11:25:15 UTC (1515151515000)<br>
FIPS Mode: not available<br>
Log Level: info<br>
Config File: There is no config file<br>
Conf.d Path: %s<br>
Expand Down Expand Up @@ -304,6 +307,7 @@ func TestCommonHeaderProviderHTMLWithFipsInformation(t *testing.T) {
Flavor: %s<br>
PID: %d<br>
Agent start: 2018-01-05 11:25:15 UTC (1515151515000)<br>
FIPS Mode: proxy<br>
Log Level: info<br>
Config File: There is no config file<br>
Conf.d Path: %s<br>
Expand Down
7 changes: 7 additions & 0 deletions comp/core/status/statusimpl/status_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ func TestGetStatus(t *testing.T) {
Python Version: n/a
Build arch: %s
Agent flavor: %s
FIPS Mode: not available
Log Level: info

Paths
Expand Down Expand Up @@ -324,6 +325,7 @@ X Section
Python Version: n/a
Build arch: %s
Agent flavor: %s
FIPS Mode: not available
Log Level: info

Paths
Expand Down Expand Up @@ -374,6 +376,7 @@ X Section
Flavor: %s<br>
PID: %d<br>
Agent start: 2018-01-05 11:25:15 UTC (1515151515000)<br>
FIPS Mode: not available<br>
Log Level: info<br>
Config File: There is no config file<br>
Conf.d Path: %s<br>
Expand Down Expand Up @@ -428,6 +431,7 @@ X Section
Flavor: %s<br>
PID: %d<br>
Agent start: 2018-01-05 11:25:15 UTC (1515151515000)<br>
FIPS Mode: not available<br>
Log Level: info<br>
Config File: There is no config file<br>
Conf.d Path: %s<br>
Expand Down Expand Up @@ -515,6 +519,7 @@ func TestGetStatusDoNotRenderHeaderIfNoProviders(t *testing.T) {
Python Version: n/a
Build arch: %s
Agent flavor: %s
FIPS Mode: not available
Log Level: info

Paths
Expand Down Expand Up @@ -602,6 +607,7 @@ func TestGetStatusWithErrors(t *testing.T) {
Python Version: n/a
Build arch: %s
Agent flavor: agent
FIPS Mode: not available
Log Level: info

Paths
Expand Down Expand Up @@ -915,6 +921,7 @@ Status render errors
Python Version: n/a
Build arch: %s
Agent flavor: agent
FIPS Mode: not available
Log Level: info

Paths
Expand Down
6 changes: 3 additions & 3 deletions comp/core/status/statusimpl/templates/html.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
<span class="stat_data">
Version: {{.version}}<br>
Flavor: {{.flavor}}<br>
{{- if .fips_status}}
FIPS compliant: {{.fips_status}}<br>
{{- end }}
PID: {{.pid}}<br>
Agent start: {{ formatUnixTime .agent_start_nano }}<br>
{{- if .fips_status}}
FIPS Mode: {{.fips_status}}<br>
{{- end }}
{{- if .config.log_file}}
Log File: {{.config.log_file}}<br>
{{end}}
Expand Down
2 changes: 1 addition & 1 deletion comp/core/status/statusimpl/templates/text.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
Build arch: {{.build_arch}}
Agent flavor: {{.flavor}}
{{- if .fips_status}}
FIPS compliant: {{.fips_status}}
FIPS Mode: {{.fips_status}}
{{- end }}
{{- if .config.log_file}}
Log File: {{.config.log_file}}
Expand Down
2 changes: 1 addition & 1 deletion pkg/fips/fips_disabled.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ package fips

// Status returns an empty string when not the datadog-fips-agent flavor
func Status() string {
return ""
return "not available"
}

// Enabled returns false when not the datadog-fips-agent flavor
Expand Down
9 changes: 6 additions & 3 deletions pkg/fips/fips_nix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@ package fips

import (
"os"
"strconv"
)

// Status returns a displayable string or error of FIPS compliance state of the agent build and runtime
// Status returns a displayable string or error of FIPS Mode of the agent build and runtime
func Status() string {
enabled, _ := Enabled()
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ question: ‏Why do Enabled function returned error if we don't checks it ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The windows "FIPS Mode" has more error modes due to the OS registry look up but they did not want to log it or display that to the user at this point. It is _ in case we want to log it in the future or other users of the fips.Enabled() interface need to surface it.

return strconv.FormatBool(enabled)
if enabled {
return "enabled"
} else {
return "disabled"
}
}

// Enabled checks to see if the agent runtime environment is as expected relating to its build to be FIPS compliant. For Linux this is that the binary is run with the GOFIPS=1 environment variable.
Expand Down
9 changes: 6 additions & 3 deletions pkg/fips/fips_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,18 @@ package fips

import (
"fmt"
"strconv"

"golang.org/x/sys/windows/registry"
)

// Status returns a displayable string or error of FIPS compliance state of the agent build and runtime
// Status returns a displayable string or error of FIPS Mode of the agent build and runtime
func Status() string {
enabled, _ := Enabled()
return strconv.FormatBool(enabled)
if enabled {
return "enabled"
} else {
return "disabled"
}
}

// Enabled checks to see if the agent runtime environment is as expected relating to its build to be FIPS compliant. For Windows this means that FIPS mode is enabled via the Windows registry.
Expand Down
1 change: 1 addition & 0 deletions test/new-e2e/tests/agent-subcommands/status/status_common_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ func (v *baseStatusSuite) TestDefaultInstallStatus() {
{
name: `Agent \(.*\)`, // TODO: verify that the right version is output
shouldBePresent: true,
shouldContain: []string{"FIPS Mode: not available"},
shouldNotContain: []string{"FIPS proxy"},
},
{
Expand Down
2 changes: 1 addition & 1 deletion test/new-e2e/tests/agent-subcommands/status/status_nix_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (v *linuxStatusSuite) TestFIPSProxyStatus() {
{
name: `Agent \(.*\)`,
shouldBePresent: true,
shouldContain: []string{"FIPS proxy"},
shouldContain: []string{"FIPS Mode: proxy", "FIPS proxy"},
},
}

Expand Down
5 changes: 2 additions & 3 deletions test/new-e2e/tests/fips-compliance/fips_nix_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,14 @@ func TestLinuxFIPSComplianceSuite(t *testing.T) {
awshost.WithEC2InstanceOptions(ec2.WithOS(os.UbuntuDefault)),
awshost.WithAgentOptions(agentparams.WithFlavor("datadog-fips-agent")),
)),
e2e.WithSkipDeleteOnFailure(),
)
}

func (v *LinuxFIPSComplianceSuite) TestFIPSDefaultConfig() {
status := v.Env().RemoteHost.MustExecute("sudo GOFIPS=0 datadog-agent status")
assert.NotContains(v.T(), status, "can't enable FIPS mode for OpenSSL")
assert.Contains(v.T(), status, "Status date")
assert.Contains(v.T(), status, "FIPS compliant: false")
assert.Contains(v.T(), status, "FIPS Mode: disabled")

v.Env().RemoteHost.MustExecute("sudo systemctl set-environment GOFIPS=1")
v.Env().RemoteHost.MustExecute("sudo systemctl restart datadog-agent")
Expand All @@ -52,7 +51,7 @@ func (v *LinuxFIPSComplianceSuite) TestFIPSDefaultConfig() {
status = v.Env().RemoteHost.MustExecute("sudo GOFIPS=1 datadog-agent status")
assert.NotContains(t, status, "can't enable FIPS mode for OpenSSL")
assert.Contains(t, status, "Status date")
assert.Contains(t, status, "FIPS compliant: true")
assert.Contains(t, status, "FIPS Mode: enabled")
}, 60*time.Second, 5*time.Second)

v.Env().RemoteHost.MustExecute("sudo systemctl unset-environment GOFIPS")
Expand Down
Loading