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

Documentation error: private_key and private_key_provider can(should) be configured simultaneously #37285

Open
MaYuan-02 opened this issue Nov 21, 2024 · 1 comment

Comments

@MaYuan-02
Copy link

Title: Documentation error, private_key and private_key_provider can be configured simultaneously

Description:
current doc

Setting both private_key and private_key_provider fields will result in an error.

private_key_provider support fallback. When current private provider is not available, Envoy will fallback to default implementation and private_key field will be used in this condition.
And I have already tested this scenario, and it is indeed the case.
When private_key and private_key_provider are configured simultaneously, envoy will try to use private_key_provider. If private_key_provider is not available and fallback is true, private key will be used.

related code:

 if (config.has_private_key_provider()) {
      private_key_method_ =
          factory_context.sslContextManager()
              .privateKeyMethodManager()
              .createPrivateKeyMethodProvider(config.private_key_provider(), factory_context);
      if (private_key_method_ == nullptr ||
          (!private_key_method_->isAvailable() && !config.private_key_provider().fallback())) {
        creation_status =
            absl::InvalidArgumentError(fmt::format("Failed to load private key provider: {}",
                                                   config.private_key_provider().provider_name()));
        return;
      }

      if (!private_key_method_->isAvailable()) {
        private_key_method_ = nullptr;
      }
    }
    if (certificate_chain_.empty()) {
      creation_status = absl::InvalidArgumentError(
          fmt::format("Failed to load incomplete certificate from {}: certificate chain not set",
                      certificate_chain_path_));
    }

    if (private_key_.empty() && private_key_method_ == nullptr) {
      creation_status = absl::InvalidArgumentError(
          fmt::format("Failed to load incomplete private key from path: {}", private_key_path_));
    }
Envoy::Ssl::PrivateKeyMethodProviderSharedPtr private_key_method_provider =
          tls_certificate.privateKeyMethod();
      // We either have a private key or a BoringSSL private key method provider.
      if (private_key_method_provider) {
        ctx.private_key_method_provider_ = private_key_method_provider;
        // The provider has a reference to the private key method for the context lifetime.
        Ssl::BoringSslPrivateKeyMethodSharedPtr private_key_method =
            private_key_method_provider->getBoringSslPrivateKeyMethod();
        if (private_key_method == nullptr) {
          creation_status = absl::InvalidArgumentError(
              fmt::format("Failed to get BoringSSL private key method from provider"));
          return;
        }
#ifdef BORINGSSL_FIPS
        if (!ctx.private_key_method_provider_->checkFips()) {
          creation_status = absl::InvalidArgumentError(
              fmt::format("Private key method doesn't support FIPS mode with current parameters"));
          return;
        }
#endif
        SSL_CTX_set_private_key_method(ctx.ssl_ctx_.get(), private_key_method.get());
      } else if (!tls_certificate.privateKey().empty()) {
        // Load private key.
        creation_status =
            ctx.loadPrivateKey(tls_certificate.privateKey(), tls_certificate.privateKeyPath(),
                               tls_certificate.password());
        if (!creation_status.ok()) {
          return;
        }
      }
@MaYuan-02 MaYuan-02 added the triage Issue requires triage label Nov 21, 2024
@soulxu soulxu added area/docs and removed triage Issue requires triage labels Nov 21, 2024
@soulxu
Copy link
Member

soulxu commented Nov 21, 2024

yea, you are right, the document is forget to update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants