Skip to content

Commit 9b96c7a

Browse files
vonzshikvonzshik
authored
Change the default value for CheckCertificateRevocation to false (npgsql#4092)
Fixes npgsql#4091 As a bonus, also fix a situation with VerifyFull + root cert, where we didn't check for remote certificate name mismatch
1 parent 7f35c4d commit 9b96c7a

3 files changed

Lines changed: 20 additions & 12 deletions

File tree

‎src/Npgsql/Internal/NpgsqlConnector.cs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -797,7 +797,7 @@ async Task RawOpen(SslMode sslMode, NpgsqlTimeout timeout, bool async, Cancellat
797797
}
798798
else if ((Settings.RootCertificate ?? PostgresEnvironment.SslCertRoot ?? PostgresEnvironment.SslCertRootDefault) is { } certRootPath)
799799
{
800-
certificateValidationCallback = SslRootValidation(certRootPath);
800+
certificateValidationCallback = SslRootValidation(certRootPath, sslMode == SslMode.VerifyFull);
801801
}
802802
else if (UserCertificateValidationCallback is not null)
803803
{
@@ -1589,12 +1589,24 @@ internal void ClearTransaction(Exception? disposeReason = null)
15891589
(sender, certificate, chain, sslPolicyErrors)
15901590
=> true;
15911591

1592-
static RemoteCertificateValidationCallback SslRootValidation(string certRootPath) =>
1592+
static RemoteCertificateValidationCallback SslRootValidation(string certRootPath, bool verifyFull) =>
15931593
(sender, certificate, chain, sslPolicyErrors) =>
15941594
{
15951595
if (certificate is null || chain is null)
15961596
return false;
15971597

1598+
// No errors here - no reason to check further
1599+
if (sslPolicyErrors == SslPolicyErrors.None)
1600+
return true;
1601+
1602+
// That's VerifyCA check and the only error is name mismatch - no reason to check further
1603+
if (!verifyFull && sslPolicyErrors == SslPolicyErrors.RemoteCertificateNameMismatch)
1604+
return true;
1605+
1606+
// That's VerifyFull check and we have name mismatch - no reason to check further
1607+
if (verifyFull && sslPolicyErrors.HasFlag(SslPolicyErrors.RemoteCertificateNameMismatch))
1608+
return false;
1609+
15981610
var certs = new X509Certificate2Collection();
15991611

16001612
#if NET5_0_OR_GREATER

‎src/Npgsql/NpgsqlConnectionStringBuilder.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,6 @@ public string? RootCertificate
547547
[Category("Security")]
548548
[Description("Whether to check the certificate revocation list during authentication.")]
549549
[DisplayName("Check Certificate Revocation")]
550-
[DefaultValue(true)]
551550
[NpgsqlConnectionStringProperty]
552551
public bool CheckCertificateRevocation
553552
{

‎test/Npgsql.Tests/SecurityTests.cs‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.Threading;
44
using System.Threading.Tasks;
55
using NUnit.Framework;
6+
using static Npgsql.Tests.TestUtil;
67

78
namespace Npgsql.Tests
89
{
@@ -39,20 +40,16 @@ public void Default_user_uses_md5_password()
3940
}
4041

4142
[Test, Description("Makes sure a certificate whose root CA isn't known isn't accepted")]
42-
public void Reject_self_signed_certificate(
43-
[Values(SslMode.VerifyCA, SslMode.VerifyFull)] SslMode sslMode,
44-
[Values] bool checkCertificateRevocation)
43+
public void Reject_self_signed_certificate([Values(SslMode.VerifyCA, SslMode.VerifyFull)] SslMode sslMode)
4544
{
46-
var connString = new NpgsqlConnectionStringBuilder(ConnectionString)
45+
var csb = new NpgsqlConnectionStringBuilder(ConnectionString)
4746
{
4847
SslMode = sslMode,
49-
CheckCertificateRevocation = checkCertificateRevocation
50-
}.ToString();
48+
CheckCertificateRevocation = false,
49+
};
5150

51+
using var _ = CreateTempPool(csb, out var connString);
5252
using var conn = new NpgsqlConnection(connString);
53-
// The following is necessary since a pooled connector may exist from a previous
54-
// SSL test
55-
NpgsqlConnection.ClearPool(conn);
5653

5754
var ex = Assert.Throws<NpgsqlException>(conn.Open)!;
5855
Assert.That(ex.InnerException, Is.TypeOf<AuthenticationException>());

0 commit comments

Comments
 (0)