eric-wang-1990 commented on code in PR #3962:
URL: https://github.com/apache/arrow-adbc/pull/3962#discussion_r2795847186
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2TlsImpl.cs:
##########
@@ -152,17 +283,46 @@ static internal bool ValidateCertificate(X509Certificate?
cert, SslPolicyErrors
if (string.IsNullOrEmpty(tlsProperties.TrustedCertificatePath))
{
- return
!policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateChainErrors) ||
(tlsProperties.AllowSelfSigned && IsSelfSigned(cert2));
+ // If chain errors exist, provide detailed error information
+ if
(policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateChainErrors))
+ {
+ if (tlsProperties.AllowSelfSigned && IsSelfSigned(cert2))
+ return true;
+
+ // Build a chain to get detailed error information
+ using (X509Chain defaultChain = new X509Chain())
+ {
+ defaultChain.ChainPolicy.RevocationMode =
tlsProperties.RevocationMode;
+ defaultChain.Build(cert2);
+
+ // Throw detailed error with actionable guidance
+ ThrowDetailedCertificateError(defaultChain,
tlsProperties);
+ }
+
+ return false;
+ }
+ return true;
}
X509Certificate2 trustedRoot = new
X509Certificate2(tlsProperties.TrustedCertificatePath);
X509Chain customChain = new();
customChain.ChainPolicy.ExtraStore.Add(trustedRoot);
// "tell the X509Chain class that I do trust this root certs and
it should check just the certs in the chain and nothing else"
customChain.ChainPolicy.VerificationFlags =
X509VerificationFlags.AllowUnknownCertificateAuthority;
- customChain.ChainPolicy.RevocationMode = X509RevocationMode.Online;
+ customChain.ChainPolicy.RevocationMode =
tlsProperties.RevocationMode;
bool chainValid = customChain.Build(cert2);
+
+ // Check for validation failures and provide detailed error message
+ if (!chainValid)
+ {
+ if (tlsProperties.AllowSelfSigned && IsSelfSigned(cert2))
+ return true;
+
+ // Throw detailed error with actionable guidance
+ ThrowDetailedCertificateError(customChain, tlsProperties);
+ }
+
return chainValid || (tlsProperties.AllowSelfSigned &&
IsSelfSigned(cert2));
Review Comment:
Can you simplify to
if (tlsProperties.AllowSelfSigned && IsSelfSigned(cert2))
return true;
if (!chainValid)
ThrowDetailedCertificateError(customChain, tlsProperties);
return true;
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2TlsImpl.cs:
##########
@@ -152,17 +283,46 @@ static internal bool ValidateCertificate(X509Certificate?
cert, SslPolicyErrors
if (string.IsNullOrEmpty(tlsProperties.TrustedCertificatePath))
{
- return
!policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateChainErrors) ||
(tlsProperties.AllowSelfSigned && IsSelfSigned(cert2));
+ // If chain errors exist, provide detailed error information
+ if
(policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateChainErrors))
+ {
+ if (tlsProperties.AllowSelfSigned && IsSelfSigned(cert2))
+ return true;
+
+ // Build a chain to get detailed error information
+ using (X509Chain defaultChain = new X509Chain())
+ {
+ defaultChain.ChainPolicy.RevocationMode =
tlsProperties.RevocationMode;
+ defaultChain.Build(cert2);
+
+ // Throw detailed error with actionable guidance
+ ThrowDetailedCertificateError(defaultChain,
tlsProperties);
+ }
+
+ return false;
Review Comment:
Can this line be reached?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
