From ec29cc2745b4dc21f03dc71a3f7f2460b5a1db6e Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Wed, 25 Mar 2026 15:19:48 +1000 Subject: [PATCH] Revert "fix: remove InsecureSkipVerify (#87)" This reverts commit a66b1a3d4888dc5801d9a956ab4f3e67207939c4. --- .golangci.yml | 2 + cmd/spinifex/cmd/admin.go | 28 +++---------- cmd/spinifex/cmd/root.go | 5 ++- scripts/iac/configure-cluster.sh | 2 +- spinifex/admin/admin.go | 2 +- spinifex/daemon/daemon_handlers.go | 15 +++---- spinifex/daemon/network.go | 2 +- spinifex/formation/formation.go | 20 ++------- spinifex/formation/formation_test.go | 51 ----------------------- spinifex/handlers/ec2/key/service_impl.go | 4 +- spinifex/handlers/ec2/vpc/eni.go | 2 +- spinifex/handlers/ec2/vpc/ipam.go | 2 +- spinifex/handlers/ec2/vpc/service_impl.go | 4 +- spinifex/objectstore/objectstore.go | 13 ++++++ spinifex/services/vpcd/topology.go | 2 +- spinifex/utils/utils.go | 12 ++++++ tests/e2e/lib/multinode-helpers.sh | 2 +- 17 files changed, 58 insertions(+), 110 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 99167e0e3..3ee184fc4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -124,3 +124,5 @@ linters: linters: [gosec] # subprocess execution — legitimate in VM orchestration - text: "G304" linters: [gosec] # file path from variable — necessary for config loading + - text: "G402" + linters: [gosec] # TLS config — we handle TLS ourselves diff --git a/cmd/spinifex/cmd/admin.go b/cmd/spinifex/cmd/admin.go index dcfe7c6e8..a460f4c5b 100644 --- a/cmd/spinifex/cmd/admin.go +++ b/cmd/spinifex/cmd/admin.go @@ -19,7 +19,6 @@ package cmd import ( "bytes" "context" - "crypto/tls" _ "embed" "encoding/base64" "encoding/json" @@ -994,11 +993,9 @@ func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, os.Exit(1) } - // Start formation server (HTTPS using the CA-signed server cert) + // Start formation server formationAddr := fmt.Sprintf("%s:%d", bindIP, port) - serverCert := filepath.Join(configDir, "server.pem") - serverKey := filepath.Join(configDir, "server.key") - if err := fs.Start(formationAddr, serverCert, serverKey); err != nil { + if err := fs.Start(formationAddr); err != nil { fmt.Fprintf(os.Stderr, "❌ Error starting formation server: %v\n", err) os.Exit(1) } @@ -1216,14 +1213,9 @@ func runAdminJoin(cmd *cobra.Command, args []string) { os.Exit(1) } - client := &http.Client{ - Timeout: 30 * time.Second, - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // joining node doesn't have CA yet - }, - } + client := &http.Client{Timeout: 30 * time.Second} - joinURL := fmt.Sprintf("https://%s/formation/join", leaderHost) + joinURL := fmt.Sprintf("http://%s/formation/join", leaderHost) resp, err := client.Post(joinURL, "application/json", bytes.NewBuffer(reqBody)) if err != nil { fmt.Fprintf(os.Stderr, "❌ Error connecting to formation server: %v\n", err) @@ -1238,11 +1230,6 @@ func runAdminJoin(cmd *cobra.Command, args []string) { os.Exit(1) } - if resp.StatusCode != http.StatusOK { - fmt.Fprintf(os.Stderr, "❌ Formation server returned HTTP %d: %s\n", resp.StatusCode, strings.TrimSpace(string(body))) - os.Exit(1) - } - var joinResp formation.JoinResponse if err := json.Unmarshal(body, &joinResp); err != nil { fmt.Fprintf(os.Stderr, "Error parsing join response: %v\n", err) @@ -1257,7 +1244,7 @@ func runAdminJoin(cmd *cobra.Command, args []string) { fmt.Printf("✅ Registered with formation server (%d/%d nodes joined)\n", joinResp.Joined, joinResp.Expected) // Poll status until formation is complete - statusURL := fmt.Sprintf("https://%s/formation/status", leaderHost) + statusURL := fmt.Sprintf("http://%s/formation/status", leaderHost) var statusResp formation.StatusResponse for { @@ -1274,11 +1261,6 @@ func runAdminJoin(cmd *cobra.Command, args []string) { os.Exit(1) } - if sResp.StatusCode != http.StatusOK { - fmt.Fprintf(os.Stderr, "❌ Formation server returned HTTP %d: %s\n", sResp.StatusCode, strings.TrimSpace(string(sBody))) - os.Exit(1) - } - if err := json.Unmarshal(sBody, &statusResp); err != nil { fmt.Fprintf(os.Stderr, "Error parsing status response: %v\n", err) os.Exit(1) diff --git a/cmd/spinifex/cmd/root.go b/cmd/spinifex/cmd/root.go index b57698ab9..49475dafb 100644 --- a/cmd/spinifex/cmd/root.go +++ b/cmd/spinifex/cmd/root.go @@ -26,8 +26,9 @@ import ( ) var ( - cfgFile string - appConfig *config.ClusterConfig //nolint:unused // TODO: wire into config loading + cfgFile string + //lint:ignore U1000 fixing config loading later + appConfig *config.ClusterConfig ) // rootCmd represents the base command when called without any subcommands diff --git a/scripts/iac/configure-cluster.sh b/scripts/iac/configure-cluster.sh index d6a3d3362..7657bb818 100755 --- a/scripts/iac/configure-cluster.sh +++ b/scripts/iac/configure-cluster.sh @@ -110,7 +110,7 @@ INIT_PID=$! # Wait for formation server to be ready echo " Waiting for formation server on $NODE1_IP..." attempts=0 -while ! curl -sk --connect-timeout 3 "https://$NODE1_IP:4432/formation/health" >/dev/null 2>&1; do +while ! curl -s --connect-timeout 3 "http://$NODE1_IP:4432/formation/health" >/dev/null 2>&1; do attempts=$((attempts + 1)) if [ $attempts -ge 60 ]; then echo "Error: Formation server did not start on $NODE1_IP after 5 minutes" diff --git a/spinifex/admin/admin.go b/spinifex/admin/admin.go index 9cd12d336..638aab812 100644 --- a/spinifex/admin/admin.go +++ b/spinifex/admin/admin.go @@ -249,7 +249,7 @@ func ChownRecursive(path, username string) { root, rootErr := os.OpenRoot(path) if rootErr != nil { // Fallback: direct chown on the top-level path only - if chownErr := os.Chown(path, uid, gid); chownErr != nil { + if chownErr := os.Chown(path, uid, gid); chownErr != nil { // #nosec G122 slog.Debug("chown failed", "path", path, "err", chownErr) } return diff --git a/spinifex/daemon/daemon_handlers.go b/spinifex/daemon/daemon_handlers.go index 8a89ef33a..424902082 100644 --- a/spinifex/daemon/daemon_handlers.go +++ b/spinifex/daemon/daemon_handlers.go @@ -1,12 +1,12 @@ package daemon import ( + "crypto/tls" "encoding/json" "fmt" "log/slog" "net" "net/http" - "strings" "sync" "time" @@ -218,7 +218,12 @@ func (d *Daemon) queryPredastoreRole() string { var roleHTTPClient = &http.Client{Timeout: 500 * time.Millisecond} -var roleTLSHTTPClient = &http.Client{Timeout: 500 * time.Millisecond} +var roleTLSHTTPClient = &http.Client{ + Timeout: 500 * time.Millisecond, + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // local self-signed cert + }, +} // fetchNATSRole queries a NATS /varz endpoint and returns "leader", "follower", or "". func fetchNATSRole(url string, client *http.Client) string { @@ -257,11 +262,7 @@ func fetchNATSRole(url string, client *http.Client) string { func fetchPredastoreRole(url string, client *http.Client) string { resp, err := client.Get(url) //nolint:noctx // internal monitoring call if err != nil { - if strings.Contains(err.Error(), "x509") || strings.Contains(err.Error(), "certificate") || strings.Contains(err.Error(), "tls:") { - slog.Warn("Failed to query Predastore status (TLS misconfiguration — is the Spinifex CA installed in the system trust store?)", "err", err) - } else { - slog.Debug("Failed to query Predastore status", "err", err) - } + slog.Debug("Failed to query Predastore status", "err", err) return "" } defer resp.Body.Close() diff --git a/spinifex/daemon/network.go b/spinifex/daemon/network.go index 9924fe4d4..f079fea50 100644 --- a/spinifex/daemon/network.go +++ b/spinifex/daemon/network.go @@ -119,7 +119,7 @@ func OVSIfaceID(eniId string) string { func generateDevMAC(instanceId string) string { h := uint32(0) for _, c := range instanceId { - h = h*31 + uint32(c) //nolint:gosec // G115: intentional overflow for hashing + h = h*31 + uint32(c) // #nosec G115 -- intentional overflow for hashing } return fmt.Sprintf("02:de:00:%02x:%02x:%02x", (h>>16)&0xff, (h>>8)&0xff, h&0xff) } diff --git a/spinifex/formation/formation.go b/spinifex/formation/formation.go index a63f8e3bc..868fb7292 100644 --- a/spinifex/formation/formation.go +++ b/spinifex/formation/formation.go @@ -2,7 +2,6 @@ package formation import ( "context" - "crypto/tls" "encoding/json" "fmt" "log/slog" @@ -166,9 +165,8 @@ func (fs *FormationServer) SetMasterKey(key string) { fs.masterKey = key } -// Start launches the HTTPS server on the given address (e.g. "10.0.0.1:4432"). -// tlsCert and tlsKey are paths to the CA-signed server certificate and key. -func (fs *FormationServer) Start(bindAddr, tlsCert, tlsKey string) error { +// Start launches the HTTP server on the given address (e.g. "10.0.0.1:4432"). +func (fs *FormationServer) Start(bindAddr string) error { mux := http.NewServeMux() mux.HandleFunc("POST /formation/join", fs.handleJoin) mux.HandleFunc("GET /formation/status", fs.handleStatus) @@ -183,28 +181,18 @@ func (fs *FormationServer) Start(bindAddr, tlsCert, tlsKey string) error { ReadHeaderTimeout: 5 * time.Second, } - cert, err := tls.LoadX509KeyPair(tlsCert, tlsKey) - if err != nil { - return fmt.Errorf("formation server TLS: %w", err) - } - ln, err := net.Listen("tcp", bindAddr) if err != nil { return fmt.Errorf("formation server listen: %w", err) } - tlsLn := tls.NewListener(ln, &tls.Config{ - Certificates: []tls.Certificate{cert}, - MinVersion: tls.VersionTLS12, - }) - go func() { - if err := fs.server.Serve(tlsLn); err != nil && err != http.ErrServerClosed { + if err := fs.server.Serve(ln); err != nil && err != http.ErrServerClosed { slog.Error("Formation server error", "error", err) } }() - slog.Info("Formation server started", "addr", bindAddr, "tls", true) + slog.Info("Formation server started", "addr", bindAddr) return nil } diff --git a/spinifex/formation/formation_test.go b/spinifex/formation/formation_test.go index 32b7325e5..21eb3ab31 100644 --- a/spinifex/formation/formation_test.go +++ b/spinifex/formation/formation_test.go @@ -2,20 +2,12 @@ package formation import ( "bytes" - "context" - "crypto/tls" - "crypto/x509" "encoding/json" - "fmt" - "net" "net/http" "net/http/httptest" - "os" - "path/filepath" "testing" "time" - "github.com/mulgadc/spinifex/spinifex/admin" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -347,46 +339,3 @@ func TestFullFormationFlow(t *testing.T) { assert.Len(t, pnodes, 3) assert.Equal(t, 1, pnodes[0].ID) } - -func TestStartTLS(t *testing.T) { - t.Parallel() - - // Generate temp CA + server cert - tmpDir := t.TempDir() - caCertPath := filepath.Join(tmpDir, "ca.pem") - caKeyPath := filepath.Join(tmpDir, "ca.key") - serverCertPath := filepath.Join(tmpDir, "server.pem") - serverKeyPath := filepath.Join(tmpDir, "server.key") - - require.NoError(t, admin.GenerateCACert(caCertPath, caKeyPath)) - require.NoError(t, admin.GenerateSignedCert(serverCertPath, serverKeyPath, caCertPath, caKeyPath, "127.0.0.1")) - - // Pick a free port - ln, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err) - addr := ln.Addr().String() - ln.Close() - - // Start the formation server with TLS - fs := NewFormationServer(1, testCreds(), "ca-cert", "ca-key") - require.NoError(t, fs.Start(addr, serverCertPath, serverKeyPath)) - defer fs.Shutdown(context.Background()) - - // Build an HTTPS client that trusts the CA - caCert, err := os.ReadFile(caCertPath) - require.NoError(t, err) - pool := x509.NewCertPool() - require.True(t, pool.AppendCertsFromPEM(caCert)) - - client := &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{RootCAs: pool}, - }, - } - - // Hit /formation/health over HTTPS and verify 200 OK - resp, err := client.Get(fmt.Sprintf("https://%s/formation/health", addr)) - require.NoError(t, err) - defer resp.Body.Close() - assert.Equal(t, http.StatusOK, resp.StatusCode) -} diff --git a/spinifex/handlers/ec2/key/service_impl.go b/spinifex/handlers/ec2/key/service_impl.go index 3aa0e1765..c0372fddb 100644 --- a/spinifex/handlers/ec2/key/service_impl.go +++ b/spinifex/handlers/ec2/key/service_impl.go @@ -2,7 +2,7 @@ package handlers_ec2_key import ( "bytes" - "crypto/md5" //nolint:gosec // G501: need md5 for AWS compatibility + "crypto/md5" //#nosec G501 - need md5 for AWS compatibility "crypto/sha256" "encoding/base64" "encoding/hex" @@ -217,7 +217,7 @@ func (s *KeyServiceImpl) calculateFingerprint(publicKeyData []byte, keyType stri } else { // RSA uses SHA-1 or MD5 fingerprint // AWS uses MD5 for RSA keys for backward compatibility - hash := md5.Sum(keyData) //nolint:gosec // G401: need md5 for AWS compatibility + hash := md5.Sum(keyData) //#nosec G401 - need md5 for AWS compatibility return formatFingerprint(hash[:], "MD5"), nil } } diff --git a/spinifex/handlers/ec2/vpc/eni.go b/spinifex/handlers/ec2/vpc/eni.go index 024dcb834..741ab0951 100644 --- a/spinifex/handlers/ec2/vpc/eni.go +++ b/spinifex/handlers/ec2/vpc/eni.go @@ -407,7 +407,7 @@ func (s *VPCServiceImpl) eniRecordToEC2(record *ENIRecord, accountID string) *ec func generateENIMac(eniId string) string { h := uint32(0) for _, c := range eniId { - h = h*31 + uint32(c) //nolint:gosec // G115: intentional overflow for hashing + h = h*31 + uint32(c) // #nosec G115 -- intentional overflow for hashing } return fmt.Sprintf("02:00:00:%02x:%02x:%02x", (h>>16)&0xff, (h>>8)&0xff, h&0xff) } diff --git a/spinifex/handlers/ec2/vpc/ipam.go b/spinifex/handlers/ec2/vpc/ipam.go index 3936bff37..f134fdf17 100644 --- a/spinifex/handlers/ec2/vpc/ipam.go +++ b/spinifex/handlers/ec2/vpc/ipam.go @@ -182,7 +182,7 @@ func (m *IPAM) nextAvailableIP(record *IPAMRecord) (string, error) { ones, bits := ipNet.Mask.Size() hostBits := bits - ones // always 0-32 for IPv4 CIDRs, safe for uint conversion - totalIPs := new(big.Int).Lsh(big.NewInt(1), uint(hostBits)) //nolint:gosec // G115: hostBits is 0-32 for IPv4 + totalIPs := new(big.Int).Lsh(big.NewInt(1), uint(hostBits)) //#nosec G115 - hostBits is 0-32 for IPv4 // Start at offset 4 (.0=network, .1=gateway, .2=DNS, .3=reserved) networkIP := ipToInt(ipNet.IP) diff --git a/spinifex/handlers/ec2/vpc/service_impl.go b/spinifex/handlers/ec2/vpc/service_impl.go index 174004974..f9739cef3 100644 --- a/spinifex/handlers/ec2/vpc/service_impl.go +++ b/spinifex/handlers/ec2/vpc/service_impl.go @@ -446,7 +446,7 @@ func (s *VPCServiceImpl) CreateSubnet(input *ec2.CreateSubnetInput, accountID st // Calculate available IPs (total hosts minus AWS reserved: network, router, DNS, future, broadcast) // ones is validated to be 16-28 above, so (32-ones) is always 4-16 and safe for uint conversion - totalHosts := max((1<<(32-ones))-5, 0) + totalHosts := max((1<<(32-ones))-5, 0) //#nosec G115 - ones validated 16-28 record := SubnetRecord{ SubnetId: subnetID, @@ -575,7 +575,7 @@ func (s *VPCServiceImpl) DescribeSubnets(input *ec2.DescribeSubnetsInput, accoun availableIPs := 0 if err == nil { ones, _ := subnetNet.Mask.Size() - availableIPs = max((1<<(32-ones))-5, 0) + availableIPs = max((1<<(32-ones))-5, 0) //#nosec G115 - ones from validated CIDR } subnets = append(subnets, s.subnetRecordToEC2(&record, availableIPs, accountID)) diff --git a/spinifex/objectstore/objectstore.go b/spinifex/objectstore/objectstore.go index e28e8f867..e5111e711 100644 --- a/spinifex/objectstore/objectstore.go +++ b/spinifex/objectstore/objectstore.go @@ -5,8 +5,10 @@ package objectstore import ( "bytes" + "crypto/tls" "errors" "io" + "net/http" "strings" "sync" @@ -55,11 +57,22 @@ type ObjectStore interface { // NewS3ObjectStoreFromConfig creates an S3ObjectStore from Predastore connection parameters, // eliminating the duplicated TLS+HTTP/2+session boilerplate in each service. func NewS3ObjectStoreFromConfig(host, region, accessKey, secretKey string) *S3ObjectStore { + tr := &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, // Skip TLS verification for self-signed certs + NextProtos: []string{"h2", "http/1.1"}, + }, + ForceAttemptHTTP2: true, + } + + httpClient := &http.Client{Transport: tr} + sess := session.Must(session.NewSession(&aws.Config{ Endpoint: aws.String(host), Region: aws.String(region), Credentials: credentials.NewStaticCredentials(accessKey, secretKey, ""), S3ForcePathStyle: aws.Bool(true), + HTTPClient: httpClient, })) return NewS3ObjectStore(s3.New(sess)) diff --git a/spinifex/services/vpcd/topology.go b/spinifex/services/vpcd/topology.go index b203cc38e..6eca46c08 100644 --- a/spinifex/services/vpcd/topology.go +++ b/spinifex/services/vpcd/topology.go @@ -1178,7 +1178,7 @@ func generateMAC(resourceID string) string { // Simple hash: use first 6 hex chars of resource ID after the prefix h := uint32(0) for _, c := range resourceID { - h = h*31 + uint32(c) //nolint:gosec // G115: intentional overflow for hashing + h = h*31 + uint32(c) // #nosec G115 -- intentional overflow for hashing } return fmt.Sprintf("02:00:00:%02x:%02x:%02x", (h>>16)&0xff, (h>>8)&0xff, h&0xff) } diff --git a/spinifex/utils/utils.go b/spinifex/utils/utils.go index 57cf63b8c..6a65af27e 100644 --- a/spinifex/utils/utils.go +++ b/spinifex/utils/utils.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "crypto/rand" + "crypto/tls" "encoding/hex" "encoding/json" "encoding/xml" @@ -193,12 +194,23 @@ func ValidateKeyPairName(name string) error { } func CreateS3Client(cfg *config.Config) *s3.S3 { + tr := &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + NextProtos: []string{"h2", "http/1.1"}, + }, + ForceAttemptHTTP2: true, + } + + httpClient := &http.Client{Transport: tr} + sess := session.Must(session.NewSession(&aws.Config{ Region: aws.String(cfg.Predastore.Region), Endpoint: aws.String(fmt.Sprintf("https://%s", cfg.Predastore.Host)), Credentials: credentials.NewStaticCredentials(cfg.Predastore.AccessKey, cfg.Predastore.SecretKey, ""), S3ForcePathStyle: aws.Bool(true), DisableSSL: aws.Bool(false), + HTTPClient: httpClient, })) return s3.New(sess) diff --git a/tests/e2e/lib/multinode-helpers.sh b/tests/e2e/lib/multinode-helpers.sh index bf48ad814..d4373424d 100755 --- a/tests/e2e/lib/multinode-helpers.sh +++ b/tests/e2e/lib/multinode-helpers.sh @@ -752,7 +752,7 @@ init_leader_node() { # Wait for formation server to be ready echo "Waiting for formation server..." for i in $(seq 1 60); do - if curl -sk "https://${NODE1_IP}:${CLUSTER_PORT}/formation/health" > /dev/null 2>&1; then + if curl -s "http://${NODE1_IP}:${CLUSTER_PORT}/formation/health" > /dev/null 2>&1; then echo " Formation server is ready (PID: $LEADER_INIT_PID)" return 0 fi