Skip to content

Enable TLS support for PostgreSQL (#3831) #484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fghanmi merged 2 commits into from
Dec 10, 2025
Merged

Enable TLS support for PostgreSQL (#3831) #484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16041e5
Enable TLS support for PostgreSQL (#3831)
fghanmi Dec 3, 2025
c1a6047
update trillian server/signer pipelines on-cel-expression
fghanmi Dec 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .tekton/logserver-pull-request.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ metadata:
pipelinesascode.tekton.dev/on-cel-expression: event == "pull_request" && target_branch
== "main" && ( "go.mod".pathChanged() || "go.sum".pathChanged() || ".tekton/logserver-pull-request.yaml".pathChanged()
|| "Dockerfile.logserver.rh".pathChanged() || "cmd/trillian_log_server/***".pathChanged()
|| "storage/***".pathChanged()
|| "trigger-konflux-builds.txt".pathChanged() )
creationTimestamp: null
labels:
Expand Down
1 change: 1 addition & 0 deletions .tekton/logserver-push.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ metadata:
pipelinesascode.tekton.dev/on-cel-expression: event == "push" && target_branch
== "main" && ( "go.mod".pathChanged() || "go.sum".pathChanged() || ".tekton/logserver-push.yaml".pathChanged()
|| "Dockerfile.logserver.rh".pathChanged() || "cmd/trillian_log_server/***".pathChanged()
|| "storage/***".pathChanged()
|| "trigger-konflux-builds.txt".pathChanged() )
creationTimestamp: null
labels:
Expand Down
1 change: 1 addition & 0 deletions .tekton/logsigner-pull-request.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ metadata:
pipelinesascode.tekton.dev/on-cel-expression: event == "pull_request" && target_branch
== "main" && ( "go.mod".pathChanged() || "go.sum".pathChanged() || ".tekton/logsigner-pull-request.yaml".pathChanged()
|| "Dockerfile.logsigner.rh".pathChanged() || "cmd/trillian_log_signer/***".pathChanged()
|| "storage/***".pathChanged()
|| "trigger-konflux-builds.txt".pathChanged() )
creationTimestamp: null
labels:
Expand Down
1 change: 1 addition & 0 deletions .tekton/logsigner-push.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ metadata:
pipelinesascode.tekton.dev/on-cel-expression: event == "push" && target_branch
== "main" && ( "go.mod".pathChanged() || "go.sum".pathChanged() || ".tekton/logsigner-pull-request.yaml".pathChanged()
|| "Dockerfile.logsigner.rh".pathChanged() || "cmd/trillian_log_signer/***".pathChanged()
|| "storage/***".pathChanged()
|| "trigger-konflux-builds.txt".pathChanged() )
creationTimestamp: null
labels:
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
* Bump golangci-lint to v2.1.6
* Fix leader resignation during a graceful shutdown by @osmman in https://github.com/google/trillian/pull/3790
* Add optional gRPC message size limit via `--max_msg_size_bytes` flag by @fghanmi in https://github.com/google/trillian/pull/3801
* Add TLS support for PostgreSQL: https://github.com/google/trillian/pull/3831
* `--postgresql_tls_ca`: users can provide a CA certificate, that is used to establish a secure communication with PostgreSQL server.
* `--postgresql_verify_full`: users can enable full TLS verification for PostgreSQL (sslmode=verify-full). If false, only sslmode=verify-ca is used.

## v1.7.2

Expand Down
44 changes: 42 additions & 2 deletions storage/postgresql/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ package postgresql

import (
"flag"
"fmt"
"net/url"
"os"
"sync"

"github.com/google/trillian/monitoring"
Expand All @@ -25,7 +28,9 @@ import (
)

var (
postgreSQLURI = flag.String("postgresql_uri", "postgresql:///defaultdb?host=localhost&user=test", "Connection URI for PostgreSQL database")
postgreSQLURI = flag.String("postgresql_uri", "postgresql:///defaultdb?host=localhost&user=test", "Connection URI for PostgreSQL database")
postgresqlTLSCA = flag.String("postgresql_tls_ca", "", "Path to the CA certificate file for PostgreSQL TLS connection ")
postgresqlVerifyFull = flag.Bool("postgresql_verify_full", false, "Enable full TLS verification for PostgreSQL (sslmode=verify-full). If false, only sslmode=verify-ca is used.")

postgresqlMu sync.Mutex
postgresqlErr error
Expand Down Expand Up @@ -76,7 +81,14 @@ func getPostgreSQLDatabaseLocked() (*pgxpool.Pool, error) {
if postgresqlDB != nil || postgresqlErr != nil {
return postgresqlDB, postgresqlErr
}
db, err := OpenDB(*postgreSQLURI)
uri := *postgreSQLURI
var err error
uri, err = BuildPostgresTLSURI(uri)
if err != nil {
postgresqlErr = err
return nil, err
}
db, err := OpenDB(uri)
if err != nil {
postgresqlErr = err
return nil, err
Expand All @@ -97,3 +109,31 @@ func (s *postgresqlProvider) Close() error {
s.db.Close()
return nil
}

// BuildPostgresTLSURI modifies the given PostgreSQL URI to include TLS parameters based on flags.
// It returns the modified URI and any error encountered.
func BuildPostgresTLSURI(uri string) (string, error) {
if *postgresqlTLSCA == "" {
return uri, nil
}
if _, err := os.Stat(*postgresqlTLSCA); err != nil {
postgresqlErr = fmt.Errorf("postgresql CA file error: %w", err)
return "", postgresqlErr
}
u, err := url.Parse(uri)
if err != nil {
Comment on lines +115 to +124
Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): Avoid mutating the global postgresqlErr inside this helper to reduce hidden side effects and possible races.

This helper both returns an error and mutates the package-level postgresqlErr. Because BuildPostgresTLSURI is exported and not clearly tied to the getPostgreSQLDatabaseLocked mutex, it could race on postgresqlErr or clobber an existing error when called from elsewhere. Prefer making this function side‑effect‑free (returning only (string, error)) and have the caller update postgresqlErr under the appropriate lock if needed.

postgresqlErr = fmt.Errorf("invalid postgresql URI %q: %w", uri, err)
return "", postgresqlErr
}
q := u.Query()
q.Set("sslrootcert", *postgresqlTLSCA)
if *postgresqlVerifyFull {
q.Set("sslmode", "verify-full")
} else {
if q.Get("sslmode") == "" {
q.Set("sslmode", "verify-ca")
}
}
u.RawQuery = q.Encode()
return u.String(), nil
}
106 changes: 106 additions & 0 deletions storage/postgresql/provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ package postgresql

import (
"flag"
"net/url"
"os"
"path/filepath"
"strings"
"testing"

"github.com/google/trillian/storage"
Expand Down Expand Up @@ -44,3 +48,105 @@ func TestPostgreSQLStorageProviderErrorPersistence(t *testing.T) {
t.Fatalf("Expected second call to 'storage.NewProvider' to fail with %q, instead got: %q", err1, err2)
}
}

func TestBuildPostgresTLSURINoTLS(t *testing.T) {
defer flagsaver.Save().MustRestore()
originalURI := "postgresql:///defaultdb?host=localhost&user=test"
if err := flag.Set("postgresql_tls_ca", ""); err != nil {
t.Fatalf("Failed to set flag: %v", err)
}

modified, err := BuildPostgresTLSURI(originalURI)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
if modified != originalURI {
t.Errorf("Expected unmodified URI %q, got %q", originalURI, modified)
}
}

func TestBuildPostgresTLSURIMissingCAFile(t *testing.T) {
defer flagsaver.Save().MustRestore()

if err := flag.Set("postgresql_tls_ca", "/path/does/not/exist.crt"); err != nil {
t.Fatalf("Failed to set flag: %v", err)
}

_, err := BuildPostgresTLSURI("postgresql:///defaultdb")
if err == nil {
t.Fatalf("Expected error due to missing CA file, got nil")
}
if !strings.Contains(err.Error(), "CA file error") {
t.Fatalf("Expected CA file error, got: %v", err)
}
}

func TestBuildPostgresTLSURIVerifyCA(t *testing.T) {
defer flagsaver.Save().MustRestore()

tmp := filepath.Join(t.TempDir(), "ca.pem")
if err := os.WriteFile(tmp, []byte("test_ca_cert_content"), 0400); err != nil {
t.Fatalf("Failed to write CA file: %v", err)
}

if err := flag.Set("postgresql_tls_ca", tmp); err != nil {
t.Fatalf("Failed to set CA flag: %v", err)
}
if err := flag.Set("postgresql_verify_full", "false"); err != nil {
t.Fatalf("Failed to set verify flag: %v", err)
}

original := "postgresql:///defaultdb?host=localhost&user=test"
modified, err := BuildPostgresTLSURI(original)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

u, err := url.Parse(modified)
if err != nil {
t.Fatalf("Failed to parse modified URI: %v", err)
}
q := u.Query()

if q.Get("sslrootcert") != tmp {
t.Errorf("Expected sslrootcert=%q, got %q", tmp, q.Get("sslrootcert"))
}
if q.Get("sslmode") != "verify-ca" {
t.Errorf("Expected sslmode=verify-ca, got %q", q.Get("sslmode"))
}
}

func TestBuildPostgresTLSURIVerifyFull(t *testing.T) {
defer flagsaver.Save().MustRestore()

tmp := filepath.Join(t.TempDir(), "ca.pem")
if err := os.WriteFile(tmp, []byte("test_ca_cert_content"), 0400); err != nil {
t.Fatalf("Failed to write CA file: %v", err)
}

if err := flag.Set("postgresql_tls_ca", tmp); err != nil {
t.Fatalf("Failed to set CA flag: %v", err)
}
if err := flag.Set("postgresql_verify_full", "true"); err != nil {
t.Fatalf("Failed to set verify_full flag: %v", err)
}

original := "postgresql:///defaultdb?host=localhost&user=test"
modified, err := BuildPostgresTLSURI(original)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

u, err := url.Parse(modified)
if err != nil {
t.Fatalf("Failed to parse modified URI: %v", err)
}
q := u.Query()

if q.Get("sslrootcert") != tmp {
t.Errorf("Expected sslrootcert=%q, got %q", tmp, q.Get("sslrootcert"))
}
if q.Get("sslmode") != "verify-full" {
t.Errorf("Expected sslmode=verify-full, got %q", q.Get("sslmode"))
}
}
Loading