fix(initdata): align cert and algorithm validation with rustls rules#44
Merged
Merged
Conversation
Contributor
bpradipt
commented
May 2, 2026
bpradipt
commented
- validateCerts now branches on IsCA: CA certs require keyCertSign, leaf certs require SAN and extendedKeyUsage=serverAuth and must not be self-signed
- Add validateCACerts for callers that only accept CA certs (--cacert / --capath); create command now uses it so leaf certs are correctly rejected even if they carry a SAN and serverAuth EKU
- Accept sha384 and sha512 in addition to sha256 for the algorithm field
- validate command prints a per-cert report: type (CA/leaf), self-signed flag, issuer, validity window with expiry warning, key algorithm and size, SAN, EKU, fingerprint, and source field within initdata
- warn when KBS URLs differ across aa.toml token_configs and cdh.toml kbc entries; warning goes to stderr so the dump|validate pipeline is not polluted
- Update README to document the expanded algorithm set, rustls cert rules, and the URL consistency warning
- Add tests covering all new validation and reporting paths
There was a problem hiding this comment.
Pull request overview
This PR expands the initdata CLI’s validation and diagnostics so users get stricter certificate checks, broader algorithm acceptance, and clearer visibility into embedded KBS-related configuration. It mainly affects the cmd/initdata validation/create flows, with supporting updates in the shared initdata package, tests, and README.
Changes:
- Broaden
algorithmvalidation to acceptsha256,sha384, andsha512. - Split certificate handling into CA-only vs mixed CA/leaf validation paths, and add per-certificate reporting plus KBS URL mismatch warnings.
- Update
initdata createto reject non-CA certs for--cacert/--capath, and extend tests/documentation around the new behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/initdata/initdata.go |
Adds the shared allow-list/helper for accepted initdata algorithms. |
cmd/initdata/validate_test.go |
Adds tests for KBS URL mismatch detection and related validate behavior. |
cmd/initdata/validate.go |
Expands validation rules, emits KBS URL warnings, and prints per-cert reports. |
cmd/initdata/create.go |
Switches create-time CA inputs to CA-only validation. |
cmd/initdata/common_test.go |
Adds coverage for new CA/leaf validation behavior and source-aware cert extraction. |
cmd/initdata/common.go |
Implements CA-only helper, leaf validation, and source-tracking for extracted certs. |
README.md |
Documents the broader algorithm allow-list and new validation/warning behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+108
to
+113
| // validateLeafCert checks rustls rules for end-entity (non-CA) certificates: | ||
| // must not be self-signed, must carry a SubjectAltName, and must have | ||
| // extendedKeyUsage serverAuth. | ||
| func validateLeafCert(cert *x509.Certificate) error { | ||
| // rustls rejects self-signed end-entity certificates | ||
| if bytes.Equal(cert.RawIssuer, cert.RawSubject) { |
Comment on lines
+167
to
+175
| selfSigned := bytes.Equal(cert.RawIssuer, cert.RawSubject) | ||
|
|
||
| typeLabel := "leaf" | ||
| if cert.IsCA { | ||
| typeLabel = "CA" | ||
| } | ||
| if selfSigned { | ||
| typeLabel += " · self-signed" | ||
| } |
Comment on lines
265
to
+268
| fp := base64.StdEncoding.EncodeToString(cert.Raw) | ||
| if !seen[fp] { | ||
| seen[fp] = true | ||
| all = append(all, cert) | ||
| all = append(all, certEntry{cert: cert, source: ps.source}) |
Comment on lines
+139
to
+145
| first := entries[0].url | ||
| for _, e := range entries[1:] { | ||
| if e.url != first { | ||
| var sb strings.Builder | ||
| sb.WriteString("WARNING: KBS URLs differ across configurations — verify this is intentional:\n") | ||
| for _, e := range entries { | ||
| fmt.Fprintf(&sb, " %-44s %s\n", e.source+":", e.url) |
| for _, ps := range pemSources { | ||
| certs, err := parsePEMCerts([]byte(ps.pem)) | ||
| if err != nil { | ||
| return nil, err |
Comment on lines
+62
to
+63
| if !pkginitdata.IsValidAlgorithm(id.Algorithm) { | ||
| failures = append(failures, fmt.Sprintf("algorithm: got %q, want one of %v", id.Algorithm, pkginitdata.ValidAlgorithms)) |
Comment on lines
+71
to
+72
| if warn := checkKBSURLMismatch(id.Data); warn != "" { | ||
| fmt.Fprint(os.Stderr, warn) |
Comment on lines
+133
to
+136
| if cert.IsCA { | ||
| err = validateCACert(cert) | ||
| } else { | ||
| err = validateLeafCert(cert) |
bpradipt
force-pushed
the
initdata-validate
branch
from
196ec9a to
bc5d444
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+160
to
+164
| func reportCerts(entries []certEntry) { | ||
| caCount, leafCount := 0, 0 | ||
| for _, e := range entries { | ||
| if e.cert.IsCA { | ||
| caCount++ |
Comment on lines
+116
to
+120
| if tc, ok := aa["token_configs"].(map[string]interface{}); ok { | ||
| for name, v := range tc { | ||
| if entry, ok := v.(map[string]interface{}); ok { | ||
| if url, ok := entry["url"].(string); ok && url != "" { | ||
| entries = append(entries, urlEntry{"aa.toml/token_configs." + name, normalizeURL(url)}) |
Comment on lines
79
to
83
| certs := make([]*x509.Certificate, len(entries)) | ||
| for i, e := range entries { | ||
| certs[i] = e.cert | ||
| } | ||
| if err := validateCerts(certs); err != nil { |
Comment on lines
+59
to
60
| if err := validateCACerts(certs); err != nil { | ||
| return err |
Comment on lines
256
to
+260
| if tc, ok := aa["token_configs"].(map[string]interface{}); ok { | ||
| for _, v := range tc { | ||
| for name, v := range tc { | ||
| if entry, ok := v.(map[string]interface{}); ok { | ||
| if cert, ok := entry["cert"].(string); ok && cert != "" { | ||
| pemStrings = append(pemStrings, cert) | ||
| pemSources = append(pemSources, pemSource{cert, "aa.toml/token_configs." + name}) |
bpradipt
force-pushed
the
initdata-validate
branch
from
bc5d444 to
57edb3c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+180
to
+193
| // validateCertsBySource applies rustls rules to each certificate and includes | ||
| // the source field in error messages so the user can locate the problematic cert. | ||
| // All initdata cert fields accept any cert valid under rustls rules: | ||
| // CA certs require keyCertSign; leaf certs require SAN + serverAuth EKU and | ||
| // must not be self-signed. | ||
| func validateCertsBySource(entries []certEntry) error { | ||
| var errs []string | ||
| for _, e := range entries { | ||
| var err error | ||
| if e.cert.IsCA { | ||
| err = validateCACert(e.cert) | ||
| } else { | ||
| err = validateLeafCert(e.cert) | ||
| } |
Comment on lines
+77
to
+88
| // When neither flag is set, GenerateRaw falls back to cfg.TrusteeCACert. | ||
| // Validate it here so a leaf cert in the config file is caught early. | ||
| if createCACert == "" && createCAPath == "" && cfg.TrusteeCACert != "" { | ||
| certs, err := loadCerts(cfg.TrusteeCACert) | ||
| if err != nil { | ||
| return fmt.Errorf("trustee_ca_cert: %w", err) | ||
| } | ||
| if len(certs) > 0 { | ||
| if err := validateCACerts(certs); err != nil { | ||
| return fmt.Errorf("trustee_ca_cert in config: %w", err) | ||
| } | ||
| } |
Comment on lines
+118
to
+121
| for _, name := range names { | ||
| if entry, ok := tc[name].(map[string]interface{}); ok { | ||
| if url, ok := entry["url"].(string); ok && url != "" { | ||
| entries = append(entries, urlEntry{"aa.toml/token_configs." + name, normalizeURL(url)}) |
Comment on lines
+104
to
+114
| // validateCACert checks that cert is a valid CA certificate: IsCA must be true | ||
| // and KeyUsageCertSign must be set. The IsCA guard is intentional — this | ||
| // function may be called directly (e.g. from validateCACerts) without the | ||
| // IsCA pre-filter that validateCerts applies. | ||
| func validateCACert(cert *x509.Certificate) error { | ||
| if !cert.IsCA { | ||
| return fmt.Errorf("certificate %q: IsCA is false", cert.Subject.CommonName) | ||
| } | ||
| if err := checkExpiry(cert); err != nil { | ||
| return err | ||
| } |
| fp := sha256.Sum256(cert.Raw) | ||
| fingerprint := fmt.Sprintf("%X", fp[:6]) // first 6 bytes for brevity | ||
|
|
||
| fmt.Printf(" [%d] %s [%s]\n", i+1, cert.Subject.CommonName, typeLabel) |
| - Required keys `aa.toml` and `cdh.toml` are present (`policy.rego` is optional) | ||
| - Any CA certificates embedded in `aa.toml` or `cdh.toml` pass rustls compatibility checks | ||
| - Embedded certificates pass rustls rules: CA certs must have `keyCertSign`; leaf certs must have a SubjectAltName and `extendedKeyUsage=serverAuth` and must not be self-signed | ||
| - KBS URLs are consistent across `aa.toml` token configs and `cdh.toml` kbc (a warning is printed if they differ) |
Comment on lines
+25
to
+30
| // ValidAlgorithms lists all hash algorithms accepted during initdata validation. | ||
| var ValidAlgorithms = []string{"sha256", "sha384", "sha512"} | ||
|
|
||
| // IsValidAlgorithm reports whether alg is an accepted initdata algorithm. | ||
| func IsValidAlgorithm(alg string) bool { | ||
| for _, v := range ValidAlgorithms { |
- Branch on IsCA: CA certs require keyCertSign; leaf certs require SAN, serverAuth EKU, and must not be self-signed - Add isSelfSigned() using signature verification, not just DN comparison - Reject expired and not-yet-valid certificates - Accept sha384 and sha512 in addition to sha256 - Validate cfg.TrusteeCACert from config when no --cacert/--capath flag; pass validated PEM to GenerateRaw so the raw file is not re-read and extra non-CERTIFICATE blocks cannot slip into initdata - validate prints a per-cert report; cert display name falls back to SAN when CN is empty; warns to stderr when token config URLs differ - Deduplicate certs across fields by fingerprint, accumulating sources; repeated certs in one PEM bundle do not produce duplicate source labels - Add testdata/gen/main.go to generate long-lived cert fixtures (private keys in memory only, never written to disk); add //go:generate directive - Add fixtures: valid-with-ca-cert, valid-with-leaf-cert, valid-with-both, invalid-expired-cert, invalid-leaf-no-san and use then for tests Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>
bpradipt
force-pushed
the
initdata-validate
branch
from
57edb3c to
1b43b19
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+153
to
+158
| for _, eku := range cert.ExtKeyUsage { | ||
| if eku == x509.ExtKeyUsageServerAuth { | ||
| return nil | ||
| } | ||
| } | ||
| return fmt.Errorf("certificate %q: missing extendedKeyUsage serverAuth", cert.Subject.CommonName) |
Comment on lines
+113
to
+122
| names := make([]string, 0, len(tc)) | ||
| for name := range tc { | ||
| names = append(names, name) | ||
| } | ||
| sort.Strings(names) | ||
| for _, name := range names { | ||
| if entry, ok := tc[name].(map[string]interface{}); ok { | ||
| if url, ok := entry["url"].(string); ok && url != "" { | ||
| entries = append(entries, urlEntry{"aa.toml/token_configs." + name, normalizeURL(url)}) | ||
| } |
Comment on lines
+233
to
+243
| func certExpiryNote(cert *x509.Certificate) string { | ||
| now := time.Now() | ||
| if now.After(cert.NotAfter) { | ||
| days := int(now.Sub(cert.NotAfter).Hours() / 24) | ||
| return fmt.Sprintf("EXPIRED %d days ago", days) | ||
| } | ||
| days := int(cert.NotAfter.Sub(now).Hours() / 24) | ||
| if days < 30 { | ||
| return fmt.Sprintf("%d days remaining — WARNING", days) | ||
| } | ||
| return fmt.Sprintf("%d days remaining", days) |
Comment on lines
+82
to
+93
| if createCACert == "" && createCAPath == "" && cfg.TrusteeCACert != "" { | ||
| certs, err := loadCerts(cfg.TrusteeCACert) | ||
| if err != nil { | ||
| return fmt.Errorf("trustee_ca_cert: %w", err) | ||
| } | ||
| if len(certs) == 0 { | ||
| return fmt.Errorf("trustee_ca_cert %s: no certificates found", cfg.TrusteeCACert) | ||
| } | ||
| if err := validateCACerts(certs); err != nil { | ||
| return fmt.Errorf("trustee_ca_cert in config: %w", err) | ||
| } | ||
| certPEM = certsToPEM(certs) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.