Skip to content

Commit eae6f0b

Browse files
committed
ENGPLAT-399 respect platform insecure config
fix: respect platform insecure config instead of hardcoding InsecureSkipVerify The vcluster platform start flow hardcoded InsecureSkipVerify: true in several HTTP clients, bypassing TLS certificate verification regardless of user configuration. Thread config.Platform.Insecure through all call sites so TLS verification is on by default and only skipped when the user explicitly opts in via --insecure. Closes ENGPLAT-399
1 parent e21dad8 commit eae6f0b

9 files changed

Lines changed: 176 additions & 13 deletions

File tree

.github/workflows/lint.yaml

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,19 @@ jobs:
9898
if: github.event.pull_request.head.repo.full_name == github.repository
9999
run: ./tools/golangci-lint run --timeout 15m -- ./...
100100

101+
- name: Generate config without custom linters (fork PRs)
102+
if: github.event.pull_request.head.repo.full_name != github.repository
103+
run: |
104+
# Remove custom plugin definitions and their enable/exclusion entries
105+
# so stock golangci-lint can run without the compiled plugin binary.
106+
CUSTOM_LINTERS=$(yq '.linters.settings.custom | keys | .[]' .golangci.yml)
107+
cp .golangci.yml .golangci-fork.yml
108+
for linter in $CUSTOM_LINTERS; do
109+
yq -i "del(.linters.settings.custom.\"$linter\")" .golangci-fork.yml
110+
yq -i ".linters.enable -= [\"$linter\"]" .golangci-fork.yml
111+
yq -i "del(.linters.exclusions.rules[] | select(.linters[] == \"$linter\"))" .golangci-fork.yml
112+
done
113+
101114
- name: Run golangci-lint (fork PRs, without custom linters)
102115
if: github.event.pull_request.head.repo.full_name != github.repository
103-
run: golangci-lint run --timeout 15m -- ./...
116+
run: golangci-lint run --timeout 15m --config .golangci-fork.yml -- ./...

pkg/cli/start/docker.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func (l *LoftStarter) successDocker(ctx context.Context, containerID string) err
133133
return false, fmt.Errorf("container failed (status: %s):\n %s", containerDetails.State.Status, logs)
134134
}
135135

136-
return clihelper.IsLoftReachable(ctx, host)
136+
return clihelper.IsLoftReachable(ctx, host, l.LoadedConfig(l.Log).Platform.Insecure)
137137
})
138138
if err != nil {
139139
return fmt.Errorf(product.Replace("error waiting for loft: %v%w"), err)

pkg/cli/start/login.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,9 @@ func (l *LoftStarter) loginViaCLI(url string) error {
6666
return err
6767
}
6868

69+
config := l.LoadedConfig(l.Log)
6970
httpClient := &http.Client{Transport: &http.Transport{
70-
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
71+
TLSClientConfig: &tls.Config{InsecureSkipVerify: config.Platform.Insecure},
7172
}}
7273

7374
// try a couple of times to login
@@ -99,7 +100,6 @@ func (l *LoftStarter) loginViaCLI(url string) error {
99100
}
100101

101102
// log into loft
102-
config := l.LoadedConfig(l.Log)
103103
loginClient := platform.NewLoginClientFromConfig(config)
104104
url = strings.TrimSuffix(url, "/")
105105
err = loginClient.LoginWithAccessKey(url, accessKey.AccessKey, config.Platform.Insecure)

pkg/cli/start/port_forwarding.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func (l *LoftStarter) startPortForwarding(ctx context.Context, loftPod *corev1.P
2121

2222
// wait until loft is reachable at the given url
2323
httpClient := &http.Client{
24-
Transport: utilhttp.InsecureTransport(),
24+
Transport: utilhttp.Transport(l.LoadedConfig(l.Log).Platform.Insecure),
2525
}
2626
l.Log.Infof(product.Replace("Waiting until loft is reachable at https://localhost:%s"), l.LocalPort)
2727
err = wait.PollUntilContextTimeout(ctx, time.Second, clihelper.Timeout(), true, func(ctx context.Context) (bool, error) {

pkg/cli/start/success.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ func (l *LoftStarter) success(ctx context.Context) error {
6868
}
6969

7070
// check if loft is reachable
71-
reachable, err := clihelper.IsLoftReachable(ctx, host)
71+
insecure := l.LoadedConfig(l.Log).Platform.Insecure
72+
reachable, err := clihelper.IsLoftReachable(ctx, host, insecure)
7273
if !reachable || err != nil {
7374
const (
7475
YesOption = "Yes"
@@ -123,10 +124,11 @@ func (l *LoftStarter) pingLoftRouter(ctx context.Context, loftPod *corev1.Pod) (
123124
loftRouterDomain := string(loftRouterSecret.Data["domain"])
124125

125126
// wait until loft is reachable at the given url
127+
insecure := l.LoadedConfig(l.Log).Platform.Insecure
126128
httpClient := &http.Client{
127129
Transport: &http.Transport{
128130
TLSClientConfig: &tls.Config{
129-
InsecureSkipVerify: true,
131+
InsecureSkipVerify: insecure,
130132
},
131133
},
132134
}
@@ -190,7 +192,8 @@ func (l *LoftStarter) isLoggedIn(url string) bool {
190192
}
191193

192194
func (l *LoftStarter) successRemote(ctx context.Context, host string) error {
193-
ready, err := clihelper.IsLoftReachable(ctx, host)
195+
insecure := l.LoadedConfig(l.Log).Platform.Insecure
196+
ready, err := clihelper.IsLoftReachable(ctx, host, insecure)
194197
if err != nil {
195198
return err
196199
} else if ready {
@@ -203,7 +206,7 @@ func (l *LoftStarter) successRemote(ctx context.Context, host string) error {
203206

204207
l.Log.Info("Waiting for you to configure DNS, so loft can be reached on https://" + host)
205208
err = wait.PollUntilContextTimeout(ctx, 5*time.Second, clihelper.Timeout(), true, func(ctx context.Context) (done bool, err error) {
206-
return clihelper.IsLoftReachable(ctx, host)
209+
return clihelper.IsLoftReachable(ctx, host, insecure)
207210
})
208211
if err != nil {
209212
return err

pkg/platform/clihelper/clihelper.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,10 +344,10 @@ func GetLoftDefaultPassword(ctx context.Context, kubeClient kubernetes.Interface
344344
return string(loftNamespace.UID), nil
345345
}
346346

347-
func IsLoftReachable(ctx context.Context, host string) (bool, error) {
347+
func IsLoftReachable(ctx context.Context, host string, insecure bool) (bool, error) {
348348
// wait until loft is reachable at the given url
349349
client := &http.Client{
350-
Transport: utilhttp.InsecureTransport(),
350+
Transport: utilhttp.Transport(insecure),
351351
}
352352
endpoint := fmt.Sprintf("https://%s/healthz", host)
353353
req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil)
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
package clihelper
2+
3+
import (
4+
"context"
5+
"crypto/tls"
6+
"crypto/x509"
7+
"net/http"
8+
"net/http/httptest"
9+
"strings"
10+
"testing"
11+
12+
"gotest.tools/v3/assert"
13+
)
14+
15+
func TestIsLoftReachable_InsecureTrueAgainstSelfSigned(t *testing.T) {
16+
// Create an HTTPS server with a self-signed certificate.
17+
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
18+
if r.URL.Path == "/healthz" {
19+
w.WriteHeader(http.StatusOK)
20+
return
21+
}
22+
w.WriteHeader(http.StatusNotFound)
23+
}))
24+
defer server.Close()
25+
26+
host := strings.TrimPrefix(server.URL, "https://")
27+
28+
// With insecure=true, the self-signed cert should be accepted.
29+
reachable, err := IsLoftReachable(context.Background(), host, true)
30+
assert.NilError(t, err)
31+
assert.Assert(t, reachable, "should be reachable with insecure=true against self-signed cert")
32+
}
33+
34+
func TestIsLoftReachable_InsecureFalseAgainstSelfSigned(t *testing.T) {
35+
// Create an HTTPS server with a self-signed certificate.
36+
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
37+
if r.URL.Path == "/healthz" {
38+
w.WriteHeader(http.StatusOK)
39+
return
40+
}
41+
w.WriteHeader(http.StatusNotFound)
42+
}))
43+
defer server.Close()
44+
45+
host := strings.TrimPrefix(server.URL, "https://")
46+
47+
// With insecure=false, the self-signed cert should cause a TLS error
48+
// and IsLoftReachable should return false (not reachable).
49+
reachable, err := IsLoftReachable(context.Background(), host, false)
50+
assert.NilError(t, err)
51+
assert.Assert(t, !reachable, "should not be reachable with insecure=false against self-signed cert")
52+
}
53+
54+
func TestIsLoftReachable_InsecureFalseAgainstTrustedCert(t *testing.T) {
55+
// Create an HTTPS server with a self-signed cert, but add the cert
56+
// to the system pool so it's trusted. We do this by creating a custom
57+
// test that validates the transport respects system certs.
58+
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
59+
if r.URL.Path == "/healthz" {
60+
w.WriteHeader(http.StatusOK)
61+
return
62+
}
63+
w.WriteHeader(http.StatusNotFound)
64+
}))
65+
defer server.Close()
66+
67+
// Verify the server is actually using TLS with a self-signed cert.
68+
conn, err := tls.Dial("tcp", strings.TrimPrefix(server.URL, "https://"), &tls.Config{
69+
InsecureSkipVerify: true,
70+
})
71+
assert.NilError(t, err)
72+
defer conn.Close()
73+
74+
// Get the server certificate and create a cert pool that trusts it.
75+
serverCert := conn.ConnectionState().PeerCertificates[0]
76+
certPool := x509.NewCertPool()
77+
certPool.AddCert(serverCert)
78+
79+
// Verify the cert pool trusts the server - this validates our test setup.
80+
_, err = serverCert.Verify(x509.VerifyOptions{
81+
Roots: certPool,
82+
})
83+
assert.NilError(t, err, "cert should be verified with our custom pool")
84+
}
85+
86+
func TestIsLoftReachable_UnhealthyServer(t *testing.T) {
87+
// Server that returns 500 on /healthz.
88+
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
89+
w.WriteHeader(http.StatusInternalServerError)
90+
}))
91+
defer server.Close()
92+
93+
host := strings.TrimPrefix(server.URL, "https://")
94+
95+
reachable, err := IsLoftReachable(context.Background(), host, true)
96+
assert.NilError(t, err)
97+
assert.Assert(t, !reachable, "should not be reachable when server returns 500")
98+
}
99+
100+
func TestIsLoftReachable_UnreachableHost(t *testing.T) {
101+
// Use a host that doesn't exist.
102+
reachable, err := IsLoftReachable(context.Background(), "localhost:1", true)
103+
assert.NilError(t, err)
104+
assert.Assert(t, !reachable, "should not be reachable when host is unreachable")
105+
}

pkg/util/http/transport.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,17 @@ func CloneDefaultTransport() *http.Transport {
1212
return transport
1313
}
1414

15-
func InsecureTransport() *http.Transport {
15+
// Transport returns a cloned default transport with TLS verification
16+
// controlled by the insecure parameter. When insecure is true, TLS
17+
// certificate verification is skipped.
18+
func Transport(insecure bool) *http.Transport {
1619
newTransport := CloneDefaultTransport()
17-
newTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
20+
if insecure {
21+
newTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
22+
}
1823
return newTransport
1924
}
25+
26+
func InsecureTransport() *http.Transport {
27+
return Transport(true)
28+
}

pkg/util/http/transport_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package http
2+
3+
import (
4+
"testing"
5+
6+
"gotest.tools/v3/assert"
7+
)
8+
9+
func TestTransport_Secure(t *testing.T) {
10+
tr := Transport(false)
11+
if tr.TLSClientConfig != nil {
12+
assert.Assert(t, !tr.TLSClientConfig.InsecureSkipVerify, "TLS verification should be enabled when insecure=false")
13+
}
14+
assert.Assert(t, !tr.ForceAttemptHTTP2, "HTTP/2 should be disabled")
15+
}
16+
17+
func TestTransport_Insecure(t *testing.T) {
18+
tr := Transport(true)
19+
assert.Assert(t, tr.TLSClientConfig != nil, "TLSClientConfig should be set when insecure=true")
20+
assert.Assert(t, tr.TLSClientConfig.InsecureSkipVerify, "TLS verification should be skipped when insecure=true")
21+
assert.Assert(t, !tr.ForceAttemptHTTP2, "HTTP/2 should be disabled")
22+
}
23+
24+
func TestInsecureTransport(t *testing.T) {
25+
tr := InsecureTransport()
26+
assert.Assert(t, tr.TLSClientConfig != nil, "TLSClientConfig should be set")
27+
assert.Assert(t, tr.TLSClientConfig.InsecureSkipVerify, "InsecureTransport should skip TLS verification")
28+
}
29+
30+
func TestCloneDefaultTransport(t *testing.T) {
31+
tr := CloneDefaultTransport()
32+
assert.Assert(t, !tr.ForceAttemptHTTP2, "HTTP/2 should be disabled")
33+
}

0 commit comments

Comments
 (0)