Skip to content

Commit 478d282

Browse files
vlasebianclaude
andcommitted
all: Validate root CA certificates are CA certs
Reject non-CA (leaf) certificates configured as root CAs to prevent misconfiguration where a server leaf certificate is set as a root CA. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 8290414 commit 478d282

2 files changed

Lines changed: 166 additions & 4 deletions

File tree

pkg/config/tlsconfig/config.go

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,11 @@
1616
package tlsconfig
1717

1818
import (
19+
"bytes"
1920
"context"
2021
"crypto/tls"
2122
"crypto/x509"
23+
"encoding/pem"
2224
"os"
2325
"strings"
2426
"sync/atomic"
@@ -46,6 +48,14 @@ var (
4648
errMissingACMEDir = errors.Define("missing_acme_dir", "missing ACME storage directory")
4749
errMissingACMEEndpoint = errors.Define("missing_acme_endpoint", "missing ACME endpoint")
4850
errMissingACMEDefaultHost = errors.Define("missing_acme_default_host", "missing ACME default host")
51+
errParsePEM = errors.DefineInvalidArgument("parse_pem", "parse PEM")
52+
errUnexpectedPEMType = errors.DefineInvalidArgument(
53+
"unexpected_pem_type", "unexpected PEM block of type `{pem_type}`, expected CERTIFICATE",
54+
)
55+
errParseCertificate = errors.DefineInvalidArgument("parse_certificate", "parse certificate")
56+
errNotCACertificate = errors.DefineInvalidArgument(
57+
"not_ca_certificate", "certificate with subject `{subject}` is not a CA certificate",
58+
)
4959
)
5060

5161
// Initialize initializes the autocert manager for the ACME configuration.
@@ -190,17 +200,53 @@ func (c Client) ApplyTo(tlsConfig *tls.Config) error {
190200
}
191201

192202
if len(rootCABytes) > 0 {
203+
certs, err := parseAndValidateCACerts(rootCABytes)
204+
if err != nil {
205+
return err
206+
}
207+
193208
if tlsConfig.RootCAs == nil {
194209
if tlsConfig.RootCAs, err = x509.SystemCertPool(); err != nil {
195210
tlsConfig.RootCAs = x509.NewCertPool()
196211
}
197212
}
198-
tlsConfig.RootCAs.AppendCertsFromPEM(rootCABytes)
213+
214+
for _, cert := range certs {
215+
tlsConfig.RootCAs.AddCert(cert)
216+
}
199217
}
200218
tlsConfig.InsecureSkipVerify = c.InsecureSkipVerify
201219
return nil
202220
}
203221

222+
func parseAndValidateCACerts(pemBytes []byte) ([]*x509.Certificate, error) {
223+
var certs []*x509.Certificate
224+
for {
225+
pemBytes = bytes.TrimSpace(pemBytes)
226+
if len(pemBytes) == 0 {
227+
break
228+
}
229+
block, rest := pem.Decode(pemBytes)
230+
if block == nil {
231+
return nil, errParsePEM.New()
232+
}
233+
if block.Type != "CERTIFICATE" {
234+
return nil, errUnexpectedPEMType.WithAttributes("pem_type", block.Type)
235+
}
236+
pemBytes = rest
237+
238+
cert, err := x509.ParseCertificate(block.Bytes)
239+
if err != nil {
240+
return nil, errParseCertificate.WithCause(err)
241+
}
242+
if !cert.IsCA {
243+
return nil, errNotCACertificate.WithAttributes("subject", cert.Subject.String())
244+
}
245+
certs = append(certs, cert)
246+
}
247+
return certs, nil
248+
}
249+
204250
func readCert(fileReader FileReader, certFile, keyFile string) (*tls.Certificate, error) {
205251
readFile := os.ReadFile
206252
if fileReader != nil {

pkg/config/tlsconfig/config_test.go

Lines changed: 119 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func (m mockFileReader) ReadFile(name string) ([]byte, error) {
4242
return nil, fmt.Errorf("not found")
4343
}
4444

45-
func genCert() (certPEM []byte, keyPEM []byte) {
45+
func genCertWithOptions(isCA bool) (certPEM []byte, keyPEM []byte) {
4646
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
4747
if err != nil {
4848
panic(err)
@@ -60,10 +60,13 @@ func genCert() (certPEM []byte, keyPEM []byte) {
6060
},
6161
NotBefore: now,
6262
NotAfter: now.Add(time.Hour),
63-
KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
63+
KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
6464
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
6565
BasicConstraintsValid: true,
66-
IsCA: true,
66+
IsCA: isCA,
67+
}
68+
if isCA {
69+
cert.KeyUsage |= x509.KeyUsageCertSign
6770
}
6871
certBytes, err := x509.CreateCertificate(rand.Reader, cert, cert, key.Public(), key)
6972
if err != nil {
@@ -80,6 +83,10 @@ func genCert() (certPEM []byte, keyPEM []byte) {
8083
})
8184
}
8285

86+
func genCert() (certPEM []byte, keyPEM []byte) {
87+
return genCertWithOptions(true)
88+
}
89+
8390
func TestApplyTLSClientConfig(t *testing.T) {
8491
t.Parallel()
8592
a := assertions.New(t)
@@ -107,6 +114,115 @@ func TestApplyTLSClientConfig(t *testing.T) {
107114
})
108115
}
109116

117+
func TestApplyTLSClientConfigRejectsLeafCert(t *testing.T) {
118+
t.Parallel()
119+
leafCert, _ := genCertWithOptions(false)
120+
caCert, _ := genCertWithOptions(true)
121+
122+
t.Run("LeafCertificate", func(t *testing.T) {
123+
t.Parallel()
124+
a := assertions.New(t)
125+
tlsConfig := &tls.Config{} //nolint:gosec
126+
err := (&tlsconfig.Client{
127+
FileReader: mockFileReader{
128+
"leaf.pem": leafCert,
129+
},
130+
RootCA: "leaf.pem",
131+
}).ApplyTo(tlsConfig)
132+
a.So(err, should.NotBeNil)
133+
a.So(err.Error(), should.ContainSubstring, "not a CA certificate")
134+
})
135+
136+
t.Run("CACertificate", func(t *testing.T) {
137+
t.Parallel()
138+
a := assertions.New(t)
139+
tlsConfig := &tls.Config{} //nolint:gosec
140+
err := (&tlsconfig.Client{
141+
FileReader: mockFileReader{
142+
"ca.pem": caCert,
143+
},
144+
RootCA: "ca.pem",
145+
}).ApplyTo(tlsConfig)
146+
a.So(err, should.BeNil)
147+
a.So(tlsConfig.RootCAs, should.NotBeNil)
148+
})
149+
150+
t.Run("MultipleCertsOneLeaf", func(t *testing.T) {
151+
t.Parallel()
152+
a := assertions.New(t)
153+
combined := append(caCert, leafCert...)
154+
tlsConfig := &tls.Config{} //nolint:gosec
155+
err := (&tlsconfig.Client{
156+
FileReader: mockFileReader{
157+
"mixed.pem": combined,
158+
},
159+
RootCA: "mixed.pem",
160+
}).ApplyTo(tlsConfig)
161+
a.So(err, should.NotBeNil)
162+
a.So(err.Error(), should.ContainSubstring, "not a CA certificate")
163+
})
164+
165+
t.Run("MultipleCACertificates", func(t *testing.T) {
166+
t.Parallel()
167+
a := assertions.New(t)
168+
caCert2, _ := genCertWithOptions(true)
169+
combined := append(caCert, caCert2...)
170+
tlsConfig := &tls.Config{} //nolint:gosec
171+
err := (&tlsconfig.Client{
172+
FileReader: mockFileReader{
173+
"cas.pem": combined,
174+
},
175+
RootCA: "cas.pem",
176+
}).ApplyTo(tlsConfig)
177+
a.So(err, should.BeNil)
178+
a.So(tlsConfig.RootCAs, should.NotBeNil)
179+
})
180+
181+
t.Run("InvalidPEM", func(t *testing.T) {
182+
t.Parallel()
183+
a := assertions.New(t)
184+
tlsConfig := &tls.Config{} //nolint:gosec
185+
err := (&tlsconfig.Client{
186+
FileReader: mockFileReader{
187+
"bad.pem": []byte("not a pem"),
188+
},
189+
RootCA: "bad.pem",
190+
}).ApplyTo(tlsConfig)
191+
a.So(err, should.NotBeNil)
192+
a.So(err.Error(), should.ContainSubstring, "parse PEM")
193+
})
194+
195+
t.Run("NonCertificatePEMBlock", func(t *testing.T) {
196+
t.Parallel()
197+
a := assertions.New(t)
198+
_, keyPEM := genCertWithOptions(true)
199+
tlsConfig := &tls.Config{} //nolint:gosec
200+
err := (&tlsconfig.Client{
201+
FileReader: mockFileReader{
202+
"key.pem": keyPEM,
203+
},
204+
RootCA: "key.pem",
205+
}).ApplyTo(tlsConfig)
206+
a.So(err, should.NotBeNil)
207+
a.So(err.Error(), should.ContainSubstring, "unexpected PEM block")
208+
})
209+
210+
t.Run("TrailingNewlines", func(t *testing.T) {
211+
t.Parallel()
212+
a := assertions.New(t)
213+
withNewlines := append(caCert, '\n', '\n', '\n')
214+
tlsConfig := &tls.Config{} //nolint:gosec
215+
err := (&tlsconfig.Client{
216+
FileReader: mockFileReader{
217+
"ca.pem": withNewlines,
218+
},
219+
RootCA: "ca.pem",
220+
}).ApplyTo(tlsConfig)
221+
a.So(err, should.BeNil)
222+
a.So(tlsConfig.RootCAs, should.NotBeNil)
223+
})
224+
}
225+
110226
func TestApplyTLSServerAuth(t *testing.T) {
111227
t.Parallel()
112228
a := assertions.New(t)

0 commit comments

Comments
 (0)