-
Notifications
You must be signed in to change notification settings - Fork 0
ROX-33250: Add support for self-singed CA #121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| {{- if and .Values.centralCACert.cert .Values.centralCACert.existingSecretName }} | ||
| {{- fail "centralCACert: cannot set both 'cert' and 'existingSecretName' — use one or the other" }} | ||
| {{- end }} | ||
| {{- if and .Values.centralCACert.cert (not .Values.centralCACert.existingSecretName) }} | ||
| apiVersion: v1 | ||
| kind: Secret | ||
| metadata: | ||
| name: {{ include "stackrox-mcp.fullname" . }}-central-ca | ||
| labels: | ||
| {{- include "stackrox-mcp.labels" . | nindent 4 }} | ||
| type: Opaque | ||
| data: | ||
| ca.crt: {{ .Values.centralCACert.cert | b64enc }} | ||
| {{- end }} |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -111,6 +111,11 @@ spec: | |||||||||||||||||||||||||||
| mountPath: /certs | ||||||||||||||||||||||||||||
| readOnly: true | ||||||||||||||||||||||||||||
| {{- end }} | ||||||||||||||||||||||||||||
| {{- if include "stackrox-mcp.centralCAEnabled" . }} | ||||||||||||||||||||||||||||
| - name: central-ca | ||||||||||||||||||||||||||||
| mountPath: /central-ca | ||||||||||||||||||||||||||||
| readOnly: true | ||||||||||||||||||||||||||||
| {{- end }} | ||||||||||||||||||||||||||||
| volumes: | ||||||||||||||||||||||||||||
| - name: config | ||||||||||||||||||||||||||||
| configMap: | ||||||||||||||||||||||||||||
|
|
@@ -121,6 +126,12 @@ spec: | |||||||||||||||||||||||||||
| secretName: {{ include "stackrox-mcp.tlsSecretName" . }} | ||||||||||||||||||||||||||||
| defaultMode: 0440 | ||||||||||||||||||||||||||||
| {{- end }} | ||||||||||||||||||||||||||||
| {{- if include "stackrox-mcp.centralCAEnabled" . }} | ||||||||||||||||||||||||||||
| - name: central-ca | ||||||||||||||||||||||||||||
| secret: | ||||||||||||||||||||||||||||
| secretName: {{ include "stackrox-mcp.centralCASecretName" . }} | ||||||||||||||||||||||||||||
| defaultMode: 0440 | ||||||||||||||||||||||||||||
|
Comment on lines
+129
to
+133
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make the existing CA secret key configurable.
🔧 Suggested change - name: central-ca
secret:
secretName: {{ include "stackrox-mcp.centralCASecretName" . }}
+ items:
+ - key: {{ default "ca.crt" .Values.centralCACert.existingSecretKey | quote }}
+ path: ca.crt
defaultMode: 0440 centralCACert:
existingSecretName: ""
+ existingSecretKey: "ca.crt"
cert: ""📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would avoid complicating configuration because this can be set with:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It would be worth adding a note in the chart's
|
||||||||||||||||||||||||||||
| {{- end }} | ||||||||||||||||||||||||||||
| {{- with .Values.nodeSelector }} | ||||||||||||||||||||||||||||
| nodeSelector: | ||||||||||||||||||||||||||||
| {{- toYaml . | nindent 8 }} | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,11 @@ package client | |
| import ( | ||
| "context" | ||
| "crypto/tls" | ||
| "crypto/x509" | ||
| "encoding/pem" | ||
| "fmt" | ||
| "log/slog" | ||
| "os" | ||
| "sync" | ||
| "testing" | ||
| "time" | ||
|
|
@@ -23,6 +27,7 @@ import ( | |
| const ( | ||
| minConnectTimeout = 5 * time.Second | ||
| backoffJitter = 0.2 | ||
| maxCACertFileSize = 1 << 20 // 1MB | ||
| ) | ||
|
|
||
| // Client provides gRPC connection to StackRox Central API. | ||
|
|
@@ -229,11 +234,112 @@ func (c *Client) tlsConfig() (*tls.Config, error) { | |
| return nil, errors.Wrap(err, "failed to get central URL hostname") | ||
| } | ||
|
|
||
| return &tls.Config{ | ||
| tlsCfg := &tls.Config{ | ||
| InsecureSkipVerify: c.config.InsecureSkipTLSVerify, //nolint:gosec | ||
| MinVersion: tls.VersionTLS12, | ||
| ServerName: hostname, | ||
| }, nil | ||
| } | ||
|
|
||
| // There is no reason to load certificates if we allow InsecureSkipTLSVerify. | ||
| if !c.config.InsecureSkipTLSVerify && c.config.CACertPath != "" { | ||
| certPool, err := loadCACertPool(c.config.CACertPath) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
Comment on lines
+243
to
+248
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrap CA-load failures with This new path can return raw filesystem/parsing errors from 🔧 Suggested change tlsConfig, err := c.tlsConfig()
if err != nil {
- return err
+ return NewError(err, "Connect")
}As per coding guidelines, "Proper error wrapping with client.NewError for user-facing errors". 🤖 Prompt for AI Agents |
||
|
|
||
| tlsCfg.RootCAs = certPool | ||
| } | ||
|
|
||
| return tlsCfg, nil | ||
| } | ||
|
|
||
| func loadCACertPool(caCertPath string) (*x509.CertPool, error) { | ||
| // File size guard | ||
| fileInfo, err := os.Stat(caCertPath) | ||
| if err != nil { | ||
| return nil, errors.Wrapf(err, "failed to access CA certificate at %s", caCertPath) | ||
| } | ||
|
|
||
| if !fileInfo.Mode().IsRegular() { | ||
| return nil, errors.Errorf("CA certificate path %s is not a regular file", caCertPath) | ||
| } | ||
|
|
||
| if fileInfo.Size() == 0 { | ||
| return nil, errors.Errorf("CA certificate file %s is empty", caCertPath) | ||
| } | ||
|
|
||
| if fileInfo.Size() > maxCACertFileSize { | ||
| return nil, errors.Errorf( | ||
| "CA certificate file %s is too large (%d bytes, max %d)", | ||
| caCertPath, fileInfo.Size(), | ||
| maxCACertFileSize, | ||
| ) | ||
| } | ||
|
mtodor marked this conversation as resolved.
|
||
|
|
||
| caCert, err := os.ReadFile(caCertPath) //nolint:gosec | ||
| if err != nil { | ||
| return nil, errors.Wrapf(err, "failed to read CA certificate from %s", caCertPath) | ||
| } | ||
|
|
||
| // Get system cert pool, warn on fallback | ||
| certPool, err := x509.SystemCertPool() | ||
| if err != nil { | ||
| slog.Warn("Failed to load system CA pool, using custom CA only", "error", err) | ||
|
|
||
| certPool = x509.NewCertPool() | ||
| } | ||
|
|
||
| if !certPool.AppendCertsFromPEM(caCert) { | ||
| return nil, errors.Errorf("failed to parse CA certificate from %s: no valid PEM data found", caCertPath) | ||
| } | ||
|
|
||
| showCertInfo(caCert) | ||
|
|
||
| return certPool, nil | ||
| } | ||
|
|
||
| // showCertInfo parses and logs certificate metadata. | ||
| func showCertInfo(caCert []byte) { | ||
| block, _ := pem.Decode(caCert) | ||
| if block == nil { | ||
| slog.Warn("Unable to decode CA certificate") | ||
|
|
||
| return | ||
| } | ||
|
|
||
| cert, err := x509.ParseCertificate(block.Bytes) | ||
| if err != nil { | ||
| slog.Warn("Failed to parse CA certificate", "error", err) | ||
|
|
||
| return | ||
| } | ||
|
|
||
| slog.Info("Loaded CA certificate", | ||
| "subject", cert.Subject.CommonName, | ||
| "issuer", cert.Issuer.CommonName, | ||
| "notAfter", cert.NotAfter, | ||
| "isCA", cert.IsCA, | ||
| ) | ||
|
|
||
| if !cert.IsCA { | ||
| slog.Warn("Provided certificate does not have the CA basic constraint set — TLS verification may fail", | ||
| "subject", cert.Subject.CommonName, | ||
| ) | ||
| } | ||
|
|
||
| if time.Now().After(cert.NotAfter) { | ||
| slog.Warn("CA certificate is expired — TLS verification will fail", | ||
| "subject", cert.Subject.CommonName, | ||
| "expiredAt", cert.NotAfter, | ||
| ) | ||
| } | ||
|
|
||
| if time.Now().Before(cert.NotBefore) { | ||
| slog.Warn("CA certificate is not yet valid", | ||
| "subject", cert.Subject.CommonName, | ||
| "validFrom", cert.NotBefore, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| func (c *Client) connectHTTP1( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.