[Add] configurable TLS and certificate verification for the PostgreSQL connection by samatstariongroup · Pull Request #410 · STARIONGROUP/COMET-WebServices-Community-Edition

samatstariongroup · 2026-10-20T07:42:25Z

Closes #406. Also fixes two items of #409.

Summary

The connection to PostgreSQL could not be secured. The services had no TLS settings, so Npgsql's default SslMode=Prefer always applied: plain text against a server without TLS, and no certificate verification against a server with TLS. This PR makes TLS configurable, verifiable and diagnosable.

Configuration (`Backtier` section, also as environment variables)

Setting	Default	Purpose
`SslMode`	`Prefer` (unchanged behaviour)	`Disable`, `Allow`, `Prefer`, `Require`, `VerifyCA`, `VerifyFull`. A misspelled value stops startup and lists the valid values
`RootCertificate`	empty	CA used to verify the server (PEM or DER). When empty: `PGSSLROOTCERT`, then the default `root.crt`, then the OS trust store, the same order as Npgsql
`ClientCertificate`, `ClientCertificateKey`, `ClientCertificatePassword`	empty	Optional client certificate: PEM (optionally encrypted key) or PFX
`CheckCertificateRevocation`	`false`	An invalid value stops startup

Behaviour

Connection strings: both are built with NpgsqlConnectionStringBuilder from one shared method, so cdp4server and cdp4manage always get identical TLS settings. A password containing ; or = no longer breaks the connection string.
Startup validation: before connecting, the certificate files are checked (they exist, can be read and parsed, and the key and password match). Each problem is reported with the setting to change.
TLS state in the log: after connecting, the log shows whether the connection is encrypted and with which TLS version and cipher (pg_stat_ssl). It warns when the connection is not encrypted, or not verified (anything other than VerifyFull).
TLS failures stop at once, with diagnostics: before, the server retried for BacktierWaitTime (300 s by default). Npgsql reports every rejected certificate with the same message, so a diagnostic TLS handshake (no credentials are sent) explains the actual cause:
- issuer not trusted by the configured CA, listing both
- host name not in the certificate, listing the valid names
- server or CA certificate expired or not yet valid
- revocation status unknown
- no TLS version in common
- server without TLS while SslMode requires it, or a server that only accepts TLS with SslMode Disable
Other permanent database errors stop at once ([Bug]: Duplicate console log lines in the Docker image, and startup logging must explain failures #409): wrong password (28P01), rejected by pg_hba.conf (28000) and no CONNECT privilege (42501). A missing database (3D000) stops at once unless seed or import is enabled, because then POST /Data/Exchange creates it; the waiting log says so. Temporary errors are still retried: connection refused, database starting up (57P03), too many connections.
"Ready" log ([Bug]: Duplicate console log lines in the Docker image, and startup logging must explain failures #409): "ready to accept requests at …" is now logged only after the data store check and the migrations, with the addresses actually listened on. Before, "Ready to accept connections" was logged before the server had started.
Exit code: a failed startup exits with code 1 instead of 0, so restart policies and orchestrators see it. A shutdown during startup is not treated as a failure. An invalid configuration is logged as one line instead of a stack trace.
Docker Compose: docker-compose.yml and docker-compose-nginx.yml now publish PostgreSQL on 127.0.0.1 only. Before, they published it on all interfaces, with default credentials.

CI

The integration suite now runs against a TLS-only PostgreSQL with SslMode=VerifyFull. The tests only use HTTP, so the whole suite runs unchanged.

Certificates: a throw-away CA and server certificate are created for each run; no secrets are needed.
Database: the same test database image, started with ssl=on and a pg_hba.conf that only accepts TLS connections over the network (hostssl).
TLS check: after the seed, a step waits for "ready" and fails the job unless the API log shows an encrypted, verified connection. The integration tests also start only once the API is ready.
Misconfiguration check: after the tests, three short runs check that a wrong CA, a wrong host name and SslMode=Disable each stop the API with exit code 1 and the expected message.

Behaviour changes to note

Existing deployments with a database without TLS now log a "NOT encrypted" warning at every startup. That is intended.
Wrong database credentials, or a missing database with seed and import disabled, now stop startup immediately instead of after BacktierWaitTime.
A failed startup now exits with code 1, so a container with restart: on-failure restarts it.

Test plan

dotnet build COMET-WebServices.sln: 0 errors. The warnings are existing NuGet package warnings, now tracked in [Update]: Docker base image aspnet:10.0.7 is six security releases behind (latest 10.0.13) #408.
dotnet test COMET-WebServices.sln: 489 passed, 0 failed (432 of them in CometServer.Tests). The certificate tests use certificates generated inside the tests, and the diagnostics are tested with real TLS handshakes against an in-process server.

Smoke tested by hand against stariongroup/cdp4-database-community-edition:4.2.0, with and without TLS:

Scenario	Result
No TLS, `Prefer`	starts, warns "NOT encrypted"
TLS, `Prefer`	encrypted, warns "not verified"
TLS, `VerifyFull`, correct CA (also through `PGSSLROOTCERT`)	encrypted and verified, no warning
Wrong CA / wrong host name	stops at once with the diagnostic finding
`VerifyFull` against a server without TLS	stops at once
`Disable` against a TLS-only server	stops at once
Missing certificate file, wrong PFX password, misspelled `SslMode`	stops at once, exit code ≠ 0
Wrong database password	stops at once, exit code 1
Database not reachable yet	retried, with a one-line reason

Docker image built from this branch on Alpine: OpenSSL present, VerifyFull works (TLS 1.3).
CI flow rehearsed locally against stariongroup/cdp4-test-database-community-edition:4.2.0 in TLS-only mode: seed over TLS, "ready", authenticated GET /SiteDirectory returns 200.
Integration suite over TLS: runs in this PR's CI.

Not in this PR

A compose example with a TLS-only database, and the wiki page "TLS between WebServices and PostgreSQL" ([Feature]: Configurable TLS and certificate verification for the PostgreSQL connection #406).
The remaining [Bug]: Duplicate console log lines in the Docker image, and startup logging must explain failures #409 items: duplicate console log lines in the Docker image, the production log file name, the data-protection warning.
The base image and NuGet package updates ([Update]: Docker base image aspnet:10.0.7 is six security releases behind (latest 10.0.13) #408).

🤖 Generated with Claude Code

…L connection; fixes #406 - New Backtier settings: SslMode, RootCertificate, ClientCertificate, ClientCertificateKey, ClientCertificatePassword, CheckCertificateRevocation. Default stays SslMode=Prefer; PGSSLROOTCERT and the default root.crt are honoured like Npgsql does. - Both connection strings are built with NpgsqlConnectionStringBuilder from one shared method (identical TLS settings, no injection through ';' or '=' in passwords). - Startup validates the certificate files (exist, readable, parseable, key and password match) and stops with a clear message when they are not. - The startup log shows whether the connection is encrypted (TLS version and cipher from pg_stat_ssl) and warns when it is not encrypted or not verified. - A TLS handshake failure stops startup at once; a diagnostic handshake (no credentials sent) explains the cause: untrusted issuer, host name mismatch, expired server or CA certificate, revocation, no common TLS version. - Permanent errors stop startup at once instead of waiting for BacktierWaitTime: wrong password (28P01), pg_hba rejection (28000), no CONNECT privilege (42501), and a missing database (3D000) unless it can still be created through /Data/Exchange (seed or import enabled) (#409). - "ready to accept requests" is logged only after the data store check and migrations, with the addresses actually listened on (#409). - A failed startup exits with code 1; invalid configuration is logged as one line instead of a stack trace. - docker-compose.yml and docker-compose-nginx.yml publish PostgreSQL on 127.0.0.1 only. - CI runs the integration suite against a TLS-only PostgreSQL with SslMode=VerifyFull, checks the TLS state in the API log, and checks that TLS misconfigurations stop the API. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

lxatstariongroup

Approved, but please check SQ thingies.

joao4all

LGTM, small comments and suggestions

joao4all · 2026-10-20T12:31:44Z

+            this.applicationLifetime.Verify(x => x.StopApplication(), Times.Once);
+            this.dataStoreConnectionChecker.Verify(x => x.CheckConnectionAsync(It.IsAny<CancellationToken>()), Times.Never);
+            this.cometHasStartedService.Verify(x => x.SetHasStartedAndIsReady(true), Times.Never);


You could use assert multiple here

joao4all · 2026-10-20T12:33:19Z

+                    It.IsAny<EventId>(),
+                    It.Is<It.IsAnyType>((state, _) => state.ToString().Contains("ready to accept requests")),
+                    It.IsAny<Exception>(),
+                    It.IsAny<Func<It.IsAnyType, Exception, string>>()),


Maybe supressing the warning with a tag for the test fixture, like

[SuppressMessage("Performance", "CA1873:Avoid potentially expensive logging", Justification = "Moq expression tree in unit test.")]

Since it will be evaluated only for tests, the impact is negligible.

joao4all · 2026-10-20T12:46:47Z

+        /// <returns>
+        /// The certificate
+        /// </returns>


I think it would easier to read if this (and param, etc) doc was just one line, like

/// <returns>The certificate</returns>

…S diagnostics handshake - S4830: the diagnostic handshake no longer accepts any certificate. The validation callback records the certificate and chain but returns the real validation result; a rejected handshake now evaluates the recorded certificate, so the findings stay the same. - CA2012: Moq returns a fresh ValueTask per call in the checker tests. - CA1859, S3267, CA1826, CA1822: small code cleanups. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

sonarqubecloud · 2026-10-20T16:11:33Z

Quality Gate passed

Issues
30 New issues
1 Accepted issue

Measures
0 Security Hotspots
85.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-advanced-security Bot found potential problems Oct 20, 2026

View reviewed changes

Comment thread CometServer/Services/DataStore/PostgreSqlTlsDiagnostics.cs Dismissed

samatstariongroup requested review from joao4all and lxatstariongroup October 20, 2026 07:57

lxatstariongroup approved these changes Oct 20, 2026

View reviewed changes

joao4all approved these changes Oct 20, 2026

View reviewed changes

samatstariongroup mentioned this pull request Oct 20, 2026

[Improvement]: Reduce logging overhead on hot paths (193 CA1873 findings), use [LoggerMessage] source generators #411

Open

6 tasks

samatstariongroup merged commit 0fbe2ac into development Oct 20, 2026
9 checks passed

samatstariongroup deleted the feature/GH406-postgresql-tls branch October 20, 2026 16:14

samatstariongroup mentioned this pull request Oct 20, 2026

[Docs]: Follow-up of #406 - wiki page, compose example and default passwords for TLS to PostgreSQL #412

Open

3 of 5 tasks

Navigation Menu

Repository navigation

[Add] configurable TLS and certificate verification for the PostgreSQL connection - #410

[Add] configurable TLS and certificate verification for the PostgreSQL connection#410
samatstariongroup merged 2 commits into
developmentfrom
feature/GH406-postgresql-tls

samatstariongroup commented Oct 20, 2026 •

edited

Loading

Uh oh!

Uh oh!

lxatstariongroup left a comment

Uh oh!

joao4all left a comment

Uh oh!

joao4all Oct 20, 2026

Uh oh!

joao4all Oct 20, 2026 •

edited

Loading

Uh oh!

joao4all Oct 20, 2026

Uh oh!

sonarqubecloud Bot commented Oct 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

samatstariongroup commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Configuration (Backtier section, also as environment variables)

Behaviour

CI

Behaviour changes to note

Test plan

Not in this PR

Uh oh!

Uh oh!

lxatstariongroup left a comment

Choose a reason for hiding this comment

Uh oh!

joao4all left a comment

Choose a reason for hiding this comment

Uh oh!

joao4all Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

joao4all Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joao4all Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Oct 20, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

samatstariongroup commented Oct 20, 2026 •

edited

Loading

Configuration (`Backtier` section, also as environment variables)

joao4all Oct 20, 2026 •

edited

Loading