Skip to content

Commit d3721cd

Browse files
committed
Document that PKI objects marshal for OpenSSL
PostgreSQL, Patroni, pgBackRest, and PgBouncer all use certificates through OpenSSL bindings. The format emitted by "MarshalText" is already compatible with OpenSSL, so document that and add tests to enforce it.
1 parent e6bce5a commit d3721cd

File tree

9 files changed

+126
-103
lines changed

9 files changed

+126
-103
lines changed

internal/patroni/certificates.go

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616
package patroni
1717

1818
import (
19-
corev1 "k8s.io/api/core/v1"
19+
"encoding"
2020

21-
"github.com/crunchydata/postgres-operator/internal/pki"
21+
corev1 "k8s.io/api/core/v1"
2222
)
2323

2424
const (
@@ -29,12 +29,12 @@ const (
2929
certServerFileKey = "patroni.crt-combined"
3030
)
3131

32-
// certAuthorities encodes roots in a format suitable for Patroni's TLS verification.
33-
func certAuthorities(roots ...pki.Certificate) ([]byte, error) {
32+
// certFile concatenates the results of multiple PEM-encoding marshalers.
33+
func certFile(texts ...encoding.TextMarshaler) ([]byte, error) {
3434
var out []byte
3535

36-
for i := range roots {
37-
if b, err := roots[i].MarshalText(); err == nil {
36+
for i := range texts {
37+
if b, err := texts[i].MarshalText(); err == nil {
3838
out = append(out, b...)
3939
} else {
4040
return nil, err
@@ -44,26 +44,6 @@ func certAuthorities(roots ...pki.Certificate) ([]byte, error) {
4444
return out, nil
4545
}
4646

47-
// certFile encodes cert and key as a combination suitable for
48-
// Patroni's TLS identification. It can be used by both the client and the server.
49-
func certFile(key pki.PrivateKey, cert pki.Certificate) ([]byte, error) {
50-
var out []byte
51-
52-
if b, err := key.MarshalText(); err == nil {
53-
out = append(out, b...)
54-
} else {
55-
return nil, err
56-
}
57-
58-
if b, err := cert.MarshalText(); err == nil {
59-
out = append(out, b...)
60-
} else {
61-
return nil, err
62-
}
63-
64-
return out, nil
65-
}
66-
6747
// instanceCertificates returns projections of Patroni's CAs, keys, and
6848
// certificates to include in the instance configuration volume.
6949
func instanceCertificates(certificates *corev1.Secret) []corev1.VolumeProjection {

internal/patroni/certificates_test.go

Lines changed: 11 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -16,62 +16,31 @@
1616
package patroni
1717

1818
import (
19+
"errors"
1920
"testing"
2021

2122
"gotest.tools/v3/assert"
2223
corev1 "k8s.io/api/core/v1"
2324

24-
"github.com/crunchydata/postgres-operator/internal/pki"
2525
"github.com/crunchydata/postgres-operator/internal/testing/cmp"
2626
)
2727

28-
const rootPEM = `-----BEGIN CERTIFICATE-----
29-
MIIBgTCCASigAwIBAgIRAO0NXdQ5ZtvI26doDvj9Dx8wCgYIKoZIzj0EAwMwHzEd
30-
MBsGA1UEAxMUcG9zdGdyZXMtb3BlcmF0b3ItY2EwHhcNMjEwMTI3MjEyNTU0WhcN
31-
MzEwMTI1MjIyNTU0WjAfMR0wGwYDVQQDExRwb3N0Z3Jlcy1vcGVyYXRvci1jYTBZ
32-
MBMGByqGSM49AgEGCCqGSM49AwEHA0IABL0xD8B6ZQHPscklofw2hpEN1F8h06Ys
33-
IRhK2xoy8ASkiKOkzXVs22R/Wnv/+jAMVf9rit0vhblZlvn2yP7e29WjRTBDMA4G
34-
A1UdDwEB/wQEAwIBBjASBgNVHRMBAf8ECDAGAQH/AgECMB0GA1UdDgQWBBQjfqdS
35-
Ynr3rFHMLd3fHO79tH3w5DAKBggqhkjOPQQDAwNHADBEAiA41LbQXeC0G/AyOHgs
36-
gaUp3fzHKSsrTGhzA8+dK2mnSgIgEKnv1FquJBJuXRBAxzrmnt0nJPiTWB926iNE
37-
BY8V4Ag=
38-
-----END CERTIFICATE-----`
28+
type funcMarshaler func() ([]byte, error)
3929

40-
func TestCertAuthorities(t *testing.T) {
41-
root := pki.Certificate{}
42-
assert.NilError(t, root.UnmarshalText([]byte(rootPEM)))
43-
44-
data, err := certAuthorities(root)
45-
assert.NilError(t, err)
46-
47-
// PEM-encoded certificates.
48-
assert.DeepEqual(t, string(data), rootPEM+"\n")
49-
}
30+
func (f funcMarshaler) MarshalText() ([]byte, error) { return f() }
5031

5132
func TestCertFile(t *testing.T) {
52-
root, err := pki.NewRootCertificateAuthority()
53-
assert.NilError(t, err)
54-
55-
instance, err := root.GenerateLeafCertificate("instance.pod-dns", nil)
56-
assert.NilError(t, err)
33+
expected := errors.New("boom")
34+
var short funcMarshaler = func() ([]byte, error) { return []byte(`one`), nil }
35+
var fail funcMarshaler = func() ([]byte, error) { return nil, expected }
5736

58-
data, err := certFile(instance.PrivateKey, instance.Certificate)
37+
text, err := certFile(short, short, short)
5938
assert.NilError(t, err)
39+
assert.DeepEqual(t, text, []byte(`oneoneone`))
6040

61-
// PEM-encoded key followed by the certificate
62-
// - https://docs.python.org/3/library/ssl.html#combined-key-and-certificate
63-
// - https://docs.python.org/3/library/ssl.html#certificate-chains
64-
assert.Assert(t,
65-
cmp.Regexp(`^`+
66-
`-----BEGIN [^ ]+ PRIVATE KEY-----\n`+
67-
`([^-]+\n)+`+
68-
`-----END [^ ]+ PRIVATE KEY-----\n`+
69-
`-----BEGIN CERTIFICATE-----\n`+
70-
`([^-]+\n)+`+
71-
`-----END CERTIFICATE-----\n`+
72-
`$`,
73-
string(data),
74-
))
41+
text, err = certFile(short, fail, short)
42+
assert.Equal(t, err, expected)
43+
assert.DeepEqual(t, text, []byte(nil))
7544
}
7645

7746
func TestInstanceCertificates(t *testing.T) {

internal/patroni/reconcile.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,10 @@ func InstanceCertificates(ctx context.Context,
8080
initialize.ByteMap(&outInstanceCertificates.Data)
8181

8282
var err error
83-
outInstanceCertificates.Data[certAuthorityFileKey], err =
84-
certAuthorities(inRoot)
83+
outInstanceCertificates.Data[certAuthorityFileKey], err = certFile(inRoot)
8584

8685
if err == nil {
87-
outInstanceCertificates.Data[certServerFileKey], err =
88-
certFile(inDNSKey, inDNS)
86+
outInstanceCertificates.Data[certServerFileKey], err = certFile(inDNSKey, inDNS)
8987
}
9088

9189
return err

internal/patroni/reconcile_test.go

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,22 +61,44 @@ func TestReconcileInstanceCertificates(t *testing.T) {
6161
leaf, err := root.GenerateLeafCertificate("any", nil)
6262
assert.NilError(t, err, "bug in test")
6363

64+
dataCA, _ := certFile(root.Certificate)
65+
assert.Assert(t,
66+
cmp.Regexp(`^`+
67+
`-----BEGIN CERTIFICATE-----\n`+
68+
`([^-]+\n)+`+
69+
`-----END CERTIFICATE-----\n`+
70+
`$`, string(dataCA),
71+
),
72+
"expected a PEM-encoded certificate bundle")
73+
74+
dataCert, _ := certFile(leaf.PrivateKey, leaf.Certificate)
75+
assert.Assert(t,
76+
cmp.Regexp(`^`+
77+
`-----BEGIN [^ ]+ PRIVATE KEY-----\n`+
78+
`([^-]+\n)+`+
79+
`-----END [^ ]+ PRIVATE KEY-----\n`+
80+
`-----BEGIN CERTIFICATE-----\n`+
81+
`([^-]+\n)+`+
82+
`-----END CERTIFICATE-----\n`+
83+
`$`, string(dataCert),
84+
),
85+
// - https://docs.python.org/3/library/ssl.html#combined-key-and-certificate
86+
// - https://docs.python.org/3/library/ssl.html#certificate-chains
87+
"expected a PEM-encoded key followed by the certificate")
88+
6489
ctx := context.Background()
6590
secret := new(corev1.Secret)
66-
cert := leaf.Certificate
67-
key := leaf.PrivateKey
68-
69-
dataCA, _ := certAuthorities(root.Certificate)
70-
dataCert, _ := certFile(key, cert)
7191

72-
assert.NilError(t, InstanceCertificates(ctx, root.Certificate, cert, key, secret))
92+
assert.NilError(t, InstanceCertificates(ctx,
93+
root.Certificate, leaf.Certificate, leaf.PrivateKey, secret))
7394

7495
assert.DeepEqual(t, secret.Data["patroni.ca-roots"], dataCA)
7596
assert.DeepEqual(t, secret.Data["patroni.crt-combined"], dataCert)
7697

7798
// No change when called again.
7899
before := secret.DeepCopy()
79-
assert.NilError(t, InstanceCertificates(ctx, root.Certificate, cert, key, secret))
100+
assert.NilError(t, InstanceCertificates(ctx,
101+
root.Certificate, leaf.Certificate, leaf.PrivateKey, secret))
80102
assert.DeepEqual(t, secret, before)
81103
}
82104

internal/pgbackrest/certificates.go

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,12 @@
1616
package pgbackrest
1717

1818
import (
19+
"encoding"
20+
1921
corev1 "k8s.io/api/core/v1"
2022
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2123

2224
"github.com/crunchydata/postgres-operator/internal/initialize"
23-
"github.com/crunchydata/postgres-operator/internal/pki"
2425
)
2526

2627
const (
@@ -47,13 +48,12 @@ const (
4748
certRepoSecretKey = "pgbackrest-repo-host.crt" // #nosec G101 this is a name, not a credential
4849
)
4950

50-
// certAuthorities returns the PEM encoding of roots that can be read by OpenSSL
51-
// for TLS verification.
52-
func certAuthorities(roots ...pki.Certificate) ([]byte, error) {
51+
// certFile concatenates the results of multiple PEM-encoding marshalers.
52+
func certFile(texts ...encoding.TextMarshaler) ([]byte, error) {
5353
var out []byte
5454

55-
for i := range roots {
56-
if b, err := roots[i].MarshalText(); err == nil {
55+
for i := range texts {
56+
if b, err := texts[i].MarshalText(); err == nil {
5757
out = append(out, b...)
5858
} else {
5959
return nil, err
@@ -63,18 +63,6 @@ func certAuthorities(roots ...pki.Certificate) ([]byte, error) {
6363
return out, nil
6464
}
6565

66-
// certFile returns the PEM encoding of cert that can be read by OpenSSL
67-
// for TLS identification.
68-
func certFile(cert pki.Certificate) ([]byte, error) {
69-
return cert.MarshalText()
70-
}
71-
72-
// certPrivateKey returns the PEM encoding of key that can be read by OpenSSL
73-
// for TLS identification.
74-
func certPrivateKey(key pki.PrivateKey) ([]byte, error) {
75-
return key.MarshalText()
76-
}
77-
7866
// clientCertificates returns projections of CAs, keys, and certificates to
7967
// include in a configuration volume from the pgBackRest Secret.
8068
func clientCertificates() []corev1.KeyToPath {

internal/pgbackrest/certificates_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package pgbackrest
1717

1818
import (
19+
"errors"
1920
"strings"
2021
"testing"
2122

@@ -26,6 +27,24 @@ import (
2627
"github.com/crunchydata/postgres-operator/internal/testing/cmp"
2728
)
2829

30+
type funcMarshaler func() ([]byte, error)
31+
32+
func (f funcMarshaler) MarshalText() ([]byte, error) { return f() }
33+
34+
func TestCertFile(t *testing.T) {
35+
expected := errors.New("boom")
36+
var short funcMarshaler = func() ([]byte, error) { return []byte(`one`), nil }
37+
var fail funcMarshaler = func() ([]byte, error) { return nil, expected }
38+
39+
text, err := certFile(short, short, short)
40+
assert.NilError(t, err)
41+
assert.DeepEqual(t, text, []byte(`oneoneone`))
42+
43+
text, err = certFile(short, fail, short)
44+
assert.Equal(t, err, expected)
45+
assert.DeepEqual(t, text, []byte(nil))
46+
}
47+
2948
func TestClientCommonName(t *testing.T) {
3049
t.Parallel()
3150

internal/pgbackrest/reconcile.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ func InstanceCertificates(ctx context.Context,
390390
outInstanceCertificates.Data[certInstanceSecretKey], err = certFile(inDNS)
391391
}
392392
if err == nil {
393-
outInstanceCertificates.Data[certInstancePrivateKeySecretKey], err = certPrivateKey(inDNSKey)
393+
outInstanceCertificates.Data[certInstancePrivateKeySecretKey], err = certFile(inDNSKey)
394394
}
395395
}
396396

@@ -510,10 +510,10 @@ func Secret(ctx context.Context,
510510
}
511511

512512
if err == nil {
513-
outSecret.Data[certAuthoritySecretKey], err = certAuthorities(inRoot.Certificate)
513+
outSecret.Data[certAuthoritySecretKey], err = certFile(inRoot.Certificate)
514514
}
515515
if err == nil {
516-
outSecret.Data[certClientPrivateKeySecretKey], err = certPrivateKey(leaf.PrivateKey)
516+
outSecret.Data[certClientPrivateKeySecretKey], err = certFile(leaf.PrivateKey)
517517
}
518518
if err == nil {
519519
outSecret.Data[certClientSecretKey], err = certFile(leaf.Certificate)
@@ -537,7 +537,7 @@ func Secret(ctx context.Context,
537537
}
538538

539539
if err == nil {
540-
outSecret.Data[certRepoPrivateKeySecretKey], err = certPrivateKey(leaf.PrivateKey)
540+
outSecret.Data[certRepoPrivateKeySecretKey], err = certFile(leaf.PrivateKey)
541541
}
542542
if err == nil {
543543
outSecret.Data[certRepoSecretKey], err = certFile(leaf.Certificate)

internal/pki/encoding.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ var (
3939
_ encoding.TextUnmarshaler = (*Certificate)(nil)
4040
)
4141

42-
// MarshalText returns a PEM encoding of c.
42+
// MarshalText returns a PEM encoding of c that OpenSSL understands.
4343
func (c Certificate) MarshalText() ([]byte, error) {
4444
if c.x509 == nil || len(c.x509.Raw) == 0 {
4545
_, err := x509.ParseCertificate(nil)
@@ -73,7 +73,7 @@ var (
7373
_ encoding.TextUnmarshaler = (*PrivateKey)(nil)
7474
)
7575

76-
// MarshalText returns a PEM encoding of k.
76+
// MarshalText returns a PEM encoding of k that OpenSSL understands.
7777
func (k PrivateKey) MarshalText() ([]byte, error) {
7878
if k.ecdsa == nil {
7979
k.ecdsa = new(ecdsa.PrivateKey)

internal/pki/encoding_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,14 @@ package pki
1717

1818
import (
1919
"bytes"
20+
"os"
21+
"os/exec"
22+
"path/filepath"
2023
"testing"
2124

2225
"gotest.tools/v3/assert"
26+
27+
"github.com/crunchydata/postgres-operator/internal/testing/require"
2328
)
2429

2530
func TestCertificateTextMarshaling(t *testing.T) {
@@ -75,6 +80,23 @@ func TestCertificateTextMarshaling(t *testing.T) {
7580
var sink Certificate
7681
assert.ErrorContains(t, sink.UnmarshalText(txt), "malformed")
7782
})
83+
84+
t.Run("ReadByOpenSSL", func(t *testing.T) {
85+
openssl := require.OpenSSL(t)
86+
dir := t.TempDir()
87+
88+
certFile := filepath.Join(dir, "cert.pem")
89+
certBytes, err := cert.MarshalText()
90+
assert.NilError(t, err)
91+
assert.NilError(t, os.WriteFile(certFile, certBytes, 0o600))
92+
93+
// The "openssl x509" command parses X.509 certificates.
94+
cmd := exec.Command(openssl, "x509",
95+
"-in", certFile, "-inform", "PEM", "-noout", "-text")
96+
97+
output, err := cmd.CombinedOutput()
98+
assert.NilError(t, err, "%q\n%s", cmd.Args, output)
99+
})
78100
}
79101

80102
func TestPrivateKeyTextMarshaling(t *testing.T) {
@@ -130,4 +152,29 @@ func TestPrivateKeyTextMarshaling(t *testing.T) {
130152
var sink PrivateKey
131153
assert.ErrorContains(t, sink.UnmarshalText(txt), "asn1")
132154
})
155+
156+
t.Run("ReadByOpenSSL", func(t *testing.T) {
157+
openssl := require.OpenSSL(t)
158+
dir := t.TempDir()
159+
160+
keyFile := filepath.Join(dir, "key.pem")
161+
keyBytes, err := key.MarshalText()
162+
assert.NilError(t, err)
163+
assert.NilError(t, os.WriteFile(keyFile, keyBytes, 0o600))
164+
165+
// The "openssl pkey" command processes public and private keys.
166+
cmd := exec.Command(openssl, "pkey",
167+
"-check", "-in", keyFile, "-inform", "PEM", "-noout", "-text")
168+
169+
output, err := cmd.CombinedOutput()
170+
assert.NilError(t, err, "%q\n%s", cmd.Args, output)
171+
172+
assert.Assert(t,
173+
bytes.Contains(output, []byte("is valid")),
174+
"expected valid private key, got:\n%s", output)
175+
176+
assert.Assert(t,
177+
bytes.Contains(output, []byte("\nPrivate-Key:")),
178+
"expected valid private key, got:\n%s", output)
179+
})
133180
}

0 commit comments

Comments
 (0)