Repository: envoyproxy/envoy
File: source/common/tls/context_config_impl.cc (C++)

Code before:
const std::string ClientContextConfigImpl::DEFAULT_CURVES = "X25519:"
                                                            "P-256";

const std::string ClientContextConfigImpl::DEFAULT_CURVES_FIPS = "P-256";

absl::StatusOr<std::unique_ptr<ClientContextConfigImpl>> ClientContextConfigImpl::create(
    const envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext& config,
    Server::Configuration::TransportSocketFactoryContext& secret_provider_context) {
  absl::Status creation_status = absl::OkStatus();
  std::unique_ptr<ClientContextConfigImpl> ret = absl::WrapUnique(
      new ClientContextConfigImpl(config, secret_provider_context, creation_status));
  RETURN_IF_NOT_OK(creation_status);
  return ret;
}

ClientContextConfigImpl::ClientContextConfigImpl(
    const envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext& config,
    Server::Configuration::TransportSocketFactoryContext& factory_context,
    absl::Status& creation_status)
    : ContextConfigImpl(
          config.common_tls_context(), config.auto_sni_san_validation(), DEFAULT_MIN_VERSION,
          DEFAULT_MAX_VERSION, FIPS_mode() ? DEFAULT_CIPHER_SUITES_FIPS : DEFAULT_CIPHER_SUITES,
          FIPS_mode() ? DEFAULT_CURVES_FIPS : DEFAULT_CURVES, factory_context, creation_status),
      server_name_indication_(config.sni()), auto_host_sni_(config.auto_host_sni()),
      allow_renegotiation_(config.allow_renegotiation()),
      enforce_rsa_key_usage_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, enforce_rsa_key_usage, true)),
      max_session_keys_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, max_session_keys, 1)) {
  // BoringSSL treats this as a C string, so embedded NULL characters will not
  // be handled correctly.
  if (server_name_indication_.find('\0') != std::string::npos) {
    creation_status = absl::InvalidArgumentError("SNI names containing NULL-byte are not allowed");
    return;
  }

  // TODO(PiotrSikora): Support multiple TLS certificates.
  if ((config.common_tls_context().tls_certificates().size() +
       config.common_tls_context().tls_certificate_sds_secret_configs().size()) > 1 &&
      !config.common_tls_context().has_custom_tls_certificate_selector()) {
    creation_status = absl::InvalidArgumentError(
        "Multiple TLS certificates are not supported for client contexts");
    return;
  }

  if (config.common_tls_context().has_custom_tls_certificate_selector()) {
    const auto& provider_config = config.common_tls_context().custom_tls_certificate_selector();
    Ssl::UpstreamTlsCertificateSelectorConfigFactory& provider_factory =
        Config::Utility::getAndCheckFactory<Ssl::UpstreamTlsCertificateSelectorConfigFactory>(
            provider_config);
    ProtobufTypes::MessagePtr message = Config::Utility::translateAnyToFactoryConfig(
        provider_config.typed_config(), factory_context.messageValidationVisitor(),
        provider_factory);

Review comment (suggestion):
In this function, I think we need:

```
if (!config.has_enforce_rsa_key_usage()) { 
  ENVOY_LOG(warn, "you didn't set this, the value changed, and the option is going away");
} else if (! enforce_rsa_key_usage_) {
  ENVOY_LOG(warn, "you set it to false, and it's going away next version and will be forced to true");
  factory_context.serverFactoryContext().runtime().countDeprecatedFeatureUse();
}
```

(But with better error text that that)

Code after:
const std::string ClientContextConfigImpl::DEFAULT_CURVES = "X25519:"
                                                            "P-256";

const std::string ClientContextConfigImpl::DEFAULT_CURVES_FIPS = "P-256";

absl::StatusOr<std::unique_ptr<ClientContextConfigImpl>> ClientContextConfigImpl::create(
    const envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext& config,
    Server::Configuration::TransportSocketFactoryContext& secret_provider_context) {
  absl::Status creation_status = absl::OkStatus();
  std::unique_ptr<ClientContextConfigImpl> ret = absl::WrapUnique(
      new ClientContextConfigImpl(config, secret_provider_context, creation_status));
  RETURN_IF_NOT_OK(creation_status);
  return ret;
}

ClientContextConfigImpl::ClientContextConfigImpl(
    const envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext& config,
    Server::Configuration::TransportSocketFactoryContext& factory_context,
    absl::Status& creation_status)
    : ContextConfigImpl(
          config.common_tls_context(), config.auto_sni_san_validation(), DEFAULT_MIN_VERSION,
          DEFAULT_MAX_VERSION, FIPS_mode() ? DEFAULT_CIPHER_SUITES_FIPS : DEFAULT_CIPHER_SUITES,
          FIPS_mode() ? DEFAULT_CURVES_FIPS : DEFAULT_CURVES, factory_context, creation_status),
      server_name_indication_(config.sni()), auto_host_sni_(config.auto_host_sni()),
      allow_renegotiation_(config.allow_renegotiation()),
      enforce_rsa_key_usage_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, enforce_rsa_key_usage, true)),
      max_session_keys_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, max_session_keys, 1)) {

  if (!enforce_rsa_key_usage_) {
    ENVOY_LOG(
        warn,
        "The 'enforce_rsa_key_usage' option is set to false, which disables the enforcement of RSA "
        "key usage. This option will be removed in the next version. The handshake will fail "
        "if the keyUsage extension is present and incompatible with the "
        "TLS usage. Please update the certificates to be compliant.");
  }

  // BoringSSL treats this as a C string, so embedded NULL characters will not
  // be handled correctly.
  if (server_name_indication_.find('\0') != std::string::npos) {
    creation_status = absl::InvalidArgumentError("SNI names containing NULL-byte are not allowed");
    return;
  }

  // TODO(PiotrSikora): Support multiple TLS certificates.
  if ((config.common_tls_context().tls_certificates().size() +
       config.common_tls_context().tls_certificate_sds_secret_configs().size()) > 1 &&
      !config.common_tls_context().has_custom_tls_certificate_selector()) {
    creation_status = absl::InvalidArgumentError(
        "Multiple TLS certificates are not supported for client contexts");
    return;