Skip to content

Commit 95de029

Browse files
CBL-Mariner-Botkevin-b-lockwood
CBL-Mariner-Bot
and
kevin-b-lockwood
authored
[AUTO-CHERRYPICK] cert-manager: [Medium] patch for CVE-2024-12401 - branch main (#12028)
Co-authored-by: Kevin Lockwood <57274670+kevin-b-lockwood@users.noreply.github.com>
1 parent 0ccbd00 commit 95de029

2 files changed

Lines changed: 370 additions & 3 deletions

File tree

‎SPECS/cert-manager/CVE-2024-12401.patch‎

Lines changed: 363 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,363 @@
1+
From 776bf7178df5f40d1c64c9691aec8186fc493fe0 Mon Sep 17 00:00:00 2001
2+
From: Kevin Lockwood <v-klockwood@microsoft.com>
3+
Date: Tue, 21 Jan 2025 14:13:08 -0800
4+
Subject: [PATCH] [Medium] fix CVE-2024-12401
5+
6+
Link: https://github.com/cert-manager/cert-manager/pull/7403
7+
---
8+
internal/controller/certificates/secrets.go | 10 +-
9+
internal/pem/decode.go | 125 ++++++++++++++++++++
10+
pkg/controller/acmeorders/sync.go | 20 ++--
11+
pkg/util/pki/csr.go | 13 +-
12+
pkg/util/pki/parse.go | 29 +++--
13+
5 files changed, 170 insertions(+), 27 deletions(-)
14+
create mode 100644 internal/pem/decode.go
15+
16+
diff --git a/internal/controller/certificates/secrets.go b/internal/controller/certificates/secrets.go
17+
index 0a401c2..f2f1852 100644
18+
--- a/internal/controller/certificates/secrets.go
19+
+++ b/internal/controller/certificates/secrets.go
20+
@@ -19,9 +19,9 @@ package certificates
21+
import (
22+
"bytes"
23+
"crypto/x509"
24+
- "encoding/pem"
25+
"strings"
26+
27+
+ "github.com/cert-manager/cert-manager/internal/pem"
28+
apiutil "github.com/cert-manager/cert-manager/pkg/api/util"
29+
cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
30+
utilpki "github.com/cert-manager/cert-manager/pkg/util/pki"
31+
@@ -54,7 +54,13 @@ func AnnotationsForCertificateSecret(crt *cmapi.Certificate, certificate *x509.C
32+
// OutputFormatDER returns the byte slice of the private key in DER format. To
33+
// be used for Certificate's Additional Output Format DER.
34+
func OutputFormatDER(privateKey []byte) []byte {
35+
- block, _ := pem.Decode(privateKey)
36+
+ // NOTE: This call to pem.SafeDecodePrivateKey ignores errors.
37+
+ // This is acceptable here since we're calling this function only on PEM data which we created
38+
+ // by encoding the private key. As such, we can be fairly confident that:
39+
+ // 1) The PEM is valid
40+
+ // 2) The PEM isn't attacker-controlled (and as such unsafe to decode)
41+
+
42+
+ block, _, _ := pem.SafeDecodePrivateKey(privateKey)
43+
return block.Bytes
44+
}
45+
46+
diff --git a/internal/pem/decode.go b/internal/pem/decode.go
47+
new file mode 100644
48+
index 0000000..1042f98
49+
--- /dev/null
50+
+++ b/internal/pem/decode.go
51+
@@ -0,0 +1,125 @@
52+
+/*
53+
+Copyright 2024 The cert-manager Authors.
54+
+
55+
+Licensed under the Apache License, Version 2.0 (the "License");
56+
+you may not use this file except in compliance with the License.
57+
+You may obtain a copy of the License at
58+
+
59+
+ http://www.apache.org/licenses/LICENSE-2.0
60+
+
61+
+Unless required by applicable law or agreed to in writing, software
62+
+distributed under the License is distributed on an "AS IS" BASIS,
63+
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
64+
+See the License for the specific language governing permissions and
65+
+limitations under the License.
66+
+*/
67+
+
68+
+// Package pem provides utility functions for safely decoding PEM data, placing upper limits on the size
69+
+// of data that will be processed. It functions as an extension to the standard library "encoding/pem" functions.
70+
+package pem
71+
+
72+
+import (
73+
+ stdpem "encoding/pem"
74+
+ "errors"
75+
+ "fmt"
76+
+)
77+
+
78+
+// The constants below are estimates at reasonable upper bounds for sizes of PEM data that cert-manager might encounter.
79+
+// cert-manager supports RSA, ECDSA and Ed25519 keys, of which RSA keys are by far the largest.
80+
+
81+
+// We'll aim to support RSA certs / keys which are larger than the maximum size (defined in pkg/util/pki.MaxRSAKeySize).
82+
+
83+
+// RSA keys grow proportional to the size of the RSA key used. For example:
84+
+// PEM-encoded RSA Keys: 4096-bit is ~3kB, 8192-bit is ~6kB and a 16k-bit key is ~12kB.
85+
+
86+
+// Certificates have two variables; the public key of the cert, and the signature from the signing cert.
87+
+// An N-bit key produces an N-byte signature, so as a worst case for us, a 16kB RSA key will create a 2kB signature.
88+
+
89+
+// PEM-encoded RSA X.509 certificates:
90+
+// Signed with 1k-bit RSA key: 4096-bit is ~1.4kB, 8192-bit is ~2kB, 16k-bit is ~3.5kB
91+
+// Signed with 16k-bit RSA key: 4096-bit is ~3.3kB, 8192-bit is ~4kB, 16k-bit is ~5.4kB
92+
+
93+
+// See https://fm4dd.com/openssl/certexamples.shtm for examples of large RSA certs / keys
94+
+
95+
+const (
96+
+ // maxCertificatePEMSize is the maximum size, in bytes, of a single PEM-encoded X.509 certificate which SafeDecodeSingleCertificate will accept.
97+
+ // The value is based on how large a "realistic" (but still very large) self-signed 16k-bit RSA certficate might be.
98+
+ // 16k-bit RSA keys are impractical on most on modern hardware due to how slow they can be,
99+
+ // so we can reasonably assume that no real-world PEM-encoded X.509 cert will be this large.
100+
+ // Note that X.509 certificates can contain extra abitrary data (e.g. DNS names, policy names, etc) whose size is hard to predict.
101+
+ // So we guess at how much of that data we'll allow in very large certs and allow about 1kB of such data.
102+
+ maxCertificatePEMSize = 6500
103+
+
104+
+ // maxPrivateKeyPEMSize is the maximum size, in bytes, of PEM-encoded private keys which SafeDecodePrivateKey will accept.
105+
+ // cert-manager supports RSA, ECDSA and Ed25519 keys, of which RSA is by far the largest.
106+
+ // The value is based on how large a "realistic" (but very large) 16k-bit RSA private key might be.
107+
+ // Given that 16k-bit RSA keys are so slow to use as to be impractical on modern hardware,
108+
+ // we can reasonably assume that no real-world PEM-encoded key will be this large.
109+
+ maxPrivateKeyPEMSize = 13000
110+
+
111+
+ // maxChainSize is the maximum number of 16k-bit RSA certificates signed by 16k-bit RSA CAs we'll allow in a given call to SafeDecodeMultipleCertificates.
112+
+ // This is _not_ the maximum number of certificates cert-manager will process in a given chain, which could be much larger.
113+
+ // This is simply the maximum number of worst-case certificates we'll accept in a chain.
114+
+ maxChainSize = 10
115+
+)
116+
+
117+
+var (
118+
+ // ErrNoPEMData is returned when the given data contained no PEM
119+
+ ErrNoPEMData = errors.New("no PEM data was found in given input")
120+
+)
121+
+
122+
+// ErrPEMDataTooLarge is returned when the given data is larger than the maximum allowed
123+
+type ErrPEMDataTooLarge int
124+
+
125+
+// Error returns an error string
126+
+func (e ErrPEMDataTooLarge) Error() string {
127+
+ return fmt.Sprintf("provided PEM data was larger than the maximum %dB", int(e))
128+
+}
129+
+
130+
+func safeDecodeInternal(b []byte, maxSize int) (*stdpem.Block, []byte, error) {
131+
+ if len(b) > maxSize {
132+
+ return nil, b, ErrPEMDataTooLarge(maxSize)
133+
+ }
134+
+
135+
+ block, rest := stdpem.Decode(b)
136+
+ if block == nil {
137+
+ return nil, rest, ErrNoPEMData
138+
+ }
139+
+
140+
+ return block, rest, nil
141+
+}
142+
+
143+
+// SafeDecodePrivateKey calls [encoding/pem.Decode] on the given input as long as it's within a sensible range for
144+
+// how large we expect a private key to be. The baseline is a 16k-bit RSA private key, which is larger than the maximum
145+
+// supported by cert-manager for key generation.
146+
+func SafeDecodePrivateKey(b []byte) (*stdpem.Block, []byte, error) {
147+
+ return safeDecodeInternal(b, maxPrivateKeyPEMSize)
148+
+}
149+
+
150+
+// SafeDecodeCSR calls [encoding/pem.Decode] on the given input as long as it's within a sensible range for
151+
+// how large we expect a single PEM-encoded PKCS#10 CSR to be.
152+
+// We assume that a PKCS#12 CSR is smaller than a single certificate because our assumptions are that
153+
+// a certificate has a large public key and a large signature, which is roughly the case for a CSR.
154+
+// We also assume that we'd only ever decode one CSR which is the case in practice.
155+
+func SafeDecodeCSR(b []byte) (*stdpem.Block, []byte, error) {
156+
+ return safeDecodeInternal(b, maxCertificatePEMSize)
157+
+}
158+
+
159+
+// SafeDecodeSingleCertificate calls [encoding/pem.Decode] on the given input as long as it's within a sensible range for
160+
+// how large we expect a single PEM-encoded X.509 certificate to be.
161+
+// The baseline is a 16k-bit RSA certificate signed by a different 16k-bit RSA CA, which is larger than the maximum
162+
+// supported by cert-manager for key generation.
163+
+func SafeDecodeSingleCertificate(b []byte) (*stdpem.Block, []byte, error) {
164+
+ return safeDecodeInternal(b, maxCertificatePEMSize)
165+
+}
166+
+
167+
+// SafeDecodeMultipleCertificates calls [encoding/pem.Decode] on the given input as long as it's within a sensible range for
168+
+// how large we expect a reasonable-length PEM-encoded X.509 certificate chain to be.
169+
+// The baseline is several 16k-bit RSA certificates, all signed by 16k-bit RSA keys, which is larger than the maximum
170+
+// supported by cert-manager for key generation.
171+
+// The maximum number of chains supported by this function is not reflective of the maximum chain length supported by
172+
+// cert-manager; a larger chain of smaller certificate should be supported.
173+
+func SafeDecodeMultipleCertificates(b []byte) (*stdpem.Block, []byte, error) {
174+
+ return safeDecodeInternal(b, maxCertificatePEMSize*maxChainSize)
175+
+}
176+
+
177+
diff --git a/pkg/controller/acmeorders/sync.go b/pkg/controller/acmeorders/sync.go
178+
index 6cde88a..0557174 100644
179+
--- a/pkg/controller/acmeorders/sync.go
180+
+++ b/pkg/controller/acmeorders/sync.go
181+
@@ -20,7 +20,7 @@ import (
182+
"bytes"
183+
"context"
184+
"crypto/x509"
185+
- "encoding/pem"
186+
+ stdpem "encoding/pem"
187+
"fmt"
188+
"time"
189+
190+
@@ -36,6 +36,7 @@ import (
191+
192+
"github.com/cert-manager/cert-manager/internal/controller/feature"
193+
internalorders "github.com/cert-manager/cert-manager/internal/controller/orders"
194+
+ "github.com/cert-manager/cert-manager/internal/pem"
195+
"github.com/cert-manager/cert-manager/pkg/acme"
196+
acmecl "github.com/cert-manager/cert-manager/pkg/acme/client"
197+
cmacme "github.com/cert-manager/cert-manager/pkg/apis/acme/v1"
198+
@@ -498,13 +499,18 @@ func (c *controller) finalizeOrder(ctx context.Context, cl acmecl.Interface, o *
199+
// only supported DER encoded CSRs and not PEM encoded as they are intended
200+
// to be as part of our API.
201+
// To work around this, we first attempt to decode the Request into DER bytes
202+
- // by running pem.Decode. If the PEM block is empty, we assume that the Request
203+
+ // by running pem.SafeDecodeCSR. If the PEM block is empty, we assume that the Request
204+
// is DER encoded and continue to call FinalizeOrder.
205+
var derBytes []byte
206+
- block, _ := pem.Decode(o.Spec.Request)
207+
- if block == nil {
208+
- log.V(logf.WarnLevel).Info("failed to parse Request as PEM data, attempting to treat Request as DER encoded for compatibility reasons")
209+
- derBytes = o.Spec.Request
210+
+ block, _, err := pem.SafeDecodeCSR(o.Spec.Request)
211+
+
212+
+ if err != nil {
213+
+ if err == pem.ErrNoPEMData {
214+
+ log.V(logf.WarnLevel).Info("failed to parse Request as PEM data, attempting to treat Request as DER encoded for compatibility reasons")
215+
+ derBytes = o.Spec.Request
216+
+ } else {
217+
+ return err
218+
+ }
219+
} else {
220+
derBytes = block.Bytes
221+
}
222+
@@ -589,7 +595,7 @@ func (c *controller) storeCertificateOnStatus(ctx context.Context, o *cmacme.Ord
223+
// encode the retrieved certificates (including the chain)
224+
certBuffer := bytes.NewBuffer([]byte{})
225+
for _, cert := range certs {
226+
- err := pem.Encode(certBuffer, &pem.Block{Type: "CERTIFICATE", Bytes: cert})
227+
+ err := stdpem.Encode(certBuffer, &stdpem.Block{Type: "CERTIFICATE", Bytes: cert})
228+
if err != nil {
229+
log.Error(err, "invalid certificate data returned by ACME server")
230+
c.setOrderState(&o.Status, string(cmacme.Errored))
231+
diff --git a/pkg/util/pki/csr.go b/pkg/util/pki/csr.go
232+
index bb4776f..b2173d5 100644
233+
--- a/pkg/util/pki/csr.go
234+
+++ b/pkg/util/pki/csr.go
235+
@@ -23,7 +23,7 @@ import (
236+
"crypto/x509"
237+
"crypto/x509/pkix"
238+
"encoding/asn1"
239+
- "encoding/pem"
240+
+ stdpem "encoding/pem"
241+
"errors"
242+
"fmt"
243+
"math/big"
244+
@@ -33,6 +33,7 @@ import (
245+
"time"
246+
247+
"github.com/cert-manager/cert-manager/internal/controller/feature"
248+
+ "github.com/cert-manager/cert-manager/internal/pem"
249+
apiutil "github.com/cert-manager/cert-manager/pkg/api/util"
250+
v1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
251+
utilfeature "github.com/cert-manager/cert-manager/pkg/util/feature"
252+
@@ -449,8 +450,8 @@ func GenerateTemplateFromCSRPEM(csrPEM []byte, duration time.Duration, isCA bool
253+
}
254+
255+
func GenerateTemplateFromCSRPEMWithUsages(csrPEM []byte, duration time.Duration, isCA bool, keyUsage x509.KeyUsage, extKeyUsage []x509.ExtKeyUsage) (*x509.Certificate, error) {
256+
- block, _ := pem.Decode(csrPEM)
257+
- if block == nil {
258+
+ block, _, err := pem.SafeDecodeCSR(csrPEM)
259+
+ if err != nil {
260+
return nil, errors.New("failed to decode csr")
261+
}
262+
263+
@@ -512,7 +513,7 @@ func SignCertificate(template *x509.Certificate, issuerCert *x509.Certificate, p
264+
}
265+
266+
pemBytes := bytes.NewBuffer([]byte{})
267+
- err = pem.Encode(pemBytes, &pem.Block{Type: "CERTIFICATE", Bytes: derBytes})
268+
+ err = stdpem.Encode(pemBytes, &stdpem.Block{Type: "CERTIFICATE", Bytes: derBytes})
269+
if err != nil {
270+
return nil, nil, fmt.Errorf("error encoding certificate PEM: %s", err.Error())
271+
}
272+
@@ -558,7 +559,7 @@ func EncodeCSR(template *x509.CertificateRequest, key crypto.Signer) ([]byte, er
273+
// EncodeX509 will encode a single *x509.Certificate into PEM format.
274+
func EncodeX509(cert *x509.Certificate) ([]byte, error) {
275+
caPem := bytes.NewBuffer([]byte{})
276+
- err := pem.Encode(caPem, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw})
277+
+ err := stdpem.Encode(caPem, &stdpem.Block{Type: "CERTIFICATE", Bytes: cert.Raw})
278+
if err != nil {
279+
return nil, err
280+
}
281+
@@ -584,7 +585,7 @@ func EncodeX509Chain(certs []*x509.Certificate) ([]byte, error) {
282+
continue
283+
}
284+
285+
- err := pem.Encode(caPem, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw})
286+
+ err := stdpem.Encode(caPem, &stdpem.Block{Type: "CERTIFICATE", Bytes: cert.Raw})
287+
if err != nil {
288+
return nil, err
289+
}
290+
diff --git a/pkg/util/pki/parse.go b/pkg/util/pki/parse.go
291+
index dff3539..428b2e8 100644
292+
--- a/pkg/util/pki/parse.go
293+
+++ b/pkg/util/pki/parse.go
294+
@@ -22,8 +22,9 @@ import (
295+
"crypto/x509"
296+
"crypto/x509/pkix"
297+
"encoding/asn1"
298+
- "encoding/pem"
299+
+ stdpem "encoding/pem"
300+
301+
+ "github.com/cert-manager/cert-manager/internal/pem"
302+
"github.com/cert-manager/cert-manager/pkg/util/errors"
303+
"github.com/go-ldap/ldap/v3"
304+
)
305+
@@ -32,8 +33,8 @@ import (
306+
// It supports ECDSA and RSA private keys only. All other types will return err.
307+
func DecodePrivateKeyBytes(keyBytes []byte) (crypto.Signer, error) {
308+
// decode the private key pem
309+
- block, _ := pem.Decode(keyBytes)
310+
- if block == nil {
311+
+ block, _, err := pem.SafeDecodePrivateKey(keyBytes)
312+
+ if err != nil {
313+
return nil, errors.NewInvalidData("error decoding private key PEM block")
314+
}
315+
316+
@@ -75,8 +76,8 @@ func DecodePrivateKeyBytes(keyBytes []byte) (crypto.Signer, error) {
317+
// DecodePKCS1PrivateKeyBytes will decode a PEM encoded RSA private key.
318+
func DecodePKCS1PrivateKeyBytes(keyBytes []byte) (*rsa.PrivateKey, error) {
319+
// decode the private key pem
320+
- block, _ := pem.Decode(keyBytes)
321+
- if block == nil {
322+
+ block, _, err := pem.SafeDecodePrivateKey(keyBytes)
323+
+ if err != nil {
324+
return nil, errors.NewInvalidData("error decoding private key PEM block")
325+
}
326+
// parse the private key
327+
@@ -95,13 +96,17 @@ func DecodePKCS1PrivateKeyBytes(keyBytes []byte) (*rsa.PrivateKey, error) {
328+
func DecodeX509CertificateChainBytes(certBytes []byte) ([]*x509.Certificate, error) {
329+
certs := []*x509.Certificate{}
330+
331+
- var block *pem.Block
332+
+ var block *stdpem.Block
333+
+ var err error
334+
335+
for {
336+
- // decode the tls certificate pem
337+
- block, certBytes = pem.Decode(certBytes)
338+
- if block == nil {
339+
- break
340+
+ block, certBytes, err = pem.SafeDecodeMultipleCertificates(certBytes)
341+
+ if err != nil {
342+
+ if err == pem.ErrNoPEMData {
343+
+ break
344+
+ }
345+
+
346+
+ return nil, err
347+
}
348+
349+
// parse the tls certificate
350+
@@ -131,8 +136,8 @@ func DecodeX509CertificateBytes(certBytes []byte) (*x509.Certificate, error) {
351+
352+
// DecodeX509CertificateRequestBytes will decode a PEM encoded x509 Certificate Request.
353+
func DecodeX509CertificateRequestBytes(csrBytes []byte) (*x509.CertificateRequest, error) {
354+
- block, _ := pem.Decode(csrBytes)
355+
- if block == nil {
356+
+ block, _, err := pem.SafeDecodeCSR(csrBytes)
357+
+ if err != nil {
358+
return nil, errors.NewInvalidData("error decoding certificate request PEM block")
359+
}
360+
361+
--
362+
2.34.1
363+

0 commit comments

Comments
 (0)