Skip to content

Commit c23243e

Browse files
0xavi00xavi0
authored
[v0.15] - Use Custom CAs when pulling OCI artifact (#5128) (#5183)
* Use Custom CAs when pulling OCI artifact (#5128) * Use Custom CAs when pulling OCI artifact This PR adds custom CAs when downloading OCI artifacts in a similar way we already use them when downloading helm charts. Refers to: #4897 Signed-off-by: Xavi Garcia <xavi.garcia@suse.com> * Change to be compatible with 0.15 Signed-off-by: Xavi Garcia <xavi.garcia@suse.com> --------- Signed-off-by: Xavi Garcia <xavi.garcia@suse.com>
1 parent c8932b5 commit c23243e

6 files changed

Lines changed: 310 additions & 25 deletions

File tree

‎_typos.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,5 @@ extend-exclude = [
1818
extend-ignore-re = [
1919
"-{5}BEGIN RSA PRIVATE KEY-{5}(?:$|[^-]{63,}-{5}END)",
2020
"-{5}BEGIN PRIVATE KEY-{5}(?:$|[^-]{63,}-{5}END)",
21+
"-{5}BEGIN CERTIFICATE-{5}(?:$|[^-]{63,}-{5}END CERTIFICATE)"
2122
]

‎internal/cmd/cli/apply/apply.go‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,7 @@ func shouldStoreInOCIRegistry(ctx context.Context, c client.Reader, ociSecretKey
575575
ociOpts.AgentPassword = opts.AgentPassword
576576
ociOpts.BasicHTTP = opts.BasicHTTP
577577
ociOpts.InsecureSkipTLS = opts.InsecureSkipTLS
578+
ociOpts.CABundle = opts.CABundle
578579

579580
return true, nil
580581
}
@@ -756,19 +757,27 @@ func newOCISecret(manifestID string, bundle *fleet.Bundle, opts ocistorage.OCIOp
756757
},
757758
},
758759
},
759-
Data: map[string][]byte{
760-
ocistorage.OCISecretReference: []byte(opts.Reference),
761-
ocistorage.OCISecretUsername: []byte(opts.Username),
762-
ocistorage.OCISecretPassword: []byte(opts.Password),
763-
ocistorage.OCISecretAgentUsername: []byte(opts.AgentUsername),
764-
ocistorage.OCISecretAgentPassword: []byte(opts.AgentPassword),
765-
ocistorage.OCISecretBasicHTTP: []byte(strconv.FormatBool(opts.BasicHTTP)),
766-
ocistorage.OCISecretInsecureSkipTLS: []byte(strconv.FormatBool(opts.InsecureSkipTLS)),
767-
},
760+
Data: newOCISecretData(opts),
768761
Type: fleet.SecretTypeOCIStorage,
769762
}
770763
}
771764

765+
func newOCISecretData(opts ocistorage.OCIOpts) map[string][]byte {
766+
data := map[string][]byte{
767+
ocistorage.OCISecretReference: []byte(opts.Reference),
768+
ocistorage.OCISecretUsername: []byte(opts.Username),
769+
ocistorage.OCISecretPassword: []byte(opts.Password),
770+
ocistorage.OCISecretAgentUsername: []byte(opts.AgentUsername),
771+
ocistorage.OCISecretAgentPassword: []byte(opts.AgentPassword),
772+
ocistorage.OCISecretBasicHTTP: []byte(strconv.FormatBool(opts.BasicHTTP)),
773+
ocistorage.OCISecretInsecureSkipTLS: []byte(strconv.FormatBool(opts.InsecureSkipTLS)),
774+
}
775+
if len(opts.CABundle) > 0 {
776+
data[ocistorage.OCISecretCABundle] = opts.CABundle
777+
}
778+
return data
779+
}
780+
772781
func newValuesSecret(bundle *fleet.Bundle, data map[string][]byte) *corev1.Secret {
773782
return &corev1.Secret{
774783
Type: fleet.SecretTypeBundleValues,

‎internal/ocistorage/ociwrapper.go‎

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"context"
66
"crypto/tls"
7+
"crypto/x509"
78
"encoding/json"
89
"fmt"
910
"io"
@@ -15,13 +16,15 @@ import (
1516

1617
"github.com/opencontainers/go-digest"
1718
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
18-
"github.com/rancher/fleet/internal/manifest"
19+
"github.com/sirupsen/logrus"
1920
"oras.land/oras-go/v2"
2021
"oras.land/oras-go/v2/content"
2122
"oras.land/oras-go/v2/content/memory"
2223
"oras.land/oras-go/v2/registry/remote"
2324
"oras.land/oras-go/v2/registry/remote/auth"
2425
"oras.land/oras-go/v2/registry/remote/retry"
26+
27+
"github.com/rancher/fleet/internal/manifest"
2528
)
2629

2730
const (
@@ -39,6 +42,7 @@ type OCIOpts struct {
3942
AgentPassword string
4043
BasicHTTP bool
4144
InsecureSkipTLS bool
45+
CABundle []byte
4246
}
4347

4448
type OrasOps interface {
@@ -71,20 +75,61 @@ func NewOCIWrapper() *OCIWrapper {
7175
}
7276
}
7377

74-
func getHTTPClient(insecureSkipTLS bool) *http.Client {
75-
if !insecureSkipTLS {
78+
func getHTTPClient(insecureSkipTLS bool, caBundle []byte) *http.Client {
79+
// Defensive copy to avoid mutations
80+
caBundle = append([]byte(nil), caBundle...)
81+
82+
// Merge proxy CA bundle if present
83+
if proxyCAPEM, ok := os.LookupEnv("PROXY_CA_BUNDLE"); ok && proxyCAPEM != "" {
84+
proxyBytes := []byte(proxyCAPEM)
85+
tmpPool := x509.NewCertPool()
86+
if !tmpPool.AppendCertsFromPEM(proxyBytes) {
87+
logrus.Warnf("%s is set but contains no valid PEM certificates; ignoring proxy CA bundle", "PROXY_CA_BUNDLE")
88+
} else {
89+
if len(caBundle) > 0 && caBundle[len(caBundle)-1] != '\n' {
90+
caBundle = append(caBundle, '\n')
91+
}
92+
caBundle = append(caBundle, proxyBytes...)
93+
}
94+
}
95+
96+
// If no custom TLS config needed, use default ORAS client
97+
if !insecureSkipTLS && len(caBundle) == 0 {
7698
return retry.DefaultClient
7799
}
78-
return &http.Client{
79-
Transport: &http.Transport{
80-
TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // #nosec G402
81-
},
100+
101+
// Clone the default transport to preserve proxy, timeout, and
102+
// connection-pooling settings.
103+
baseTransport, ok := http.DefaultTransport.(*http.Transport)
104+
if !ok {
105+
baseTransport = &http.Transport{
106+
Proxy: http.ProxyFromEnvironment,
107+
}
108+
}
109+
transport := baseTransport.Clone()
110+
transport.TLSClientConfig = &tls.Config{
111+
InsecureSkipVerify: insecureSkipTLS, // #nosec G402
112+
}
113+
114+
// Merge custom CA bundle with system cert pool
115+
if len(caBundle) > 0 {
116+
pool, err := x509.SystemCertPool()
117+
if err != nil {
118+
pool = x509.NewCertPool()
119+
}
120+
if !pool.AppendCertsFromPEM(caBundle) {
121+
logrus.Warnf("CA bundle contains no valid PEM certificates; proceeding with system cert pool only")
122+
}
123+
transport.TLSClientConfig.RootCAs = pool
124+
transport.TLSClientConfig.MinVersion = tls.VersionTLS12
82125
}
126+
127+
return &http.Client{Transport: retry.NewTransport(transport)}
83128
}
84129

85130
func getAuthClient(opts OCIOpts) *auth.Client {
86131
client := &auth.Client{
87-
Client: getHTTPClient(opts.InsecureSkipTLS),
132+
Client: getHTTPClient(opts.InsecureSkipTLS, opts.CABundle),
88133
Cache: auth.NewCache(),
89134
}
90135
if opts.Username != "" {

‎internal/ocistorage/ociwrapper_test.go‎

Lines changed: 173 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"context"
66
"crypto/tls"
7+
"crypto/x509"
78
"fmt"
89
"net/http"
910
"os"
@@ -136,17 +137,181 @@ var _ = Describe("OCIUtils tests", func() {
136137
Expect(repo.PlainHTTP).To(BeFalse())
137138
})
138139
It("return the expected tls client", func() {
139-
client := getHTTPClient(true)
140-
expected := &http.Client{
141-
Transport: &http.Transport{
142-
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
143-
},
144-
}
145-
Expect(client).To(Equal(expected))
140+
client := getHTTPClient(true, nil)
141+
142+
// Custom path wraps transport in retry.Transport
143+
retryTransport, ok := client.Transport.(*retry.Transport)
144+
Expect(ok).To(BeTrue())
145+
innerTransport, ok := retryTransport.Base.(*http.Transport)
146+
Expect(ok).To(BeTrue())
147+
Expect(innerTransport.TLSClientConfig).ToNot(BeNil())
148+
Expect(innerTransport.TLSClientConfig.InsecureSkipVerify).To(BeTrue())
149+
Expect(innerTransport.Proxy).ToNot(BeNil())
150+
151+
client = getHTTPClient(false, nil)
152+
Expect(client).To(Equal(retry.DefaultClient))
153+
})
154+
It("should use custom CA bundle when provided", func() {
155+
// Use a valid test certificate from the codebase pattern (same as netutils_test.go)
156+
caBundle := []byte(`-----BEGIN CERTIFICATE-----
157+
MIICGTCCAZ+gAwIBAgIQCeCTZaz32ci5PhwLBCou8zAKBggqhkjOPQQDAzBOMQsw
158+
CQYDVQQGEwJVUzEXMBUGA1UEChMORGlnaUNlcnQsIEluYy4xJjAkBgNVBAMTHURp
159+
Z2lDZXJ0IFRMUyBFQ0MgUDM4NCBSb290IEc1MB4XDTIxMDExNTAwMDAwMFoXDTQ2
160+
MDExNDIzNTk1OVowTjELMAkGA1UEBhMCVVMxFzAVBgNVBAoTDkRpZ2lDZXJ0LCBJ
161+
bmMuMSYwJAYDVQQDEx1EaWdpQ2VydCBUTFMgRUNDIFAzODQgUm9vdCBHNTB2MBAG
162+
ByqGSM49AgEGBSuBBAAiA2IABMFEoc8Rl1Ca3iOCNQfN0MsYndLxf3c1TzvdlHJS
163+
7cI7+Oz6e2tYIOyZrsn8aLN1udsJ7MgT9U7GCh1mMEy7H0cKPGEQQil8pQgO4CLp
164+
0zVozptjn4S1mU1YoI71VOeVyaNCMEAwHQYDVR0OBBYEFMFRRVBZqz7nLFr6ICIS
165+
B4CIfBFqMA4GA1UdDwEB/wQEAwIBhjAPBgNVHRMBAf8EBTADAQH/MAoGCCqGSM49
166+
BAMDA2gAMGUCMQCJao1H5+z8blUD2WdsJk6Dxv3J+ysTvLd6jLRl0mlpYxNjOyZQ
167+
LgGheQaRnUi/wr4CMEfDFXuxoJGZSZOoPHzoRgaLLPIxAJSdYsiJvRmEFOml+wG4
168+
DXZDjC5Ty3zfDBeWUA==
169+
-----END CERTIFICATE-----`)
170+
client := getHTTPClient(false, caBundle)
171+
172+
// Verify transport is configured with custom TLS config
173+
retryT, ok := client.Transport.(*retry.Transport)
174+
Expect(ok).To(BeTrue())
175+
transport, ok := retryT.Base.(*http.Transport)
176+
Expect(ok).To(BeTrue())
177+
Expect(transport.TLSClientConfig).ToNot(BeNil())
178+
Expect(transport.TLSClientConfig.RootCAs).ToNot(BeNil())
179+
Expect(transport.TLSClientConfig.MinVersion).To(Equal(uint16(tls.VersionTLS12)))
180+
Expect(transport.TLSClientConfig.InsecureSkipVerify).To(BeFalse())
181+
})
182+
It("should merge proxy CA bundle from environment variable", func() {
183+
// Use a valid certificate for proxy CA (same as custom test)
184+
proxyCA := `-----BEGIN CERTIFICATE-----
185+
MIICGTCCAZ+gAwIBAgIQCeCTZaz32ci5PhwLBCou8zAKBggqhkjOPQQDAzBOMQsw
186+
CQYDVQQGEwJVUzEXMBUGA1UEChMORGlnaUNlcnQsIEluYy4xJjAkBgNVBAMTHURp
187+
Z2lDZXJ0IFRMUyBFQ0MgUDM4NCBSb290IEc1MB4XDTIxMDExNTAwMDAwMFoXDTQ2
188+
MDExNDIzNTk1OVowTjELMAkGA1UEBhMCVVMxFzAVBgNVBAoTDkRpZ2lDZXJ0LCBJ
189+
bmMuMSYwJAYDVQQDEx1EaWdpQ2VydCBUTFMgRUNDIFAzODQgUm9vdCBHNTB2MBAG
190+
ByqGSM49AgEGBSuBBAAiA2IABMFEoc8Rl1Ca3iOCNQfN0MsYndLxf3c1TzvdlHJS
191+
7cI7+Oz6e2tYIOyZrsn8aLN1udsJ7MgT9U7GCh1mMEy7H0cKPGEQQil8pQgO4CLp
192+
0zVozptjn4S1mU1YoI71VOeVyaNCMEAwHQYDVR0OBBYEFMFRRVBZqz7nLFr6ICIS
193+
B4CIfBFqMA4GA1UdDwEB/wQEAwIBhjAPBgNVHRMBAf8EBTADAQH/MAoGCCqGSM49
194+
BAMDA2gAMGUCMQCJao1H5+z8blUD2WdsJk6Dxv3J+ysTvLd6jLRl0mlpYxNjOyZQ
195+
LgGheQaRnUi/wr4CMEfDFXuxoJGZSZOoPHzoRgaLLPIxAJSdYsiJvRmEFOml+wG4
196+
DXZDjC5Ty3zfDBeWUA==
197+
-----END CERTIFICATE-----`
198+
os.Setenv("PROXY_CA_BUNDLE", proxyCA)
199+
defer os.Unsetenv("PROXY_CA_BUNDLE")
200+
201+
// Use the same valid certificate for custom CA (simpler for testing)
202+
customCA := []byte(proxyCA)
203+
client := getHTTPClient(false, customCA)
204+
205+
// Should have merged both CAs
206+
retryT, ok := client.Transport.(*retry.Transport)
207+
Expect(ok).To(BeTrue())
208+
transport, ok := retryT.Base.(*http.Transport)
209+
Expect(ok).To(BeTrue())
210+
Expect(transport.TLSClientConfig).ToNot(BeNil())
211+
Expect(transport.TLSClientConfig.RootCAs).ToNot(BeNil())
212+
Expect(transport.TLSClientConfig.MinVersion).To(Equal(uint16(tls.VersionTLS12)))
213+
214+
// Verify both CAs are valid and would have been loaded
215+
testPool := x509.NewCertPool()
216+
ok = testPool.AppendCertsFromPEM(customCA)
217+
Expect(ok).To(BeTrue(), "custom CA bundle should be valid PEM")
218+
ok = testPool.AppendCertsFromPEM([]byte(proxyCA))
219+
Expect(ok).To(BeTrue(), "proxy CA bundle should be valid PEM")
220+
})
221+
It("should handle invalid CA bundle gracefully", func() {
222+
invalidCA := []byte("not a valid PEM certificate")
223+
client := getHTTPClient(false, invalidCA)
224+
225+
// Should still create a client with TLS config (warning logged)
226+
retryT, ok := client.Transport.(*retry.Transport)
227+
Expect(ok).To(BeTrue())
228+
transport, ok := retryT.Base.(*http.Transport)
229+
Expect(ok).To(BeTrue())
230+
Expect(transport.TLSClientConfig).ToNot(BeNil())
231+
Expect(transport.TLSClientConfig.RootCAs).ToNot(BeNil())
232+
})
233+
It("should handle invalid proxy CA bundle gracefully", func() {
234+
os.Setenv("PROXY_CA_BUNDLE", "invalid proxy PEM")
235+
defer os.Unsetenv("PROXY_CA_BUNDLE")
236+
237+
client := getHTTPClient(false, nil)
146238

147-
client = getHTTPClient(false)
239+
// Should return default client when no valid CA and not insecure
148240
Expect(client).To(Equal(retry.DefaultClient))
149241
})
242+
It("should combine insecureSkipTLS with CA bundle", func() {
243+
caBundle := []byte(`-----BEGIN CERTIFICATE-----
244+
MIICGTCCAZ+gAwIBAgIQCeCTZaz32ci5PhwLBCou8zAKBggqhkjOPQQDAzBOMQsw
245+
CQYDVQQGEwJVUzEXMBUGA1UEChMORGlnaUNlcnQsIEluYy4xJjAkBgNVBAMTHURp
246+
Z2lDZXJ0IFRMUyBFQ0MgUDM4NCBSb290IEc1MB4XDTIxMDExNTAwMDAwMFoXDTQ2
247+
MDExNDIzNTk1OVowTjELMAkGA1UEBhMCVVMxFzAVBgNVBAoTDkRpZ2lDZXJ0LCBJ
248+
bmMuMSYwJAYDVQQDEx1EaWdpQ2VydCBUTFMgRUNDIFAzODQgUm9vdCBHNTB2MBAG
249+
ByqGSM49AgEGBSuBBAAiA2IABMFEoc8Rl1Ca3iOCNQfN0MsYndLxf3c1TzvdlHJS
250+
7cI7+Oz6e2tYIOyZrsn8aLN1udsJ7MgT9U7GCh1mMEy7H0cKPGEQQil8pQgO4CLp
251+
0zVozptjn4S1mU1YoI71VOeVyaNCMEAwHQYDVR0OBBYEFMFRRVBZqz7nLFr6ICIS
252+
B4CIfBFqMA4GA1UdDwEB/wQEAwIBhjAPBgNVHRMBAf8EBTADAQH/MAoGCCqGSM49
253+
BAMDA2gAMGUCMQCJao1H5+z8blUD2WdsJk6Dxv3J+ysTvLd6jLRl0mlpYxNjOyZQ
254+
LgGheQaRnUi/wr4CMEfDFXuxoJGZSZOoPHzoRgaLLPIxAJSdYsiJvRmEFOml+wG4
255+
DXZDjC5Ty3zfDBeWUA==
256+
-----END CERTIFICATE-----`)
257+
client := getHTTPClient(true, caBundle)
258+
259+
retryT, ok := client.Transport.(*retry.Transport)
260+
Expect(ok).To(BeTrue())
261+
transport, ok := retryT.Base.(*http.Transport)
262+
Expect(ok).To(BeTrue())
263+
Expect(transport.TLSClientConfig).ToNot(BeNil())
264+
Expect(transport.TLSClientConfig.InsecureSkipVerify).To(BeTrue())
265+
Expect(transport.TLSClientConfig.RootCAs).ToNot(BeNil())
266+
Expect(transport.TLSClientConfig.MinVersion).To(Equal(uint16(tls.VersionTLS12)))
267+
268+
// Verify the CA bundle is valid PEM (even though InsecureSkipVerify is set)
269+
testPool := x509.NewCertPool()
270+
ok = testPool.AppendCertsFromPEM(caBundle)
271+
Expect(ok).To(BeTrue(), "CA bundle should be valid PEM")
272+
})
273+
It("should not mutate the original CA bundle slice", func() {
274+
// validPEM is a real certificate so that AppendCertsFromPEM succeeds and
275+
// getHTTPClient actually reaches the append(caBundle, proxyBytes...) path.
276+
// An invalid PEM would make the function return early without appending,
277+
// meaning the defensive copy would never be exercised.
278+
validPEM := []byte(`-----BEGIN CERTIFICATE-----
279+
MIICGTCCAZ+gAwIBAgIQCeCTZaz32ci5PhwLBCou8zAKBggqhkjOPQQDAzBOMQsw
280+
CQYDVQQGEwJVUzEXMBUGA1UEChMORGlnaUNlcnQsIEluYy4xJjAkBgNVBAMTHURp
281+
Z2lDZXJ0IFRMUyBFQ0MgUDM4NCBSb290IEc1MB4XDTIxMDExNTAwMDAwMFoXDTQ2
282+
MDExNDIzNTk1OVowTjELMAkGA1UEBhMCVVMxFzAVBgNVBAoTDkRpZ2lDZXJ0LCBJ
283+
bmMuMSYwJAYDVQQDEx1EaWdpQ2VydCBUTFMgRUNDIFAzODQgUm9vdCBHNTB2MBAG
284+
ByqGSM49AgEGBSuBBAAiA2IABMFEoc8Rl1Ca3iOCNQfN0MsYndLxf3c1TzvdlHJS
285+
7cI7+Oz6e2tYIOyZrsn8aLN1udsJ7MgT9U7GCh1mMEy7H0cKPGEQQil8pQgO4CLp
286+
0zVozptjn4S1mU1YoI71VOeVyaNCMEAwHQYDVR0OBBYEFMFRRVBZqz7nLFr6ICIS
287+
B4CIfBFqMA4GA1UdDwEB/wQEAwIBhjAPBgNVHRMBAf8EBTADAQH/MAoGCCqGSM49
288+
BAMDA2gAMGUCMQCJao1H5+z8blUD2WdsJk6Dxv3J+ysTvLd6jLRl0mlpYxNjOyZQ
289+
LgGheQaRnUi/wr4CMEfDFXuxoJGZSZOoPHzoRgaLLPIxAJSdYsiJvRmEFOml+wG4
290+
DXZDjC5Ty3zfDBeWUA==
291+
-----END CERTIFICATE-----`)
292+
293+
os.Setenv("PROXY_CA_BUNDLE", string(validPEM))
294+
defer os.Unsetenv("PROXY_CA_BUNDLE")
295+
296+
// Allocate with excess capacity so that append can write into the backing
297+
// array without reallocating. With len == cap (e.g. []byte("literal")),
298+
// append always reallocates and the defensive copy makes no observable
299+
// difference — the test would pass even without it.
300+
originalCA := make([]byte, len(validPEM), len(validPEM)+512)
301+
copy(originalCA, validPEM)
302+
303+
// backing covers the full allocation so we can detect writes past len(originalCA).
304+
backing := originalCA[:cap(originalCA)]
305+
306+
_ = getHTTPClient(false, originalCA)
307+
308+
// The slice header and visible content must be unchanged.
309+
Expect(originalCA).To(Equal(validPEM))
310+
// Without the defensive copy, getHTTPClient would append '\n'+proxyBytes
311+
// directly into the excess capacity of the caller's backing array.
312+
Expect(backing[len(validPEM):]).To(Equal(make([]byte, 512)),
313+
"backing array beyond len(originalCA) must not be written to")
314+
})
150315
It("return the expected credentials", func() {
151316
opts := OCIOpts{
152317
Reference: "test.com",

‎internal/ocistorage/secret.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const (
2121
OCISecretBasicHTTP = "basicHTTP"
2222
OCISecretInsecureSkipTLS = "insecureSkipTLS"
2323
OCISecretInsecure = "insecure" // legacy alias
24+
OCISecretCABundle = "cacerts"
2425
)
2526

2627
// ReadOptsFromSecret reads the secret identified by the given NamespacedName and
@@ -84,6 +85,9 @@ func ReadOptsFromSecret(ctx context.Context, c client.Reader, ns client.ObjectKe
8485
return OCIOpts{}, err
8586
}
8687

88+
// Read optional CA bundle
89+
opts.CABundle = secret.Data[OCISecretCABundle]
90+
8791
return opts, nil
8892
}
8993

0 commit comments

Comments
 (0)