diff --git a/README.md b/README.md index 3096c25..4d0f540 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ Async PostgreSQL client in Nim. - Pipeline mode — batch multiple operations in a single network round trip - Connection pooling with health checks and maintenance (broken connections discarded on acquire/release) - Pool cluster with read replica routing -- SSL/TLS support (disable, allow, prefer, require, verify-ca, verify-full) +- SSL/TLS support (disable, allow, prefer, require, verify-ca, verify-full) with optional client certificate authentication (mTLS) - MD5, SCRAM-SHA-256 and SCRAM-SHA-256-PLUS authentication - `channel_binding` policy (disable, prefer, require) to harden SCRAM against downgrade - DSN connection string parsing @@ -129,6 +129,17 @@ SSL backend differs by async backend: - asyncdispatch: OpenSSL (requires `-d:ssl`) - chronos: BearSSL (via [nim-bearssl](https://github.com/status-im/nim-bearssl)) +Client certificate authentication (mTLS) is enabled by setting `sslCert` and +`sslKey` on `ConnConfig` (or `sslcert=` / `sslkey=` in a DSN, which load the +files from disk). Both must be provided together, and `sslMode` must be +`sslPrefer` or stronger (`sslcert`/`sslkey` paired with `sslDisable` or +`sslAllow` is rejected at config time because TLS would not be negotiated). +The private key must be **unencrypted** on both backends — neither chronos +(BearSSL) nor asyncdispatch (OpenSSL) is wired to a passphrase callback. On +chronos the key specifically must be PKCS#8 PEM (RSA or EC); PKCS#1 is not +supported. When loaded via DSN on POSIX, the `sslkey` file must not be group- +or world-accessible (libpq parity). + ## Examples The [examples](examples/) directory contains runnable samples: diff --git a/async_postgres/pg_connection/dsn.nim b/async_postgres/pg_connection/dsn.nim index 68a2b9d..5b5f067 100644 --- a/async_postgres/pg_connection/dsn.nim +++ b/async_postgres/pg_connection/dsn.nim @@ -8,6 +8,8 @@ ## (in particular, does not touch `PgConnection`). import std/[strutils, uri] +when defined(posix): + import std/posix import ../[async_backend, pg_errors] import types @@ -150,6 +152,61 @@ proc buildHosts(hostStr, hostaddrStr, portStr: string): seq[HostEntry] = parsePort(p), ) +proc validateClientCertConfig*(config: ConnConfig) = + ## Reject inconsistent client certificate configurations early (at config + ## build time, before any connection is opened). Both halves of an mTLS + ## credential must be present together, and the SSL mode must actually + ## negotiate TLS — otherwise the cert/key would be silently ignored. + if (config.sslCert.len > 0) xor (config.sslKey.len > 0): + raise newException(PgError, ClientCertPairingErrorMsg) + if (config.sslCert.len > 0 or config.sslKey.len > 0) and + config.sslMode in {sslDisable, sslAllow}: + raise newException( + PgError, + "sslcert/sslkey require sslmode of prefer or stronger (got " & $config.sslMode & + "); they would otherwise be silently unused", + ) + +proc readPemFileParam(path, label: string): string = + ## Read a PEM parameter file (sslrootcert/sslcert), wrapping the stdlib + ## `IOError` into a `PgError` with the parameter name so the three sslX file + ## params share one read+error-wrap path. + try: + result = readFile(path) + except IOError: + raise newException(PgError, "Cannot read " & label & " file: " & path) + +proc readPrivateKeyFile(path: string): string = + ## Read an SSL private key file, rejecting it (libpq parity) when it is group- + ## or world-accessible. On POSIX the permission check and the read share one + ## file descriptor (`fstat` on the open handle, then read), so the bytes + ## returned are guaranteed to come from the inode that passed the check — this + ## closes the TOCTOU window a separate `stat()`+`readFile()` would leave open + ## when the key lives in an attacker-writable directory. + when defined(posix): + var f: File + if not open(f, path, fmRead): + raise newException(PgError, "Cannot read sslkey file: " & path) + try: + var st: Stat + if fstat(getFileHandle(f), st) != 0: + raise newException(PgError, "Cannot stat sslkey file: " & path) + if (st.st_mode.int and (S_IRWXG or S_IRWXO).int) != 0: + raise newException( + PgError, + "sslkey file has group or world accessible permissions, refusing to use: " & + path, + ) + result = readAll(f) + finally: + close(f) + else: + # No POSIX permission model to check (libpq also skips this on Windows). + try: + result = readFile(path) + except IOError: + raise newException(PgError, "Cannot read sslkey file: " & path) + proc applyParam*(result: var ConnConfig, key, val: string) = ## Apply a single connection parameter to a ConnConfig. ## @@ -194,10 +251,11 @@ proc applyParam*(result: var ConnConfig, key, val: string) = else: seconds(secs) of "sslrootcert": - try: - result.sslRootCert = readFile(val) - except IOError: - raise newException(PgError, "Cannot read sslrootcert file: " & val) + result.sslRootCert = readPemFileParam(val, "sslrootcert") + of "sslcert": + result.sslCert = readPemFileParam(val, "sslcert") + of "sslkey": + result.sslKey = readPrivateKeyFile(val) of "keepalives": try: result.keepAlive = parseInt(val) != 0 @@ -463,6 +521,8 @@ proc initConnConfig*( database = "", sslMode = sslPrefer, sslRootCert = "", + sslCert = "", + sslKey = "", channelBinding = cbPrefer, applicationName = "", connectTimeout = ZeroDuration, @@ -479,7 +539,7 @@ proc initConnConfig*( ): ConnConfig = ## Create a connection configuration with sensible defaults. ## For DSN-based configuration, use `parseDsn` instead. - ConnConfig( + result = ConnConfig( host: host, port: port, hostaddr: hostaddr, @@ -488,6 +548,8 @@ proc initConnConfig*( database: database, sslMode: sslMode, sslRootCert: sslRootCert, + sslCert: sslCert, + sslKey: sslKey, channelBinding: channelBinding, applicationName: applicationName, connectTimeout: connectTimeout, @@ -502,6 +564,7 @@ proc initConnConfig*( extraParams: extraParams, maxMessageSize: maxMessageSize, ) + validateClientCertConfig(result) proc parseDsn*(dsn: string): ConnConfig = ## Parse a PostgreSQL connection string into a ConnConfig. @@ -512,6 +575,7 @@ proc parseDsn*(dsn: string): ConnConfig = ## ## Both ``postgresql://`` and ``postgres://`` schemes are accepted for URI format. if dsn.startsWith("postgresql://") or dsn.startsWith("postgres://"): - parseUriDsn(dsn) + result = parseUriDsn(dsn) else: - parseKeyValueDsn(dsn) + result = parseKeyValueDsn(dsn) + validateClientCertConfig(result) diff --git a/async_postgres/pg_connection/lifecycle.nim b/async_postgres/pg_connection/lifecycle.nim index f36bc9e..ebc7dcb 100644 --- a/async_postgres/pg_connection/lifecycle.nim +++ b/async_postgres/pg_connection/lifecycle.nim @@ -620,6 +620,22 @@ proc connect*(config: ConnConfig): Future[PgConnection] = ) proc wrapped(): Future[PgConnection] {.async.} = + # `ConnConfig` may be built directly or mutated after parseDsn/initConnConfig, + # bypassing their `validateClientCertConfig`. `negotiateSSL` re-checks the + # cert/key pairing for modes that negotiate TLS, but it is skipped entirely + # for `sslDisable` and runs only on the plaintext-first leg for `sslAllow`, + # so a cert/key paired with those modes would be silently dropped with no + # error — the exact footgun the validation exists to prevent. Close that gap + # at the connect chokepoint. (Other modes still go through negotiateSSL's + # own guard, so this stays a narrow top-up rather than duplicating the full + # check on the hot path.) The builders raise `PgError`; surface it as the + # connect-path `PgConnectionError` (a `PgError` subtype). + if config.sslMode in {sslDisable, sslAllow} and + (config.sslCert.len > 0 or config.sslKey.len > 0): + try: + validateClientCertConfig(config) + except PgError as e: + raise newException(PgConnectionError, e.msg, e) # Compute the ordered host list once so the trace and the actual connection # attempts see the same order under lbhRandom. let hosts = config.orderedHosts() diff --git a/async_postgres/pg_connection/ssl.nim b/async_postgres/pg_connection/ssl.nim index 3d54732..cb44dd3 100644 --- a/async_postgres/pg_connection/ssl.nim +++ b/async_postgres/pg_connection/ssl.nim @@ -77,6 +77,8 @@ proc negotiateSSL*(conn: PgConnection, config: ConnConfig, sslHost: string) {.as raise newException( PgConnectionError, "A host name must be specified for a verified SSL connection" ) + if (config.sslCert.len > 0) xor (config.sslKey.len > 0): + raise newException(PgConnectionError, ClientCertPairingErrorMsg) let sslReq = encodeSSLRequest() var respChar: char var extraBytesBuffered = false @@ -137,6 +139,21 @@ proc negotiateSSL*(conn: PgConnection, config: ConnConfig, sslHost: string) {.as let serverName = if config.sslMode == sslVerifyFull: sslHost else: "" + # ``newTLSClientAsyncStream`` stores these on ``TLSAsyncStream`` + # (clientCertificate/clientPrivateKey) so BearSSL keeps a valid + # reference for the lifetime of conn.tlsStream — no extra retention + # on PgConnection is needed (unlike trustAnchorBufs above). + var clientCert: TLSCertificate + var clientKey: TLSPrivateKey + if config.sslCert.len > 0 and config.sslKey.len > 0: + try: + clientCert = TLSCertificate.init(config.sslCert) + clientKey = TLSPrivateKey.init(config.sslKey) + except TLSStreamProtocolError as e: + raise newException( + PgConnectionError, "Failed to load client certificate/key: " & e.msg + ) + if config.sslRootCert.len > 0: let parsed = parseTrustAnchors(config.sslRootCert) conn.trustAnchorBufs = parsed.backing @@ -149,6 +166,8 @@ proc negotiateSSL*(conn: PgConnection, config: ConnConfig, sslHost: string) {.as minVersion = TLSVersion.TLS12, maxVersion = TLSVersion.TLS12, trustAnchors = parsed.store, + certificate = clientCert, + privateKey = clientKey, ) else: conn.tlsStream = newTLSClientAsyncStream( @@ -158,6 +177,8 @@ proc negotiateSSL*(conn: PgConnection, config: ConnConfig, sslHost: string) {.as flags = flags, minVersion = TLSVersion.TLS12, maxVersion = TLSVersion.TLS12, + certificate = clientCert, + privateKey = clientKey, ) installX509Capture( conn.x509Capture, conn.tlsStream.ccontext.eng, addr conn.serverCertDer @@ -174,21 +195,48 @@ proc negotiateSSL*(conn: PgConnection, config: ConnConfig, sslHost: string) {.as else: SslCVerifyMode.CVerifyNone var ctx: SslContext - var tmpPath: string - if config.sslRootCert.len > 0: - let (tmpFile, tp) = createTempFile("pg_ca_", ".pem") - tmpPath = tp + var tmpPaths: seq[string] + + # std/net's newContext only accepts cert/key/CA as filesystem paths, so + # each PEM is written to a private temp file (0600, O_EXCL — see + # createTempFile) and removed in the `finally` below. The write is + # wrapped so the File handle is closed even if `write` throws (otherwise + # the fd would leak, holding the now-unlinked key contents open). + proc writeTempPem(content, prefix: string): string = + if content.len == 0: + return "" + let (f, p) = createTempFile(prefix, ".pem") + tmpPaths.add(p) try: - tmpFile.write(config.sslRootCert) - tmpFile.close() - ctx = newContext(verifyMode = verifyMode, caFile = tmpPath) - except: - removeFile(tmpPath) - raise - else: - ctx = newContext(verifyMode = verifyMode) + f.write(content) + finally: + f.close() + p try: + let caPath = writeTempPem(config.sslRootCert, "pg_ca_") + let certPath = writeTempPem(config.sslCert, "pg_cert_") + let keyPath = writeTempPem(config.sslKey, "pg_key_") + + try: + ctx = newContext( + verifyMode = verifyMode, + certFile = certPath, + keyFile = keyPath, + caFile = caPath, + ) + except CatchableError as e: + # std/net surfaces a malformed/mismatched client cert or key (and CA + # load failures) as a raw SslError/IOError. Translate to + # PgConnectionError so the asyncdispatch backend reports cert/key + # problems like the chronos backend does (which wraps + # TLSStreamProtocolError above). + raise newException( + PgConnectionError, + "Failed to load client certificate/key or CA: " & e.msg, + e, + ) + let hostname = if config.sslMode == sslVerifyFull: sslHost else: "" wrapConnectedSocket(ctx, conn.socket, handshakeAsClient, hostname) # wrapConnectedSocket skips name verification for IP hostnames; for @@ -215,8 +263,15 @@ proc negotiateSSL*(conn: PgConnection, config: ConnConfig, sslHost: string) {.as else: stderr.writeLine "pg_connection: server certificate unavailable; SCRAM-SHA-256-PLUS channel binding unavailable" finally: - if tmpPath.len > 0: - removeFile(tmpPath) + # Each removal is wrapped so one failure does not skip the rest, but + # we still surface failures because tmpPaths may contain the client + # private key PEM — silently leaving it in /tmp would be a footgun. + for p in tmpPaths: + try: + removeFile(p) + except OSError as e: + stderr.writeLine "pg_connection: failed to remove temp SSL file " & p & + ": " & e.msg else: raise newException(PgConnectionError, "SSL support requires compiling with -d:ssl") @@ -228,5 +283,10 @@ proc negotiateSSL*(conn: PgConnection, config: ConnConfig, sslHost: string) {.as # attacker can intercept the SSLRequest and reply 'N' to force # plaintext. Use sslRequire or stronger if security is needed. stderr.writeLine "pg_connection: SSL refused by server, falling back to plaintext (sslmode=prefer)" + if config.sslCert.len > 0: + # The configured client certificate cannot be presented over a plaintext + # connection — make the silent mTLS drop observable (sslmode=prefer allows + # this fallback; use sslmode=require or stronger to enforce the cert). + stderr.writeLine "pg_connection: client certificate will NOT be sent over the plaintext fallback connection" else: raise newException(PgConnectionError, "Unexpected SSL response: " & $respChar) diff --git a/async_postgres/pg_connection/types.nim b/async_postgres/pg_connection/types.nim index 0aea2fd..d2c3221 100644 --- a/async_postgres/pg_connection/types.nim +++ b/async_postgres/pg_connection/types.nim @@ -116,6 +116,16 @@ type ## to `sslPrefer` (libpq parity); a raw zero-initialized `ConnConfig` has ## `sslDisable`. sslRootCert*: string ## PEM-encoded CA certificate(s) for sslVerifyCa/sslVerifyFull + sslCert*: string + ## PEM-encoded client certificate (and any intermediates) for mutual TLS. + ## Must be paired with ``sslKey``; ``sslMode`` must also be ``sslPrefer`` + ## or stronger, otherwise TLS would not be negotiated and the credential + ## would be silently unused — config validation rejects that. + sslKey*: string + ## PEM-encoded client private key for mutual TLS. The key must be + ## **unencrypted** on both backends (no passphrase callback is wired up). + ## On chronos/BearSSL specifically it must be PKCS#8 (RSA or EC); PKCS#1 + ## is not supported. Must be paired with ``sslCert``. channelBinding*: ChannelBindingMode ## SCRAM channel binding policy (default cbPrefer). `cbRequire` fails the ## connection if SCRAM-SHA-256-PLUS cannot actually be used (libpq parity). @@ -671,7 +681,14 @@ else: type CopyInCallback* = proc(): Future[seq[byte]] {.gcsafe.} ## Callback supplying data chunks during streaming COPY IN. Return empty seq to finish. -const RecvBufSize* = 131072 ## Size of the temporary read buffer for recv operations +const + RecvBufSize* = 131072 ## Size of the temporary read buffer for recv operations + + ClientCertPairingErrorMsg* = + "sslcert and sslkey must be provided together for client certificate auth" + ## Shared by the config-time validation (`validateClientCertConfig`, raised as + ## `PgError`) and the connect-time guard in `negotiateSSL` (raised as + ## `PgConnectionError`); kept here so the wording can't drift between them. # HostEntry accessors diff --git a/tests/test_dsn.nim b/tests/test_dsn.nim index defaf73..8307667 100644 --- a/tests/test_dsn.nim +++ b/tests/test_dsn.nim @@ -1,7 +1,27 @@ -import std/[random, unittest] +import std/[random, unittest, os, tempfiles] +when defined(posix): + import std/posix import ../async_postgres/[async_backend, pg_connection] +const dummyPem = "-----BEGIN CERTIFICATE-----\ndummy\n-----END CERTIFICATE-----\n" + +proc writeKeyFile(content: string, mode: int = 0o600): string = + ## Create a temp file holding ``content`` with the requested POSIX mode and + ## return its path. Caller is responsible for removing it. + let (f, p) = createTempFile("pg_test_key_", ".pem") + f.write(content) + f.close() + when defined(posix): + discard chmod(cstring(p), Mode(mode)) + p + +proc writePemFile(content: string): string = + let (f, p) = createTempFile("pg_test_pem_", ".pem") + f.write(content) + f.close() + p + suite "parseDsn": test "full DSN": let cfg = parseDsn("postgresql://myuser:mypass@dbhost:5433/mydb") @@ -364,6 +384,14 @@ suite "parseDsn": expect PgError: discard parseDsn("postgresql://host/db?sslrootcert=/nonexistent/file.pem") + test "error: sslcert file not found": + expect PgError: + discard parseDsn("postgresql://host/db?sslcert=/nonexistent/file.pem") + + test "error: sslkey file not found": + expect PgError: + discard parseDsn("postgresql://host/db?sslkey=/nonexistent/file.pem") + test "multi-host DSN": let cfg = parseDsn("postgresql://h1:5432,h2:5433/db") check cfg.hosts.len == 2 @@ -605,6 +633,65 @@ suite "parseDsn": check cfg.hosts[1] == HostEntry(host: "h2", port: 5433) check cfg.hosts[2] == HostEntry(host: "h3", port: 5432) + test "sslcert/sslkey loaded from valid files": + let certPath = writePemFile(dummyPem) + let keyPath = writeKeyFile(dummyPem) + defer: + removeFile(certPath) + removeFile(keyPath) + let dsn = + "postgresql://host/db?sslmode=require&sslcert=" & certPath & "&sslkey=" & keyPath + let cfg = parseDsn(dsn) + check cfg.sslCert == dummyPem + check cfg.sslKey == dummyPem + + test "error: sslcert with sslmode=disable rejected": + let certPath = writePemFile(dummyPem) + let keyPath = writeKeyFile(dummyPem) + defer: + removeFile(certPath) + removeFile(keyPath) + expect PgError: + discard parseDsn( + "postgresql://host/db?sslmode=disable&sslcert=" & certPath & "&sslkey=" & keyPath + ) + + test "error: sslcert with sslmode=allow rejected": + let certPath = writePemFile(dummyPem) + let keyPath = writeKeyFile(dummyPem) + defer: + removeFile(certPath) + removeFile(keyPath) + expect PgError: + discard parseDsn( + "postgresql://host/db?sslmode=allow&sslcert=" & certPath & "&sslkey=" & keyPath + ) + + when defined(posix): + test "error: sslkey with group-readable permissions rejected": + let certPath = writePemFile(dummyPem) + let keyPath = writeKeyFile(dummyPem, 0o640) + defer: + removeFile(certPath) + removeFile(keyPath) + expect PgError: + discard parseDsn( + "postgresql://host/db?sslmode=require&sslcert=" & certPath & "&sslkey=" & + keyPath + ) + + test "error: sslkey with world-readable permissions rejected": + let certPath = writePemFile(dummyPem) + let keyPath = writeKeyFile(dummyPem, 0o604) + defer: + removeFile(certPath) + removeFile(keyPath) + expect PgError: + discard parseDsn( + "postgresql://host/db?sslmode=require&sslcert=" & certPath & "&sslkey=" & + keyPath + ) + suite "parseDsn keyword=value": test "full connection string": let cfg = parseDsn("host=dbhost port=5433 dbname=mydb user=myuser password=mypass") diff --git a/tests/test_ssl.nim b/tests/test_ssl.nim index 70254d4..b50b0d0 100644 --- a/tests/test_ssl.nim +++ b/tests/test_ssl.nim @@ -419,6 +419,118 @@ suite "SSL negotiation - sslVerifyCa": let config = ConnConfig() check config.sslRootCert == "" + test "ConnConfig sslCert and sslKey default to empty": + let config = ConnConfig() + check config.sslCert == "" + check config.sslKey == "" + +suite "initConnConfig client certificate validation": + test "sslCert without sslKey is rejected": + expect PgError: + discard initConnConfig(sslMode = sslRequire, sslCert = "dummy") + + test "sslKey without sslCert is rejected": + expect PgError: + discard initConnConfig(sslMode = sslRequire, sslKey = "dummy") + + test "sslCert/sslKey with sslDisable is rejected": + expect PgError: + discard initConnConfig(sslMode = sslDisable, sslCert = "cert", sslKey = "key") + + test "sslCert/sslKey with sslAllow is rejected": + expect PgError: + discard initConnConfig(sslMode = sslAllow, sslCert = "cert", sslKey = "key") + + test "sslCert/sslKey with sslPrefer is accepted": + let cfg = initConnConfig(sslMode = sslPrefer, sslCert = "cert", sslKey = "key") + check cfg.sslCert == "cert" + check cfg.sslKey == "key" + + test "sslCert/sslKey with sslRequire is accepted": + let cfg = initConnConfig(sslMode = sslRequire, sslCert = "cert", sslKey = "key") + check cfg.sslCert == "cert" + check cfg.sslKey == "key" + +suite "Client certificate config validation": + test "providing only sslCert raises PgConnectionError": + var raised = false + var msgMatches = false + + proc testBody() {.async.} = + let ms = startMockServer() + + proc serverHandler() {.async.} = + let st = await ms.accept() + try: + discard await readN(st, 8) + await sendBytes(st, @[byte('S')]) + except CatchableError: + discard + await closeClient(st) + + let serverFut = serverHandler() + + let config = ConnConfig( + host: "127.0.0.1", + port: ms.port, + user: "test", + database: "test", + sslMode: sslRequire, + sslCert: "dummy", + ) + + try: + let conn = await connect(config) + await conn.close() + except PgConnectionError as e: + raised = true + msgMatches = "sslcert and sslkey must be provided together" in e.msg + + await serverFut + await closeServer(ms) + + waitFor testBody() + check raised + check msgMatches + + test "providing only sslKey raises PgConnectionError": + var raised = false + + proc testBody() {.async.} = + let ms = startMockServer() + + proc serverHandler() {.async.} = + let st = await ms.accept() + try: + discard await readN(st, 8) + await sendBytes(st, @[byte('S')]) + except CatchableError: + discard + await closeClient(st) + + let serverFut = serverHandler() + + let config = ConnConfig( + host: "127.0.0.1", + port: ms.port, + user: "test", + database: "test", + sslMode: sslRequire, + sslKey: "dummy", + ) + + try: + let conn = await connect(config) + await conn.close() + except PgConnectionError: + raised = true + + await serverFut + await closeServer(ms) + + waitFor testBody() + check raised + suite "SSL negotiation - sslAllow": test "sslAllow connects without SSL when server accepts plaintext": var connState: PgConnState