Add support for environment variable PGSSLNEGOTIATION (#5882)

vonzshik · web-flow · commit ab8e8db61617 · 2024-10-15T09:51:01.000Z
Related to #5677 Followup to #5881
diff --git a/src/Npgsql/Internal/NpgsqlConnector.cs b/src/Npgsql/Internal/NpgsqlConnector.cs
@@ -783,9 +783,9 @@ async Task RawOpen(SslMode sslMode, NpgsqlTimeout timeout, bool async, Cancellat
 
             IsSecure = false;
 
-            if (Settings.SslNegotiation == SslNegotiation.Direct)
+            if (GetSslNegotiation(Settings) == SslNegotiation.Direct)
             {
-                // We already check that in NpgsqlConnectionStringBuilder.PostProcessAndValidate, but just on the off case
+                // We already check that in NpgsqlConnectionStringBuilder.PostProcessAndValidate, but since we also allow environment variables...
                 if (Settings.SslMode is not SslMode.Require and not SslMode.VerifyCA and not SslMode.VerifyFull)
                     throw new ArgumentException("SSL Mode has to be Require or higher to be used with direct SSL Negotiation");
                 await DataSource.TransportSecurityHandler.NegotiateEncryption(async, this, sslMode, timeout, cancellationToken).ConfigureAwait(false);
@@ -836,6 +836,20 @@ async Task RawOpen(SslMode sslMode, NpgsqlTimeout timeout, bool async, Cancellat
         }
     }
 
+    static SslNegotiation GetSslNegotiation(NpgsqlConnectionStringBuilder settings)
+    {
+        if (settings.UserProvidedSslNegotiation is { } userProvidedSslNegotiation)
+            return userProvidedSslNegotiation;
+
+        if (PostgresEnvironment.SslNegotiation is { } sslNegotiationEnv)
+        {
+            if (Enum.TryParse<SslNegotiation>(sslNegotiationEnv, ignoreCase: true, out var sslNegotiation))
+                return sslNegotiation;
+        }
+
+        return SslNegotiation.Postgres;
+    }
+
     internal async Task NegotiateEncryption(SslMode sslMode, NpgsqlTimeout timeout, bool async, CancellationToken cancellationToken)
     {
         var clientCertificates = new X509Certificate2Collection();
diff --git a/src/Npgsql/NpgsqlConnectionStringBuilder.cs b/src/Npgsql/NpgsqlConnectionStringBuilder.cs
@@ -467,19 +467,18 @@ public SslMode SslMode
     [Category("Security")]
     [Description("Controls how SSL encryption is negotiated with the server, if SSL is used.")]
     [DisplayName("SSL Negotiation")]
-    [DefaultValue(SslNegotiation.Postgres)]
     [NpgsqlConnectionStringProperty]
     public SslNegotiation SslNegotiation
     {
-        get => _sslNegotiation;
+        get => UserProvidedSslNegotiation ?? SslNegotiation.Postgres;
         set
         {
-            _sslNegotiation = value;
+            UserProvidedSslNegotiation = value;
             SetValue(nameof(SslNegotiation), value);
         }
     }
 
-    SslNegotiation _sslNegotiation;
+    internal SslNegotiation? UserProvidedSslNegotiation { get; private set; }
 
     /// <summary>
     /// Location of a client certificate to be sent to the server.
diff --git a/src/Npgsql/PostgresEnvironment.cs b/src/Npgsql/PostgresEnvironment.cs
@@ -48,11 +48,13 @@ internal static string? SslCertRootDefault
 
     internal static string? TargetSessionAttributes => Environment.GetEnvironmentVariable("PGTARGETSESSIONATTRS");
 
+    internal static string? SslNegotiation => Environment.GetEnvironmentVariable("PGSSLNEGOTIATION");
+
     static string? GetHomeDir()
         => Environment.GetEnvironmentVariable(RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "APPDATA" : "HOME");
 
     static string? GetHomePostgresDir()
         => GetHomeDir() is string homedir
             ? Path.Combine(homedir, RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "postgresql" : ".postgresql")
             : null;
-}
+}
diff --git a/test/Npgsql.Tests/SecurityTests.cs b/test/Npgsql.Tests/SecurityTests.cs
@@ -522,6 +522,33 @@ public void Direct_ssl_requires_correct_sslmode([Values] SslMode sslMode)
         }
     }
 
+    [Test]
+    [NonParallelizable] // Sets environment variable
+    public async Task Direct_ssl_via_env_requires_correct_sslmode()
+    {
+        await using var adminConn = await OpenConnectionAsync();
+        MinimumPgVersion(adminConn, "17.0");
+
+        // NonParallelizable attribute doesn't work with parameters that well
+        foreach (var sslMode in new[] { SslMode.Disable, SslMode.Allow, SslMode.Prefer, SslMode.Require })
+        {
+            using var _ = SetEnvironmentVariable("PGSSLNEGOTIATION", nameof(SslNegotiation.Direct));
+            await using var dataSource = CreateDataSource(csb =>
+            {
+                csb.SslMode = sslMode;
+            });
+            if (sslMode is SslMode.Disable or SslMode.Allow or SslMode.Prefer)
+            {
+                var ex = Assert.ThrowsAsync<ArgumentException>(async () => await dataSource.OpenConnectionAsync())!;
+                Assert.That(ex.Message, Is.EqualTo("SSL Mode has to be Require or higher to be used with direct SSL Negotiation"));
+            }
+            else
+            {
+                await using var conn = await dataSource.OpenConnectionAsync();
+            }
+        }
+    }
+
     #region Setup / Teardown / Utils
 
     [OneTimeSetUp]

Original file line number	Diff line number	Diff line change
`@@ -467,19 +467,18 @@ public SslMode SslMode`
`467`	`467`	`[Category("Security")]`
`468`	`468`	`[Description("Controls how SSL encryption is negotiated with the server, if SSL is used.")]`
`469`	`469`	`[DisplayName("SSL Negotiation")]`
`470`		`- [DefaultValue(SslNegotiation.Postgres)]`
`471`	`470`	`[NpgsqlConnectionStringProperty]`
`472`	`471`	`public SslNegotiation SslNegotiation`
`473`	`472`	`{`
`474`		`- get => _sslNegotiation;`
	`473`	`+ get => UserProvidedSslNegotiation ?? SslNegotiation.Postgres;`
`475`	`474`	`set`
`476`	`475`	`{`
`477`		`- _sslNegotiation = value;`
	`476`	`+ UserProvidedSslNegotiation = value;`
`478`	`477`	`SetValue(nameof(SslNegotiation), value);`
`479`	`478`	`}`
`480`	`479`	`}`
`481`	`480`
`482`		`- SslNegotiation _sslNegotiation;`
	`481`	`+ internal SslNegotiation? UserProvidedSslNegotiation { get; private set; }`
`483`	`482`
`484`	`483`	`/// <summary>`
`485`	`484`	`/// Location of a client certificate to be sent to the server.`