Skip to content

Commit

Permalink
[tls] add missing built in cipher stat names (#14956)
Browse files Browse the repository at this point in the history
Backport of #14676

Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Piotr Sikora <piotrsikora@google.com>

Co-authored-by: asraa <asraa@google.com>
  • Loading branch information
PiotrSikora and asraa authored Feb 5, 2021
1 parent 2ad79ce commit cf6f2b5
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 10 deletions.
5 changes: 3 additions & 2 deletions source/extensions/transport_sockets/tls/context_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -471,9 +471,10 @@ ContextImpl::ContextImpl(Stats::Scope& scope, const Envoy::Ssl::ContextConfig& c
}

// Add hardcoded cipher suites from the TLS 1.3 spec:
// https://tools.ietf.org/html/rfc8446
// https://tools.ietf.org/html/rfc8446#appendix-B.4
// AES-CCM cipher suites are removed (no BoringSSL support).
stat_name_set_->rememberBuiltins(
{"TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384", "TLS_AES_128_GCM_SHA256"});
{"TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384", "TLS_CHACHA20_POLY1305_SHA256"});

// Curves from
// https://github.com/google/boringssl/blob/f4d8b969200f1ee2dd872ffb85802e6a0976afe7/ssl/ssl_key_share.cc#L384
Expand Down
21 changes: 13 additions & 8 deletions test/extensions/transport_sockets/tls/context_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1977,11 +1977,16 @@ class SslContextStatsTest : public SslContextImplTest {
TEST_F(SslContextStatsTest, IncOnlyKnownCounters) {
// Incrementing a value for a cipher that is part of the configuration works, and
// we'll be able to find the value in the stats store.
context_->incCounter("ssl.ciphers", "ECDHE-ECDSA-AES256-GCM-SHA384");
Stats::CounterOptConstRef cipher =
store_.findCounterByString("ssl.ciphers.ECDHE-ECDSA-AES256-GCM-SHA384");
ASSERT_TRUE(cipher.has_value());
EXPECT_EQ(1, cipher->get().value());
for (const auto& cipher :
{"TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384", "TLS_CHACHA20_POLY1305_SHA256"}) {
// Test supported built-in TLS v1.3 cipher suites
// https://tools.ietf.org/html/rfc8446#appendix-B.4.
context_->incCounter("ssl.ciphers", cipher);
Stats::CounterOptConstRef stat =
store_.findCounterByString(absl::StrCat("ssl.ciphers.", cipher));
ASSERT_TRUE(stat.has_value());
EXPECT_EQ(1, stat->get().value());
}

// Incrementing a stat for a random unknown cipher does not work. A
// rate-limited error log message will also be generated but that is hard to
Expand All @@ -1995,9 +2000,9 @@ TEST_F(SslContextStatsTest, IncOnlyKnownCounters) {
// fallback registration does not occur. So we test for the fallback only in
// release builds.
#ifdef NDEBUG
cipher = store_.findCounterByString("ssl.ciphers.fallback");
ASSERT_TRUE(cipher.has_value());
EXPECT_EQ(1, cipher->get().value());
Stats::CounterOptConstRef stat = store_.findCounterByString("ssl.ciphers.fallback");
ASSERT_TRUE(stat.has_value());
EXPECT_EQ(1, stat->get().value());
#endif
}

Expand Down
1 change: 1 addition & 0 deletions tools/spelling/spelling_dictionary.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ STATNAME
SkyWalking
TIDs
ceil
CCM
CHACHA
CHLO
CHMOD
Expand Down

0 comments on commit cf6f2b5

Please sign in to comment.