Skip to content

Firmware validation: save work from Vince#52

Draft
ofaurax wants to merge 6 commits intomainfrom
ofa-validation-integration
Draft

Firmware validation: save work from Vince#52
ofaurax wants to merge 6 commits intomainfrom
ofa-validation-integration

Conversation

@ofaurax
Copy link
Copy Markdown
Contributor

@ofaurax ofaurax commented Jan 17, 2025

From Vince:

I refactored the firmware validation function in flipflop to use a "low-level" Supermicro handle. The structure of code is reworked a bit as well, and I defined some internal interfaces to make mocking and testing a little easier. I also added a test for our firmware validation routine.

@ofaurax ofaurax mentioned this pull request Jan 17, 2025
@ofaurax ofaurax force-pushed the ofa-validation-integration branch from 4b72a4b to 96e0bd2 Compare January 29, 2025 10:40
func defaultBMCTransport() *http.Transport {
return &http.Transport{
//nolint:gosec // BMCs use self-signed certs
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},

Check failure

Code scanning / CodeQL

Disabled TLS certificate check

InsecureSkipVerify should not be used in production code.

Copilot Autofix

AI about 1 year ago

To fix the problem, we need to ensure that TLS certificate verification is enabled. This can be done by setting InsecureSkipVerify to false and properly configuring the application to trust the self-signed certificates used by the BMCs. This involves adding the self-signed certificates to the application's trusted certificate pool.

  1. Modify the defaultBMCTransport function to set InsecureSkipVerify to false.
  2. Load the self-signed certificates and add them to the RootCAs of the tls.Config.
Suggested changeset 1
internal/flipflop/validation_client.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/internal/flipflop/validation_client.go b/internal/flipflop/validation_client.go
--- a/internal/flipflop/validation_client.go
+++ b/internal/flipflop/validation_client.go
@@ -8,2 +8,5 @@
 	"crypto/tls"
+	"crypto/x509"
+	"io/ioutil"
+	"log"
 	"net"
@@ -20,5 +23,17 @@
 func defaultBMCTransport() *http.Transport {
+	// Load self-signed certificates
+	certPool := x509.NewCertPool()
+	cert, err := ioutil.ReadFile("/path/to/self-signed-cert.pem")
+	if err != nil {
+		log.Fatalf("Failed to read self-signed certificate: %v", err)
+	}
+	if ok := certPool.AppendCertsFromPEM(cert); !ok {
+		log.Fatalf("Failed to append self-signed certificate to pool")
+	}
+
 	return &http.Transport{
-		//nolint:gosec // BMCs use self-signed certs
-		TLSClientConfig:   &tls.Config{InsecureSkipVerify: true},
+		TLSClientConfig: &tls.Config{
+			InsecureSkipVerify: false,
+			RootCAs:            certPool,
+		},
 		DisableKeepAlives: true,
EOF
@@ -8,2 +8,5 @@
"crypto/tls"
"crypto/x509"
"io/ioutil"
"log"
"net"
@@ -20,5 +23,17 @@
func defaultBMCTransport() *http.Transport {
// Load self-signed certificates
certPool := x509.NewCertPool()
cert, err := ioutil.ReadFile("/path/to/self-signed-cert.pem")
if err != nil {
log.Fatalf("Failed to read self-signed certificate: %v", err)
}
if ok := certPool.AppendCertsFromPEM(cert); !ok {
log.Fatalf("Failed to append self-signed certificate to pool")
}

return &http.Transport{
//nolint:gosec // BMCs use self-signed certs
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
TLSClientConfig: &tls.Config{
InsecureSkipVerify: false,
RootCAs: certPool,
},
DisableKeepAlives: true,
Copilot is powered by AI and may make mistakes. Always verify output.
@ofaurax ofaurax force-pushed the ofa-validation-integration branch from 96e0bd2 to 894b95f Compare January 29, 2025 10:42
I imported a pre-release version of bmclib to get access to the new
versions of the supermicro client object. Using that I defined some
minimalist interfaces and used mockery to provide test collateral.
@ofaurax ofaurax force-pushed the ofa-validation-integration branch from 894b95f to 0f28e86 Compare February 4, 2025 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants