Remove more panic corrupt enum (envoyproxy#20089)

Commit Message: Removing several instances of PANIC_DUE_TO_CORRUPT_ENUM as part of envoyproxy#19229

Signed-off-by: tommyp1ckles <[email protected]>

source/common/common/dns_utils.cc

@@ -27,7 +27,8 @@ getDnsLookupFamilyFromEnum(envoy::config::cluster::v3::Cluster::DnsLookupFamily
   case envoy::config::cluster::v3::Cluster::ALL:
     return Network::DnsLookupFamily::All;
   }
-  PANIC_DUE_TO_CORRUPT_ENUM;
+  IS_ENVOY_BUG("unexpected dns lookup family enum");
+  return Network::DnsLookupFamily::All;
 }
 
 std::vector<Network::Address::InstanceConstSharedPtr>

source/common/upstream/upstream_impl.cc

@@ -28,6 +28,7 @@
 #include "envoy/upstream/health_checker.h"
 #include "envoy/upstream/upstream.h"
 
+#include "source/common/common/dns_utils.h"
 #include "source/common/common/enum_to_int.h"
 #include "source/common/common/fmt.h"
 #include "source/common/common/utility.h"
@@ -1841,25 +1842,7 @@ bool BaseDynamicClusterImpl::updateDynamicHostList(
 
 Network::DnsLookupFamily
 getDnsLookupFamilyFromCluster(const envoy::config::cluster::v3::Cluster& cluster) {
-  return getDnsLookupFamilyFromEnum(cluster.dns_lookup_family());
-}
-
-Network::DnsLookupFamily
-getDnsLookupFamilyFromEnum(envoy::config::cluster::v3::Cluster::DnsLookupFamily family) {
-  switch (family) {
-    PANIC_ON_PROTO_ENUM_SENTINEL_VALUES;
-  case envoy::config::cluster::v3::Cluster::V6_ONLY:
-    return Network::DnsLookupFamily::V6Only;
-  case envoy::config::cluster::v3::Cluster::V4_ONLY:
-    return Network::DnsLookupFamily::V4Only;
-  case envoy::config::cluster::v3::Cluster::AUTO:
-    return Network::DnsLookupFamily::Auto;
-  case envoy::config::cluster::v3::Cluster::V4_PREFERRED:
-    return Network::DnsLookupFamily::V4Preferred;
-  case envoy::config::cluster::v3::Cluster::ALL:
-    return Network::DnsLookupFamily::All;
-  }
-  PANIC_DUE_TO_CORRUPT_ENUM;
+  return DnsUtils::getDnsLookupFamilyFromEnum(cluster.dns_lookup_family());
 }
 
 void reportUpstreamCxDestroy(const Upstream::HostDescriptionConstSharedPtr& host,

source/common/upstream/upstream_impl.h

@@ -1055,8 +1055,6 @@ class BaseDynamicClusterImpl : public ClusterImplBase {
  */
 Network::DnsLookupFamily
 getDnsLookupFamilyFromCluster(const envoy::config::cluster::v3::Cluster& cluster);
-Network::DnsLookupFamily
-getDnsLookupFamilyFromEnum(envoy::config::cluster::v3::Cluster::DnsLookupFamily family);
 
 /**
  * Utility function to report upstream cx destroy metrics

source/extensions/filters/listener/tls_inspector/tls_inspector.cc

@@ -121,7 +121,9 @@ Network::FilterStatus Filter::onAccept(Network::ListenerFilterCallbacks& cb) {
         Event::PlatformDefaultTriggerType, Event::FileReadyType::Read);
     return Network::FilterStatus::StopIteration;
   }
-  PANIC_DUE_TO_CORRUPT_ENUM;
+
+  IS_ENVOY_BUG("unexpected tcp filter parse_state");
+  return Network::FilterStatus::StopIteration;
 }
 
 void Filter::onALPN(const unsigned char* data, unsigned int len) {

source/extensions/network/dns_resolver/apple/apple_dns_impl.cc

@@ -219,7 +219,8 @@ std::list<DnsResponse>& AppleDnsResolverImpl::PendingResolution::finalAddressLis
                                             pending_response_.v6_responses_.end());
     return pending_response_.all_responses_;
   }
-  PANIC_DUE_TO_CORRUPT_ENUM;
+  IS_ENVOY_BUG("unexpected DnsLookupFamily enum");
+  return pending_response_.all_responses_;
 }
 
 void AppleDnsResolverImpl::PendingResolution::finishResolve() {
@@ -379,7 +380,9 @@ AppleDnsResolverImpl::PendingResolution::buildDnsResponse(const struct sockaddr*
     address_in6.sin6_addr = reinterpret_cast<const sockaddr_in6*>(address)->sin6_addr;
     return {std::make_shared<const Address::Ipv6Instance>(address_in6), std::chrono::seconds(ttl)};
   }
-  PANIC_DUE_TO_CORRUPT_ENUM;
+  IS_ENVOY_BUG("unexpected DnsLookupFamily enum");
+  sockaddr_in address_in;
+  return {std::make_shared<const Address::Ipv4Instance>(&address_in), std::chrono::seconds(ttl)};
 }
 
 // apple DNS resolver factory

source/extensions/stat_sinks/graphite_statsd/config.cc

@@ -38,9 +38,9 @@ GraphiteStatsdSinkFactory::createStatsSink(const Protobuf::Message& config,
   }
   case envoy::extensions::stat_sinks::graphite_statsd::v3::GraphiteStatsdSink::StatsdSpecifierCase::
       STATSD_SPECIFIER_NOT_SET:
-    break; // Fall through to PANIC.
+    break;
   }
-  PANIC_DUE_TO_CORRUPT_ENUM;
+  throw EnvoyException("unexpected statsd specifier enum");
 }
 
 ProtobufTypes::MessagePtr GraphiteStatsdSinkFactory::createEmptyConfigProto() {

source/extensions/stat_sinks/statsd/config.cc

@@ -37,7 +37,7 @@ StatsdSinkFactory::createStatsSink(const Protobuf::Message& config,
   case envoy::config::metrics::v3::StatsdSink::StatsdSpecifierCase::STATSD_SPECIFIER_NOT_SET:
     break; // Fall through to PANIC
   }
-  PANIC_DUE_TO_CORRUPT_ENUM;
+  throw EnvoyException("unexpected statsd specifier case num");
 }
 
 ProtobufTypes::MessagePtr StatsdSinkFactory::createEmptyConfigProto() {

source/extensions/transport_sockets/tls/cert_validator/default_validator.cc

@@ -150,7 +150,12 @@ int DefaultCertValidator::initializeSslContexts(std::vector<SSL_CTX*> contexts,
     if (!cert_validation_config->subjectAltNameMatchers().empty()) {
       for (const envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher& matcher :
            cert_validation_config->subjectAltNameMatchers()) {
-        subject_alt_name_matchers_.emplace_back(createStringSanMatcher(matcher));
+        auto san_matcher = createStringSanMatcher(matcher);
+        if (san_matcher == nullptr) {
+          throw EnvoyException(
+              absl::StrCat("Failed to create string SAN matcher of type ", matcher.san_type()));
+        }
+        subject_alt_name_matchers_.push_back(std::move(san_matcher));
       }
       verify_mode = verify_mode_validation_context;
     }

source/extensions/transport_sockets/tls/cert_validator/san_matcher.cc

@@ -46,7 +46,7 @@ SanMatcherPtr createStringSanMatcher(
   case envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::SAN_TYPE_UNSPECIFIED:
     PANIC("unhandled value");
   }
-  PANIC_DUE_TO_CORRUPT_ENUM;
+  return nullptr;
 }
 
 } // namespace Tls

source/extensions/transport_sockets/tls/context_config_impl.cc

@@ -310,7 +310,8 @@ unsigned ContextConfigImpl::tlsVersionFromProto(
   case envoy::extensions::transport_sockets::tls::v3::TlsParameters::TLSv1_3:
     return TLS1_3_VERSION;
   }
-  PANIC_DUE_TO_CORRUPT_ENUM;
+  IS_ENVOY_BUG("unexpected tls version provided");
+  return default_version;
 }
 
 const unsigned ClientContextConfigImpl::DEFAULT_MIN_VERSION = TLS1_2_VERSION;

source/server/admin/utils.cc

@@ -18,7 +18,8 @@ envoy::admin::v3::ServerInfo::State serverState(Init::Manager::State state,
     return health_check_failed ? envoy::admin::v3::ServerInfo::DRAINING
                                : envoy::admin::v3::ServerInfo::LIVE;
   }
-  PANIC_DUE_TO_CORRUPT_ENUM;
+  IS_ENVOY_BUG("unexpected server state enum");
+  return envoy::admin::v3::ServerInfo::PRE_INITIALIZING;
 }
 
 void populateFallbackResponseHeaders(Http::Code code, Http::ResponseHeaderMap& header_map) {

source/server/overload_manager_impl.cc

@@ -257,7 +257,7 @@ OverloadAction::OverloadAction(const envoy::config::overload::v3::OverloadAction
       trigger = std::make_unique<ScaledTriggerImpl>(trigger_config.scaled());
       break;
     case envoy::config::overload::v3::Trigger::TriggerOneofCase::TRIGGER_ONEOF_NOT_SET:
-      PANIC_DUE_TO_CORRUPT_ENUM;
+      throw EnvoyException(absl::StrCat("action not set for trigger ", trigger_config.name()));
     }
 
     if (!triggers_.try_emplace(trigger_config.name(), std::move(trigger)).second) {

test/extensions/transport_sockets/tls/cert_validator/default_validator_test.cc

@@ -187,6 +187,56 @@ TEST(DefaultCertValidatorTest, NoSanInCert) {
   EXPECT_FALSE(DefaultCertValidator::matchSubjectAltName(cert.get(), subject_alt_name_matchers));
 }
 
+class MockCertificateValidationContextConfig : public Ssl::CertificateValidationContextConfig {
+public:
+  MockCertificateValidationContextConfig() {
+    auto matcher = envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher();
+    matcher.set_san_type(
+        static_cast<envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher_SanType>(
+            123));
+    matchers_.emplace_back(matcher);
+  };
+  const std::string& caCert() const override { return s_; }
+  const std::string& caCertPath() const override { return s_; }
+  const std::string& certificateRevocationList() const override { return s_; }
+  const std::string& certificateRevocationListPath() const override { return s_; }
+  const std::vector<envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher>&
+  subjectAltNameMatchers() const override {
+    return matchers_;
+  }
+  const std::vector<std::string>& verifyCertificateHashList() const override { return strs_; }
+  const std::vector<std::string>& verifyCertificateSpkiList() const override { return strs_; }
+  bool allowExpiredCertificate() const override { return false; }
+  MOCK_METHOD(envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext::
+                  TrustChainVerification,
+              trustChainVerification, (), (const override));
+  MOCK_METHOD(const absl::optional<envoy::config::core::v3::TypedExtensionConfig>&,
+              customValidatorConfig, (), (const override));
+  MOCK_METHOD(Api::Api&, api, (), (const override));
+  bool onlyVerifyLeafCertificateCrl() const override { return false; }
+
+private:
+  std::string s_;
+  std::vector<std::string> strs_;
+  std::vector<envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher> matchers_;
+};
+
+TEST(DefaultCertValidatorTest, TestUnexpectedSanMatcherType) {
+  auto mock_context_config = std::make_unique<MockCertificateValidationContextConfig>();
+  EXPECT_CALL(*mock_context_config.get(), trustChainVerification())
+      .WillRepeatedly(testing::Return(envoy::extensions::transport_sockets::tls::v3::
+                                          CertificateValidationContext::ACCEPT_UNTRUSTED));
+  auto matchers =
+      std::vector<envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher>();
+  Stats::TestUtil::TestStore store;
+  auto ssl_stats = generateSslStats(store);
+  auto validator = std::make_unique<DefaultCertValidator>(mock_context_config.get(), ssl_stats,
+                                                          Event::GlobalTimeSystem().timeSystem());
+  auto ctx = std::vector<SSL_CTX*>();
+  EXPECT_THROW_WITH_REGEX(validator->initializeSslContexts(ctx, false), EnvoyException,
+                          "Failed to create string SAN matcher of type.*");
+}
+
 } // namespace Tls
 } // namespace TransportSockets
 } // namespace Extensions

test/extensions/transport_sockets/tls/cert_validator/san_matcher_test.cc

@@ -49,6 +49,12 @@ TEST(SanMatcherConfigTest, UnspecifiedSanType) {
       envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::SAN_TYPE_UNSPECIFIED);
   EXPECT_THROW_WITH_REGEX(TestUtility::validate(san_matcher), EnvoyException,
                           "Proto constraint validation failed");
+
+  auto san_type =
+      static_cast<envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher::SanType>(
+          static_cast<int>(123));
+  san_matcher.set_san_type(san_type);
+  EXPECT_EQ(createStringSanMatcher(san_matcher), nullptr);
 }
 
 } // namespace Tls

test/server/admin/BUILD

@@ -82,6 +82,14 @@ envoy_cc_benchmark_binary(
     ],
 )
 
+envoy_cc_test(
+    name = "utils_test",
+    srcs = ["utils_test.cc"],
+    deps = [
+        "//source/server/admin:utils_lib",
+    ],
+)
+
 envoy_cc_test(
     name = "runtime_handler_test",
     srcs = ["runtime_handler_test.cc"],

test/server/admin/utils_test.cc

@@ -0,0 +1,22 @@
+#include "source/server/admin/utils.h"
+
+#include "test/test_common/utility.h"
+
+namespace Envoy {
+namespace Server {
+
+class UtilsTest : public testing::Test {
+public:
+  UtilsTest() = default;
+};
+
+// Most utils paths are covered through other tests, these tests take of
+// of special cases to get remaining coverage.
+TEST(UtilsTest, BadServerState) {
+  Utility::serverState(Init::Manager::State::Uninitialized, true);
+  EXPECT_ENVOY_BUG(Utility::serverState(static_cast<Init::Manager::State>(123), true),
+                   "unexpected server state");
+}
+
+} // namespace Server
+} // namespace Envoy

test/server/overload_manager_impl_test.cc

@@ -799,6 +799,17 @@ TEST_F(OverloadManagerImplTest, Shutdown) {
   manager->stop();
 }
 
+TEST_F(OverloadManagerImplTest, MissingConfigTriggerType) {
+  constexpr char missingTriggerTypeConfig[] = R"YAML(
+  actions:
+    - name: envoy.overload_actions.dummy_action
+      triggers:
+        - name: envoy.resource_monitors.fake_resource1
+)YAML";
+  EXPECT_THROW_WITH_REGEX(createOverloadManager(missingTriggerTypeConfig), EnvoyException,
+                          "action not set for trigger.*");
+}
+
 TEST_F(OverloadManagerImplTest, ProactiveResourceAllocateAndDeallocateResourceTest) {
   setDispatcherExpectation();
   auto manager(createOverloadManager(proactiveResourceConfig));