From c5247bdee52d8c652bf36e4d22de95722c2e29b2 Mon Sep 17 00:00:00 2001 From: Pavel Okhlopkov Date: Mon, 1 Jun 2026 18:01:31 +0300 Subject: [PATCH] add cert validate Signed-off-by: Pavel Okhlopkov --- internal/module/module.go | 2 + pkg/config.go | 11 +- pkg/config/linters_settings.go | 8 +- pkg/linters/hooks/README.md | 82 +++++++ pkg/linters/hooks/hooks.go | 3 + .../hooks/rules/testdata/tls/invalid_usage.go | 9 + .../hooks/rules/testdata/tls/no_import.go | 12 ++ pkg/linters/hooks/rules/testdata/tls/valid.go | 11 + .../hooks/rules/testdata/tls/with_groups.go | 11 + pkg/linters/hooks/rules/tls_certificate.go | 203 ++++++++++++++++++ .../hooks/rules/tls_certificate_test.go | 87 ++++++++ 11 files changed, 435 insertions(+), 4 deletions(-) create mode 100644 pkg/linters/hooks/rules/testdata/tls/invalid_usage.go create mode 100644 pkg/linters/hooks/rules/testdata/tls/no_import.go create mode 100644 pkg/linters/hooks/rules/testdata/tls/valid.go create mode 100644 pkg/linters/hooks/rules/testdata/tls/with_groups.go create mode 100644 pkg/linters/hooks/rules/tls_certificate.go create mode 100644 pkg/linters/hooks/rules/tls_certificate_test.go diff --git a/internal/module/module.go b/internal/module/module.go index 1a96b138..1648933c 100644 --- a/internal/module/module.go +++ b/internal/module/module.go @@ -372,6 +372,7 @@ func mapSimpleLinterRules(linterSettings *pkg.LintersSettings, configSettings *c // Hooks rules linterSettings.Hooks.Rules.HooksRule.SetLevel("", configSettings.Hooks.Impact) + linterSettings.Hooks.Rules.TLSCertificateRule.SetLevel("", configSettings.Hooks.Impact) } // mapExclusionRulesAndSettings maps exclusion rules and additional linter settings @@ -469,6 +470,7 @@ func mapRBACExclusions(linterSettings *pkg.LintersSettings, configSettings *conf // mapHooksSettings maps Hooks linter settings func mapHooksSettings(linterSettings *pkg.LintersSettings, configSettings *config.LintersSettings) { linterSettings.Hooks.IngressRuleSettings.Disable = configSettings.Hooks.Ingress.Disable + linterSettings.Hooks.TLSCertificateRuleSettings.Disable = configSettings.Hooks.TLSCertificate.Disable } // mapModuleExclusionsAndSettings maps Module linter exclusions and settings diff --git a/pkg/config.go b/pkg/config.go index 3a307a70..d4bbf92f 100644 --- a/pkg/config.go +++ b/pkg/config.go @@ -195,15 +195,20 @@ type RBACExcludeRules struct { } type HooksLinterConfig struct { LinterConfig - Rules HooksLinterRules - IngressRuleSettings IngressRuleSettings + Rules HooksLinterRules + IngressRuleSettings IngressRuleSettings + TLSCertificateRuleSettings TLSCertificateRuleSettings } type HooksLinterRules struct { - HooksRule RuleConfig + HooksRule RuleConfig + TLSCertificateRule RuleConfig } type IngressRuleSettings struct { Disable bool } +type TLSCertificateRuleSettings struct { + Disable bool +} type ModuleLinterConfig struct { LinterConfig diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 9c118dee..a5352dca 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -74,7 +74,8 @@ type ContainerExcludeRules struct { } type HooksSettings struct { - Ingress HooksIngressRuleSetting `mapstructure:"ingress"` + Ingress HooksIngressRuleSetting `mapstructure:"ingress"` + TLSCertificate HooksTLSCertificateRuleSetting `mapstructure:"tls-certificate"` Impact string `mapstructure:"impact"` } @@ -84,6 +85,11 @@ type HooksIngressRuleSetting struct { Disable bool `mapstructure:"disable"` } +type HooksTLSCertificateRuleSetting struct { + // disable tls-certificate rule completely + Disable bool `mapstructure:"disable"` +} + type ImageSettings struct { ExcludeRules ImageExcludeRules `mapstructure:"exclude-rules"` diff --git a/pkg/linters/hooks/README.md b/pkg/linters/hooks/README.md index 7685a8fc..5bf2d527 100644 --- a/pkg/linters/hooks/README.md +++ b/pkg/linters/hooks/README.md @@ -11,6 +11,7 @@ Hooks are Go or Python scripts that react to Kubernetes resource changes and imp | Rule | Description | Configurable | Default | |------|-------------|--------------|---------| | [ingress](#ingress) | Validates copy_custom_certificate hook presence for Ingress resources | ✅ | enabled | +| [tls-certificate](#tls-certificate) | Detects invalid self-signed certificate generation in Go hooks | ✅ | enabled | ## Rule Details @@ -104,6 +105,85 @@ linters-settings: disable: true ``` +### tls-certificate + +**Purpose:** Detects places in Go hooks that generate invalid self-signed TLS certificates using the `go_lib/hooks/tls_certificate` helpers (`RegisterInternalTLSHook` / `GenerateSelfSignedCert`). + +**Description:** + +Self-signed certificates produced by these helpers had defects that caused validation failures outside of Go/kube-apiserver (`openssl verify`, Trivy, Java keystores, MaxPatrol). This rule statically scans Go hook source for the known causes, based on [deckhouse/deckhouse#20138](https://github.com/deckhouse/deckhouse/pull/20138). + +**What it checks:** + +The rule only inspects `.go` files in the module's `hooks/` directory that import: + +```go +"github.com/deckhouse/deckhouse/go_lib/hooks/tls_certificate" +``` + +For those files it reports: + +1. **Bogus `"requestheader-client"` usage.** This string does not exist in cfssl's KeyUsage/ExtKeyUsage maps, so cfssl silently discards it and emits a certificate with an empty `ExtendedKeyUsage` extension. Strict validators require at least `serverAuth`. Use `"server auth"` instead. +2. **`WithGroups` on a leaf certificate.** Passing `WithGroups(...)` into a `GenerateSelfSignedCert(...)` call copies the CA's `Organization` onto the leaf, recreating the `Subject == Issuer` (depth-0 self-signed) collision that OpenSSL rejects with `X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT`. + +**Why it matters:** + +Certificates that pass Go's lenient verification still fail in OpenSSL, Trivy, and other strict validators, breaking webhooks and security scans in environments outside of kube-apiserver. + +**Examples:** + +❌ **Incorrect** - bogus usage that drops the EKU extension: + +```go +import "github.com/deckhouse/deckhouse/go_lib/hooks/tls_certificate" + +var _ = tls_certificate.RegisterInternalTLSHook(tls_certificate.GenSelfSignedTLSHookConf{ + Usages: []string{"requestheader-client"}, // no EKU is emitted + // ... +}) +``` + +❌ **Incorrect** - `WithGroups` on the leaf recreates Subject == Issuer: + +```go +cert, _ := tls_certificate.GenerateSelfSignedCert( + "leaf", + ca, + tls_certificate.WithGroups("Deckhouse"), // copies CA Organization onto the leaf +) +``` + +✅ **Correct**: + +```go +var _ = tls_certificate.RegisterInternalTLSHook(tls_certificate.GenSelfSignedTLSHookConf{ + Usages: []string{"server auth"}, + // ... +}) + +cert, _ := tls_certificate.GenerateSelfSignedCert("leaf", ca) +``` + +**Error:** + +``` +Invalid certificate usage "requestheader-client" produces a certificate with an empty ExtendedKeyUsage extension and is rejected by strict validators. Use "server auth" instead. +``` + +``` +WithGroups applied to a leaf certificate via GenerateSelfSignedCert copies the CA Organization onto the leaf, recreating the Subject == Issuer (depth-0 self-signed) collision rejected by OpenSSL. Remove WithGroups from the leaf certificate. +``` + +**Configuration:** + +```yaml +# .dmt.yaml +linters-settings: + hooks: + tls-certificate: + disable: false # Enable/disable the tls-certificate rule +``` + ## Configuration The Hooks linter can be configured at both the module level and for individual rules. @@ -150,6 +230,8 @@ linters-settings: # Rule-specific settings ingress: disable: false + tls-certificate: + disable: false ``` ### Configuration in Module Directory diff --git a/pkg/linters/hooks/hooks.go b/pkg/linters/hooks/hooks.go index 8a2bccff..8feb11f8 100644 --- a/pkg/linters/hooks/hooks.go +++ b/pkg/linters/hooks/hooks.go @@ -52,6 +52,9 @@ func (h *Hooks) Run(m *module.Module) { for _, object := range m.GetStorage() { r.CheckIngressCopyCustomCertificateRule(m, object, errorList) } + + rules.NewTLSCertificateRule(h.cfg.TLSCertificateRuleSettings.Disable). + CheckTLSCertificateHooks(m, errorList.WithMaxLevel(h.cfg.Rules.TLSCertificateRule.GetLevel())) } func (h *Hooks) Name() string { diff --git a/pkg/linters/hooks/rules/testdata/tls/invalid_usage.go b/pkg/linters/hooks/rules/testdata/tls/invalid_usage.go new file mode 100644 index 00000000..9e2f218a --- /dev/null +++ b/pkg/linters/hooks/rules/testdata/tls/invalid_usage.go @@ -0,0 +1,9 @@ +package tls + +import ( + "github.com/deckhouse/deckhouse/go_lib/hooks/tls_certificate" +) + +var _ = tls_certificate.RegisterInternalTLSHook(tls_certificate.GenSelfSignedTLSHookConf{ + Usages: []string{"requestheader-client"}, +}) diff --git a/pkg/linters/hooks/rules/testdata/tls/no_import.go b/pkg/linters/hooks/rules/testdata/tls/no_import.go new file mode 100644 index 00000000..c1472a12 --- /dev/null +++ b/pkg/linters/hooks/rules/testdata/tls/no_import.go @@ -0,0 +1,12 @@ +package tls + +// This file uses the same bogus strings/options but does NOT import the +// tls_certificate library, so the rule must skip it entirely. + +func WithGroups(string) string { return "" } + +func GenerateSelfSignedCert(args ...any) string { return "" } + +var usages = []string{"requestheader-client"} + +var skipped = GenerateSelfSignedCert("leaf", WithGroups("Deckhouse")) diff --git a/pkg/linters/hooks/rules/testdata/tls/valid.go b/pkg/linters/hooks/rules/testdata/tls/valid.go new file mode 100644 index 00000000..f0480a6c --- /dev/null +++ b/pkg/linters/hooks/rules/testdata/tls/valid.go @@ -0,0 +1,11 @@ +package tls + +import ( + "github.com/deckhouse/deckhouse/go_lib/hooks/tls_certificate" +) + +var _ = tls_certificate.RegisterInternalTLSHook(tls_certificate.GenSelfSignedTLSHookConf{ + Usages: []string{"server auth"}, +}) + +var validLeaf, _ = tls_certificate.GenerateSelfSignedCert("leaf", nil) diff --git a/pkg/linters/hooks/rules/testdata/tls/with_groups.go b/pkg/linters/hooks/rules/testdata/tls/with_groups.go new file mode 100644 index 00000000..5c244cf3 --- /dev/null +++ b/pkg/linters/hooks/rules/testdata/tls/with_groups.go @@ -0,0 +1,11 @@ +package tls + +import ( + "github.com/deckhouse/deckhouse/go_lib/hooks/tls_certificate" +) + +var leaf, _ = tls_certificate.GenerateSelfSignedCert( + "leaf", + nil, + tls_certificate.WithGroups("Deckhouse"), +) diff --git a/pkg/linters/hooks/rules/tls_certificate.go b/pkg/linters/hooks/rules/tls_certificate.go new file mode 100644 index 00000000..000444fa --- /dev/null +++ b/pkg/linters/hooks/rules/tls_certificate.go @@ -0,0 +1,203 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rules + +import ( + "go/ast" + "go/parser" + "go/token" + "path/filepath" + + "k8s.io/utils/ptr" + + "github.com/deckhouse/dmt/internal/fsutils" + "github.com/deckhouse/dmt/internal/module" + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +const ( + TLSCertificateRuleName = "tls-certificate" + + // tlsCertificateImport is the go_lib package whose helpers + // (RegisterInternalTLSHook / GenerateSelfSignedCert) are known to + // produce invalid self-signed certificates when misused. + // See https://github.com/deckhouse/deckhouse/pull/20138. + tlsCertificateImport = `"github.com/deckhouse/deckhouse/go_lib/hooks/tls_certificate"` + + // invalidUsage is a bogus usage string that does not exist in cfssl's + // KeyUsage/ExtKeyUsage maps. cfssl silently discards it, emitting a + // certificate with no ExtendedKeyUsage extension which strict validators + // (Java keystores, Trivy, MaxPatrol) reject. + invalidUsage = "requestheader-client" + + // recommendedUsage is the EKU that must be present for server certificates. + recommendedUsage = "server auth" + + // generateSelfSignedCertFunc is the helper that issues a leaf certificate. + generateSelfSignedCertFunc = "GenerateSelfSignedCert" + + // withGroupsOption copies the CA's O= onto the leaf, recreating the + // Subject == Issuer (depth-0 self-signed) collision rejected by OpenSSL. + withGroupsOption = "WithGroups" +) + +type TLSCertificateRule struct { + pkg.RuleMeta + pkg.BoolRule +} + +func NewTLSCertificateRule(disable bool) *TLSCertificateRule { + return &TLSCertificateRule{ + RuleMeta: pkg.RuleMeta{ + Name: TLSCertificateRuleName, + }, + BoolRule: pkg.BoolRule{ + Exclude: disable, + }, + } +} + +// CheckTLSCertificateHooks scans the module's Go hooks for the self-signed +// certificate defects fixed in deckhouse/deckhouse#20138: +// +// 1. Bogus "requestheader-client" usage that results in an empty +// ExtendedKeyUsage extension instead of "server auth". +// 2. WithGroups applied to a leaf certificate, which copies the CA's +// Organization onto the leaf and recreates the Subject == Issuer +// (depth-0 self-signed) collision. +func (r *TLSCertificateRule) CheckTLSCertificateHooks(m *module.Module, errorList *errors.LintRuleErrorsList) { + errorList = errorList.WithRule(r.GetName()) + + if !r.Enabled() { + errorList = errorList.WithMaxLevel(ptr.To(pkg.Ignored)) + } + + modulePath := m.GetPath() + hooksDir := filepath.Join(modulePath, "hooks") + + for _, hookPath := range fsutils.GetFiles(hooksDir, false, filterGoHooks) { + r.checkFile(modulePath, hookPath, errorList) + } +} + +func (r *TLSCertificateRule) checkFile(modulePath, hookPath string, errorList *errors.LintRuleErrorsList) { + fSet := token.NewFileSet() + + astFile, err := parser.ParseFile(fSet, hookPath, nil, parser.AllErrors) + if err != nil { + // Parsing errors are reported by the compiler/other tooling, skip here. + return + } + + if !fileImportsTLSCertificate(astFile) { + return + } + + relPath := fsutils.Rel(modulePath, hookPath) + fileErrorList := errorList.WithFilePath(relPath) + + ast.Inspect(astFile, func(n ast.Node) bool { + switch node := n.(type) { + case *ast.BasicLit: + if node.Kind == token.STRING && stringLitValue(node) == invalidUsage { + fileErrorList. + WithLineNumber(fSet.Position(node.Pos()).Line). + WithValue(invalidUsage). + Errorf("Invalid certificate usage %q produces a certificate with an empty ExtendedKeyUsage "+ + "extension and is rejected by strict validators. Use %q instead.", invalidUsage, recommendedUsage) + } + case *ast.CallExpr: + if isCallNamed(node, generateSelfSignedCertFunc) { + if pos, ok := findOptionCall(node, withGroupsOption); ok { + fileErrorList. + WithLineNumber(fSet.Position(pos).Line). + WithValue(withGroupsOption). + Errorf("%s applied to a leaf certificate via %s copies the CA Organization onto the leaf, "+ + "recreating the Subject == Issuer (depth-0 self-signed) collision rejected by OpenSSL. "+ + "Remove %s from the leaf certificate.", withGroupsOption, generateSelfSignedCertFunc, withGroupsOption) + } + } + } + + return true + }) +} + +// fileImportsTLSCertificate reports whether the file imports the tls_certificate go_lib package. +func fileImportsTLSCertificate(astFile *ast.File) bool { + for _, imp := range astFile.Imports { + if imp.Path != nil && imp.Path.Value == tlsCertificateImport { + return true + } + } + + return false +} + +// stringLitValue returns the unquoted value of a string literal, or the raw +// value if it cannot be unquoted. +func stringLitValue(lit *ast.BasicLit) string { + if len(lit.Value) >= 2 { + first, last := lit.Value[0], lit.Value[len(lit.Value)-1] + if (first == '"' && last == '"') || (first == '`' && last == '`') { + return lit.Value[1 : len(lit.Value)-1] + } + } + + return lit.Value +} + +// isCallNamed reports whether call invokes a function or method with the given name. +func isCallNamed(call *ast.CallExpr, name string) bool { + switch fun := call.Fun.(type) { + case *ast.SelectorExpr: + return fun.Sel != nil && fun.Sel.Name == name + case *ast.Ident: + return fun.Name == name + } + + return false +} + +// findOptionCall searches the arguments of a call for a nested call to the +// option with the given name, returning its position when found. +func findOptionCall(call *ast.CallExpr, name string) (token.Pos, bool) { + var ( + pos token.Pos + found bool + ) + + for _, arg := range call.Args { + ast.Inspect(arg, func(n ast.Node) bool { + if found { + return false + } + + if inner, ok := n.(*ast.CallExpr); ok && isCallNamed(inner, name) { + pos = inner.Pos() + found = true + + return false + } + + return true + }) + } + + return pos, found +} diff --git a/pkg/linters/hooks/rules/tls_certificate_test.go b/pkg/linters/hooks/rules/tls_certificate_test.go new file mode 100644 index 00000000..364509a4 --- /dev/null +++ b/pkg/linters/hooks/rules/tls_certificate_test.go @@ -0,0 +1,87 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rules + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/deckhouse/dmt/pkg/errors" +) + +func TestTLSCertificateRule_CheckFile(t *testing.T) { + const fixtureDir = "testdata/tls" + + tests := []struct { + name string + fixture string + wantErrors int + wantSnippet string + }{ + { + name: "invalid requestheader-client usage", + fixture: "invalid_usage.go", + wantErrors: 1, + wantSnippet: "empty ExtendedKeyUsage", + }, + { + name: "WithGroups on leaf certificate", + fixture: "with_groups.go", + wantErrors: 1, + wantSnippet: "Subject == Issuer", + }, + { + name: "valid certificate generation", + fixture: "valid.go", + wantErrors: 0, + }, + { + name: "defects without tls_certificate import are skipped", + fixture: "no_import.go", + wantErrors: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + errorList := errors.NewLintRuleErrorsList() + rule := NewTLSCertificateRule(false) + + rule.checkFile(fixtureDir, filepath.Join(fixtureDir, tt.fixture), errorList) + + errs := errorList.GetErrors() + if len(errs) != tt.wantErrors { + t.Fatalf("expected %d errors, got %d: %+v", tt.wantErrors, len(errs), errs) + } + + if tt.wantSnippet != "" && !strings.Contains(errs[0].Text, tt.wantSnippet) { + t.Fatalf("expected error text to contain %q, got %q", tt.wantSnippet, errs[0].Text) + } + }) + } +} + +func TestTLSCertificateRule_Enabled(t *testing.T) { + if NewTLSCertificateRule(false).Enabled() != true { + t.Fatalf("expected rule to be enabled when disable=false") + } + + if NewTLSCertificateRule(true).Enabled() != false { + t.Fatalf("expected rule to be disabled when disable=true") + } +}