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

server: add FIPS mode statistic indicating FIPS compliance #14719

Merged
merged 11 commits into from
Jan 21, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions docs/root/configuration/observability/statistics.rst
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,5 @@ Server related statistics are rooted at *server.* with following statistics:
envoy_bug_failures, Counter, Number of envoy bug failures detected in a release build. File or report the issue if this increments as this may be serious.
static_unknown_fields, Counter, Number of messages in static configuration with unknown fields
dynamic_unknown_fields, Counter, Number of messages in dynamic configuration with unknown fields
fips_mode, Gauge, Integer representing whether the envoy build is FIPS compliant or not

1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ New Features
* access log: added the :ref:`formatters <envoy_v3_api_field_config.core.v3.SubstitutionFormatString.formatters>` extension point for custom formatters (command operators).
* http: added support for :ref:`:ref:`preconnecting <envoy_v3_api_msg_config.cluster.v3.Cluster.PreconnectPolicy>`. Preconnecting is off by default, but recommended for clusters serving latency-sensitive traffic, especially if using HTTP/1.1.
* http: change frame flood and abuse checks to the upstream HTTP/2 codec to ON by default. It can be disabled by setting the `envoy.reloadable_features.upstream_http2_flood_checks` runtime key to false.
* server: added :ref:`fips_mode <statistics>` statistic.
* tcp_proxy: add support for converting raw TCP streams into HTTP/1.1 CONNECT requests. See :ref:`upgrade documentation <tunneling-tcp-over-http>` for details.

Deprecated
Expand Down
2 changes: 1 addition & 1 deletion source/common/version/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ class VersionInfo {
static const std::string& revisionStatus();
// Repository information and build type.
static const std::string& version();
static const std::string& sslVersion();

static const envoy::config::core::v3::BuildVersion& buildVersion();

private:
friend class Envoy::VersionInfoTestPeer;
// RELEASE or DEBUG
static const std::string& buildType();
static const std::string& sslVersion();
static envoy::config::core::v3::BuildVersion makeBuildVersion(const char* version);
};

Expand Down
4 changes: 4 additions & 0 deletions source/server/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,10 @@ void InstanceImpl::initialize(const Options& options,
}
}
server_stats_->version_.set(version_int);
const std::string fips_ssl_version = "BoringSSL-FIPS";
if (VersionInfo::sslVersion() == fips_ssl_version) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of string-matching, please add a new function to VersionInfo for sslFipsCompliant that returns bool, and is set via #ifdef BORINGSSL_FIPS.

Copy link
Member

Choose a reason for hiding this comment

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

+1, this will leave the openssl fork to be able to indicate the FIPS compliance. cc @envoyproxy/openssl-dev

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a new function as suggested.

server_stats_->fips_mode_.set(1);
}

bootstrap_.mutable_node()->set_hidden_envoy_deprecated_build_version(VersionInfo::version());
bootstrap_.mutable_node()->set_user_agent_name("envoy");
Expand Down
1 change: 1 addition & 0 deletions source/server/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ namespace Server {
GAUGE(total_connections, Accumulate) \
GAUGE(uptime, Accumulate) \
GAUGE(version, NeverImport) \
GAUGE(fips_mode, NeverImport) \
Copy link
Contributor

Choose a reason for hiding this comment

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

In order to add similar stats for other compile-time settings in the future, please move this into a nested namespace, so the full name would be server.compilation_settings.fips_mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved it to a nested namespace

HISTOGRAM(initialization_time_ms, Milliseconds)

struct ServerStats {
Expand Down
16 changes: 16 additions & 0 deletions test/server/server_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,22 @@ TEST_P(ServerInstanceImplTest, ProxyVersionOveridesFromBootstrap) {
server_thread->join();
}

// Validates that the "server.fips_mode" stat indicates the FIPS compliance from the Envoy Build
TEST_P(ServerInstanceImplTest, ValidateFIPSModeStat) {
auto server_thread =
startTestServer("test/server/test_data/server/proxy_version_bootstrap.yaml", true);

const std::string fips_ssl_version = "BoringSSL-FIPS";
if (VersionInfo::sslVersion() == fips_ssl_version) {
EXPECT_EQ(1L, TestUtility::findGauge(stats_store_, "server.fips_mode")->value());
} else {
EXPECT_EQ(0L, TestUtility::findGauge(stats_store_, "server.fips_mode")->value());
}

server_->dispatcher().post([&] { server_->shutdown(); });
server_thread->join();
}

TEST_P(ServerInstanceImplTest, EmptyShutdownLifecycleNotifications) {
auto server_thread = startTestServer("test/server/test_data/server/node_bootstrap.yaml", false);
server_->dispatcher().post([&] { server_->shutdown(); });
Expand Down