Skip to content

Commit

Permalink
[tls] Provide more detailed error messages when validating credential…
Browse files Browse the repository at this point in the history
…s. (grpc#38543)

Users of the TLS certificate provider were given the option to have validate their credentials at startup time in grpc#37565, so that they can fail-fast. This is a small follow-up PR that provides finer-grained error messages in the event of a failure.

Closes grpc#38543

COPYBARA_INTEGRATE_REVIEW=grpc#38543 from matthewstevenson88:finer-grained-error-message bc6d699
PiperOrigin-RevId: 720210228
  • Loading branch information
matthewstevenson88 authored and copybara-github committed Jan 27, 2025
1 parent b4e9cc5 commit 4c7c1fa
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ absl::Status ValidateRootCertificates(absl::string_view root_certificates) {
absl::StatusOr<std::vector<X509*>> parsed_roots =
ParsePemCertificateChain(root_certificates);
if (!parsed_roots.ok()) {
return parsed_roots.status();
return absl::Status(
parsed_roots.status().code(),
absl::StrCat("Failed to parse root certificates as PEM: ",
parsed_roots.status().message()));
}
for (X509* x509 : *parsed_roots) {
X509_free(x509);
Expand All @@ -65,7 +68,10 @@ absl::Status ValidatePemKeyCertPair(absl::string_view cert_chain,
absl::StatusOr<std::vector<X509*>> parsed_certs =
ParsePemCertificateChain(cert_chain);
if (!parsed_certs.ok()) {
return parsed_certs.status();
return absl::Status(
parsed_certs.status().code(),
absl::StrCat("Failed to parse certificate chain as PEM: ",
parsed_certs.status().message()));
}
for (X509* x509 : *parsed_certs) {
X509_free(x509);
Expand All @@ -74,7 +80,9 @@ absl::Status ValidatePemKeyCertPair(absl::string_view cert_chain,
absl::StatusOr<EVP_PKEY*> parsed_private_key =
ParsePemPrivateKey(private_key);
if (!parsed_private_key.ok()) {
return parsed_private_key.status();
return absl::Status(parsed_private_key.status().code(),
absl::StrCat("Failed to parse private key as PEM: ",
parsed_private_key.status().message()));
}
EVP_PKEY_free(*parsed_private_key);
return absl::OkStatus();
Expand Down
18 changes: 12 additions & 6 deletions test/core/security/grpc_tls_certificate_provider_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,8 @@ TEST_F(GrpcTlsCertificateProviderTest,
malformed_cert_,
MakeCertKeyPairs(private_key_.c_str(), cert_chain_.c_str()));
EXPECT_EQ(provider.ValidateCredentials(),
absl::FailedPreconditionError("Invalid PEM."));
absl::FailedPreconditionError(
"Failed to parse root certificates as PEM: Invalid PEM."));
}

TEST_F(GrpcTlsCertificateProviderTest,
Expand All @@ -257,7 +258,8 @@ TEST_F(GrpcTlsCertificateProviderTest,
root_cert_,
MakeCertKeyPairs(private_key_.c_str(), malformed_cert_.c_str()));
EXPECT_EQ(provider.ValidateCredentials(),
absl::FailedPreconditionError("Invalid PEM."));
absl::FailedPreconditionError(
"Failed to parse certificate chain as PEM: Invalid PEM."));
}

TEST_F(GrpcTlsCertificateProviderTest,
Expand All @@ -266,7 +268,8 @@ TEST_F(GrpcTlsCertificateProviderTest,
root_cert_,
MakeCertKeyPairs(malformed_key_.c_str(), cert_chain_.c_str()));
EXPECT_EQ(provider.ValidateCredentials(),
absl::NotFoundError("No private key found."));
absl::NotFoundError(
"Failed to parse private key as PEM: No private key found."));
}

TEST_F(GrpcTlsCertificateProviderTest,
Expand Down Expand Up @@ -309,23 +312,26 @@ TEST_F(GrpcTlsCertificateProviderTest,
FileWatcherCertificateProvider provider(SERVER_KEY_PATH_2, SERVER_CERT_PATH_2,
MALFORMED_CERT_PATH, 1);
EXPECT_EQ(provider.ValidateCredentials(),
absl::FailedPreconditionError("Invalid PEM."));
absl::FailedPreconditionError(
"Failed to parse root certificates as PEM: Invalid PEM."));
}

TEST_F(GrpcTlsCertificateProviderTest,
FileWatcherCertificateProviderWithMalformedIdentityCertificate) {
FileWatcherCertificateProvider provider(
SERVER_KEY_PATH_2, MALFORMED_CERT_PATH, CA_CERT_PATH_2, 1);
EXPECT_EQ(provider.ValidateCredentials(),
absl::FailedPreconditionError("Invalid PEM."));
absl::FailedPreconditionError(
"Failed to parse certificate chain as PEM: Invalid PEM."));
}

TEST_F(GrpcTlsCertificateProviderTest,
FileWatcherCertificateProviderWithMalformedIdentityKey) {
FileWatcherCertificateProvider provider(
MALFORMED_KEY_PATH, SERVER_CERT_PATH_2, CA_CERT_PATH_2, 1);
EXPECT_EQ(provider.ValidateCredentials(),
absl::NotFoundError("No private key found."));
absl::NotFoundError(
"Failed to parse private key as PEM: No private key found."));
}

TEST_F(GrpcTlsCertificateProviderTest,
Expand Down
6 changes: 4 additions & 2 deletions test/cpp/server/credentials_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ TEST(CredentialsTest, StaticDataCertificateProviderWithMalformedRoot) {
key_cert_pair.certificate_chain = GetFileContents(SERVER_CERT_PATH);
StaticDataCertificateProvider provider(root_certificates, {key_cert_pair});
EXPECT_EQ(provider.ValidateCredentials(),
absl::FailedPreconditionError("Invalid PEM."));
absl::FailedPreconditionError(
"Failed to parse root certificates as PEM: Invalid PEM."));
}

TEST(CredentialsTest,
Expand All @@ -148,7 +149,8 @@ TEST(CredentialsTest, FileWatcherCertificateProviderWithMalformedRoot) {
FileWatcherCertificateProvider provider(SERVER_KEY_PATH, SERVER_CERT_PATH,
MALFORMED_CERT_PATH, 1);
EXPECT_EQ(provider.ValidateCredentials(),
absl::FailedPreconditionError("Invalid PEM."));
absl::FailedPreconditionError(
"Failed to parse root certificates as PEM: Invalid PEM."));
}

TEST(CredentialsTest, TlsServerCredentialsWithCrlChecking) {
Expand Down

0 comments on commit 4c7c1fa

Please sign in to comment.