diff --git a/AGENTS.md b/AGENTS.md index b2da9d2..6e4ef64 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,8 +31,8 @@ cmd/certmanager/main.go # CLI (flattened: the positional arg is the comman cmd/certmanager/main_test.go # end-to-end tests against an httptest Vault stub test/integration/ # end-to-end tests against a real Vault dev server internal/config/ # YAML config: parsing, defaults, validation -internal/pki/ # issue request/response + the JSON output shape -internal/vault/ # authenticated HTTP client (approle, kubernetes) +internal/pki/ # issue + ca_chain requests and the output shapes +internal/vault/ # HTTP client: approle, kubernetes, unauthenticated Makefile # build / test / lint / completions / version bump .goreleaser.yaml # single release driver: binaries + RPM + Gitea release scripts/integration-test.sh # runs the integration test against a Vault dev server @@ -46,7 +46,12 @@ dist/ # build output: binaries, RPM, checksums (not comm wires it to the real streams and `os.Exit`. Cobra runs with `SilenceUsage` and `SilenceErrors` so nothing reaches stderr unless the run failed. - **Flat CLI**: the root command takes `common_name` positionally - (`cobra.ExactArgs(1)`). Only `completion` and `help` are subcommands. + (`cobra.ExactArgs(1)`). `ca`, `completion` and `help` are the only + subcommands; `--config` is persistent so `ca` shares it. +- **`ca` is unauthenticated**: `GET {addr}/v1/{mount}/ca_chain` sends no + `X-Vault-Token` and loads the config through `config.LoadCA`, which requires + only `vault.addr`. The body is parsed as PEM, never sniffed — Vault labels it + `application/pkix-cert`. It prints raw PEM: Puppet uses it as file content. - **`parseList`** splits comma lists, trims whitespace, drops empty tokens. - **`config.Load`** applies defaults (`auth_method: approle`, `jwt_path`) then `Validate()`, which names the offending key and rejects an unknown @@ -59,8 +64,10 @@ dist/ # build output: binaries, RPM, checksums (not comm ## Deliberate deviations from the Python script -- Emits `ca_certificate`; the Python did not, so Puppet's - `/etc/pki/tls/vault/ca_certificate.crt` was silently never populated. +- The CA is served by `certmanager ca` from the mount's `/ca_chain`, not welded + to issuance. `issuing_ca` is the intermediate only, and deriving the CA from + an issue response would pin CA rotation to each node's 90 day renewal. +- `full_chain` is leaf first; the Python put the issuer first. - `output_path` is read from under `vault:` where it is actually written. The Python declared it at the top level but read it from the unwrapped `vault` dict, so it never resolved. diff --git a/README.md b/README.md index 846267a..fe364aa 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,8 @@ failure path exits non-zero. ## Usage ``` -certmanager [flags] +certmanager [flags] # issue a certificate +certmanager ca [flags] # print the CA chain ``` | Flag | Short | Type | Default | Description | @@ -31,6 +32,12 @@ certmanager [flags] | `--json` | | bool | `false` | Print the certificate data as JSON | | `--config` | | path | `/opt/certmanager/config.yaml` | Config file location | +`ca` takes `--config` plus: + +| Flag | Short | Type | Default | Description | +|---|---|---|---|---| +| `--mount` | `-m` | string | `vault.mount_point` | PKI mount to read | + Comma lists are split on `,`, whitespace-trimmed, and empty tokens dropped. ### JSON output @@ -39,15 +46,15 @@ Comma lists are split on `,`, whitespace-trimmed, and empty tokens dropped. { "certificate": "", "private_key": "", - "full_chain": "\n", - "ca_certificate": "" + "full_chain": "\n" } ``` -`ca_certificate` is new. `profiles::pki::vault` writes -`$cert_data['ca_certificate']` to `/etc/pki/tls/vault/ca_certificate.crt`, but -the Python never emitted the key, so Puppet saw `undef` and the file was never -populated. +`full_chain` is leaf first, the order every TLS server expects. + +The CA is not part of this document. It is estate-wide and public, so binding +it to a node's issuance would pin CA distribution to that node's renewal cycle; +`certmanager ca` fetches it instead. ### File output @@ -58,12 +65,40 @@ Without `--json` the certificate is written under `vault.output_path` |---|---| | `//certificate.crt` | Signed certificate | | `//private.key` | Private key (mode 0600) | -| `//full_chain.crt` | Issuing CA + certificate | -| `//ca_certificate.crt` | Issuing CA certificate | +| `//full_chain.crt` | Certificate + issuing CA | -With `-c` the same four files are written into +With `-c` the same three files are written into `/.zip`. +## The `ca` subcommand + +``` +certmanager ca [--mount ] [--config ] +``` + +Prints the mount's CA chain to stdout as raw PEM — not JSON, because Puppet +uses it as file content directly: + +```puppet +file { '/etc/pki/tls/vault/ca_certificate.crt': + content => generate('/usr/local/bin/certmanager', 'ca'), +} +``` + +It reads `GET {addr}/v1/{mount}/ca_chain` unauthenticated: no AppRole login, no +Kubernetes login, no token header. A config carrying only `addr` and +`mount_point` is enough, and `--mount` overrides the configured mount. + +`/ca_chain` returns the intermediate **and** the root, unlike `issuing_ca` or +`/ca/pem`, which return the intermediate alone. An intermediate-only `CAfile` +is not self-sufficient: strict path builders that require a chain to a +self-signed root reject it. + +The endpoint advertises `application/pkix-cert` while serving PEM, so the body +is parsed rather than sniffed. A non-200, an empty body or anything that is not +a PEM certificate chain exits non-zero with the cause on stderr and nothing on +stdout. + ## Configuration YAML, everything nested under `vault:`. The path is baked in at build time @@ -74,11 +109,11 @@ YAML, everything nested under `vault:`. The path is baked in at build time | `addr` | yes | | e.g. `https://vault.service.consul:8200` | | `auth_method` | no | `approle` | `approle` or `kubernetes` | | `mount_point` | yes | | e.g. `pki_int` | -| `role_name` | yes | | e.g. `servers_default` | -| `role_id` | iff `approle` | | | -| `approle_path` | iff `approle` | | e.g. `approle` | -| `k8s_mount` | iff `kubernetes` | | e.g. `k8s/au/syd1` | -| `k8s_role` | iff `kubernetes` | | e.g. `puppet_certmanager` | +| `role_name` | iff issuing | | e.g. `servers_default` | +| `role_id` | iff `approle` and issuing | | | +| `approle_path` | iff `approle` and issuing | | e.g. `approle` | +| `k8s_mount` | iff `kubernetes` and issuing | | e.g. `k8s/au/syd1` | +| `k8s_role` | iff `kubernetes` and issuing | | e.g. `puppet_certmanager` | | `jwt_path` | no | `/var/run/secrets/kubernetes.io/serviceaccount/token` | | | `output_path` | no | `.` | Non-JSON output directory | | `tls_skip_verify` | no | `true` | Set `false` once every node trusts the CA | @@ -95,6 +130,8 @@ vault: output_path: /tmp/certmanager ``` +`ca` requires only `addr` and a mount; every auth key above is optional for it. + AppRole logs in with `role_id` only — the Vault role sets `bind_secret_id: false`. Kubernetes reads the JWT from `jwt_path` and posts it with `k8s_role`. A missing, unreadable or empty JWT, a rejected login, or an @@ -115,7 +152,9 @@ make release-snapshot # goreleaser: binaries, tar.gz and RPM into dist/ `make integration-test` starts a `hashicorp/vault` dev container with docker/podman, provisions a PKI mount and an approle role, then runs the built -binary with the exact argv Puppet uses and asserts on the issued certificate. +binary with the exact argv Puppet uses and asserts on the issued certificate. A +root mount signs an intermediate so the CA chain under test is the two +certificate shape production serves. Point it at an existing dev server with `CERTMANAGER_IT_VAULT_ADDR` (`VAULT_ADDR` is deliberately ignored — the test writes to the PKI mount). diff --git a/cmd/certmanager/main.go b/cmd/certmanager/main.go index 434fa6f..0b1f622 100644 --- a/cmd/certmanager/main.go +++ b/cmd/certmanager/main.go @@ -79,13 +79,64 @@ func rootCmd() *cobra.Command { }, } - cmd.Flags().StringVar(&cfgFile, "config", config.DefaultPath(), "path to config.yaml") + // Persistent so the ca subcommand shares it; the positional form keeps + // parsing --config exactly as before. + cmd.PersistentFlags().StringVar(&cfgFile, "config", config.DefaultPath(), "path to config.yaml") cmd.Flags().StringVarP(&altNames, "alt-names", "a", "", "comma-separated DNS subject alternative names") cmd.Flags().StringVarP(&ipSans, "ip-sans", "i", "", "comma-separated IP subject alternative names") cmd.Flags().IntVarP(&expiryDays, "expiry-days", "e", 365, "certificate validity in days") cmd.Flags().BoolVarP(&compress, "compress", "c", false, "write the certificate, key and chain into a zip file") cmd.Flags().BoolVar(&jsonOut, "json", false, "print the certificate data as JSON instead of writing files") + cmd.AddCommand(caCmd(&cfgFile)) + + return cmd +} + +// caCmd prints the mount's CA chain. Distributing the CA this way decouples it +// from certificate issuance, so a CA rotation reaches every node on the next +// catalog compile instead of waiting out a 90 day renewal cycle. +func caCmd(cfgFile *string) *cobra.Command { + var mount string + + cmd := &cobra.Command{ + Use: "ca", + Short: "Print the PKI mount's CA chain (intermediate and root) as PEM", + Args: cobra.NoArgs, + SilenceUsage: true, + SilenceErrors: true, + RunE: func(cmd *cobra.Command, _ []string) error { + cfg, err := config.LoadCA(*cfgFile) + if err != nil { + return err + } + + mountPoint := cfg.Vault.MountPoint + if mount != "" { + mountPoint = mount + } + if mountPoint == "" { + return fmt.Errorf("vault.mount_point is required (or pass --mount)") + } + + client, err := vault.NewUnauthenticated(cfg.Vault) + if err != nil { + return err + } + + // The chain is fully validated before a byte reaches stdout: a + // half-written document would break the catalog compile. + chain, err := pki.FetchCAChain(client, mountPoint) + if err != nil { + return err + } + _, err = io.WriteString(cmd.OutOrStdout(), chain) + return err + }, + } + + cmd.Flags().StringVarP(&mount, "mount", "m", "", "PKI mount to read (default: vault.mount_point)") + return cmd } @@ -123,7 +174,6 @@ func writeFiles(outputPath, commonName string, compress bool, cert *pki.Certific {"certificate.crt", cert.Certificate, 0o644}, {"private.key", cert.PrivateKey, 0o600}, {"full_chain.crt", cert.FullChain, 0o644}, - {"ca_certificate.crt", cert.CACertificate, 0o644}, } if compress { diff --git a/cmd/certmanager/main_test.go b/cmd/certmanager/main_test.go index 858fc6e..a02c591 100644 --- a/cmd/certmanager/main_test.go +++ b/cmd/certmanager/main_test.go @@ -3,9 +3,16 @@ package main import ( "archive/zip" "bytes" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" "encoding/json" + "encoding/pem" "fmt" "io" + "math/big" "net/http" "net/http/httptest" "os" @@ -27,6 +34,12 @@ type stubOptions struct { k8sStatus int issueStatus int issueDelay time.Duration + + caStatus int + // caBody overrides the CA chain response; caBodySet distinguishes an + // intentional empty body from "unset". + caBody string + caBodySet bool } type capture struct { @@ -34,11 +47,50 @@ type capture struct { authBody map[string]any issueBody map[string]any token string + + caPath string + caRequested bool + // caToken records the header verbatim; the CA endpoint must be called + // without one. + caToken string + caTokenSet bool + // chain is the PEM the stub serves from /ca_chain. + chain string +} + +// caChainPEM is a real two certificate PEM chain, since the ca path parses what +// Vault returns rather than trusting the content type. +func caChainPEM(t *testing.T) string { + t.Helper() + return selfSignedPEM(t, "test intermediate") + selfSignedPEM(t, "test root") +} + +func selfSignedPEM(t *testing.T, cn string) string { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generate key: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(time.Now().UnixNano()), + Subject: pkix.Name{CommonName: cn}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + IsCA: true, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("create certificate: %v", err) + } + return string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})) } func newVaultStub(t *testing.T, opts stubOptions) (*httptest.Server, *capture) { t.Helper() - got := &capture{} + defaultChain := caChainPEM(t) + got := &capture{chain: defaultChain} decode := func(r *http.Request) map[string]any { var m map[string]any @@ -83,6 +135,26 @@ func newVaultStub(t *testing.T, opts stubOptions) (*httptest.Server, *capture) { }} _ = json.NewEncoder(w).Encode(resp) }) + mux.HandleFunc("/v1/pki_int/ca_chain", func(w http.ResponseWriter, r *http.Request) { + got.caRequested = true + got.caPath = r.URL.Path + got.caToken = r.Header.Get("X-Vault-Token") + _, got.caTokenSet = r.Header["X-Vault-Token"] + + if s := orOK(opts.caStatus); s != http.StatusOK { + w.WriteHeader(s) + _, _ = fmt.Fprint(w, `{"errors":["unsupported path"]}`) + return + } + body := opts.caBody + if !opts.caBodySet { + body = defaultChain + } + // Vault advertises pkix-cert while serving PEM; the stub does too, so + // nothing downstream may sniff the content type. + w.Header().Set("Content-Type", "application/pkix-cert") + _, _ = io.WriteString(w, body) + }) srv := httptest.NewTLSServer(mux) t.Cleanup(srv.Close) @@ -140,8 +212,7 @@ func TestRunJSONGolden(t *testing.T) { want := `{"certificate":"-----BEGIN CERTIFICATE-----\nLEAF\n-----END CERTIFICATE-----",` + `"private_key":"-----BEGIN RSA PRIVATE KEY-----\nKEY\n-----END RSA PRIVATE KEY-----",` + - `"full_chain":"-----BEGIN CERTIFICATE-----\nCA\n-----END CERTIFICATE-----\n-----BEGIN CERTIFICATE-----\nLEAF\n-----END CERTIFICATE-----",` + - `"ca_certificate":"-----BEGIN CERTIFICATE-----\nCA\n-----END CERTIFICATE-----"}` + "\n" + `"full_chain":"-----BEGIN CERTIFICATE-----\nLEAF\n-----END CERTIFICATE-----\n-----BEGIN CERTIFICATE-----\nCA\n-----END CERTIFICATE-----"}` + "\n" if stdout.String() != want { t.Errorf("stdout mismatch\n got: %q\nwant: %q", stdout.String(), want) } @@ -151,11 +222,18 @@ func TestRunJSONGolden(t *testing.T) { if err := json.Unmarshal(stdout.Bytes(), &parsed); err != nil { t.Fatalf("stdout is not valid JSON: %v", err) } - if parsed["ca_certificate"] != testCA { - t.Errorf("ca_certificate = %q, want issuing_ca", parsed["ca_certificate"]) + if _, ok := parsed["ca_certificate"]; ok { + t.Error("ca_certificate must not be emitted; the CA comes from the ca subcommand") } - if parsed["full_chain"] != testCA+"\n"+testCert { - t.Errorf("full_chain = %q", parsed["full_chain"]) + if len(parsed) != 3 { + t.Errorf("document has %d keys, want exactly 3: %v", len(parsed), parsed) + } + // Leaf first, issuer after -- asserted as an ordering, not a membership. + if parsed["full_chain"] != testCert+"\n"+testCA { + t.Errorf("full_chain = %q, want leaf then issuing CA", parsed["full_chain"]) + } + if strings.Index(parsed["full_chain"], "LEAF") > strings.Index(parsed["full_chain"], "\nCA\n") { + t.Errorf("full_chain puts the CA ahead of the leaf: %q", parsed["full_chain"]) } if got.token != "s.testtoken" { @@ -357,10 +435,12 @@ func TestRunWritesFiles(t *testing.T) { } want := map[string]string{ - "certificate.crt": testCert, - "private.key": testKey, - "full_chain.crt": testCA + "\n" + testCert, - "ca_certificate.crt": testCA, + "certificate.crt": testCert, + "private.key": testKey, + "full_chain.crt": testCert + "\n" + testCA, + } + if _, err := os.Stat(filepath.Join(out, "host.example.com", "ca_certificate.crt")); !os.IsNotExist(err) { + t.Errorf("ca_certificate.crt must no longer be written by the issue path (stat err = %v)", err) } for name, body := range want { got, err := os.ReadFile(filepath.Join(out, "host.example.com", name)) @@ -401,7 +481,7 @@ func TestRunCompress(t *testing.T) { for _, f := range zr.File { names = append(names, f.Name) } - wantNames := []string{"certificate.crt", "private.key", "full_chain.crt", "ca_certificate.crt"} + wantNames := []string{"certificate.crt", "private.key", "full_chain.crt"} if !reflect.DeepEqual(names, wantNames) { t.Errorf("zip entries = %v, want %v", names, wantNames) } @@ -448,3 +528,244 @@ func TestCompletionSubcommand(t *testing.T) { }) } } + +// authlessConfig carries only addr and mount_point: the CA chain endpoint is +// public, so the ca subcommand must not need auth keys to be present. +func authlessConfig(t *testing.T, addr string, extra ...string) string { + t.Helper() + return writeConfig(t, "vault:\n"+ + " addr: "+addr+"\n"+ + " mount_point: pki_int\n"+ + strings.Join(extra, "")) +} + +func TestRunCAPrintsChain(t *testing.T) { + srv, got := newVaultStub(t, stubOptions{}) + cfg := authlessConfig(t, srv.URL) + + var stdout, stderr bytes.Buffer + if code := run([]string{"ca", "--config", cfg}, &stdout, &stderr); code != 0 { + t.Fatalf("exit code = %d, stderr = %q", code, stderr.String()) + } + + if got.caPath != "/v1/pki_int/ca_chain" { + t.Errorf("requested path = %q, want /v1/pki_int/ca_chain", got.caPath) + } + // Unauthenticated: the header must be absent, not merely empty. + if got.caTokenSet { + t.Errorf("X-Vault-Token must not be sent to the CA endpoint, got %q", got.caToken) + } + if got.issueBody != nil { + t.Error("the ca subcommand must not issue a certificate") + } + + want := strings.TrimSpace(got.chain) + "\n" + if stdout.String() != want { + t.Errorf("stdout = %q, want the PEM chain %q", stdout.String(), want) + } + if n := strings.Count(stdout.String(), "-----BEGIN CERTIFICATE-----"); n != 2 { + t.Errorf("stdout carries %d certificates, want 2", n) + } + if stderr.Len() != 0 { + t.Errorf("stderr = %q, want empty", stderr.String()) + } +} + +// "ca" is the subcommand, never a common name to issue for. +func TestRunCAIsNotTreatedAsACommonName(t *testing.T) { + srv, got := newVaultStub(t, stubOptions{}) + cfg := approleConfig(t, srv.URL) + + var stdout, stderr bytes.Buffer + if code := run([]string{"ca", "--config", cfg}, &stdout, &stderr); code != 0 { + t.Fatalf("exit code = %d, stderr = %q", code, stderr.String()) + } + if !got.caRequested { + t.Error("the CA chain endpoint was never called") + } + if got.issueBody != nil { + t.Errorf("issuance was attempted for common name %v", got.issueBody["common_name"]) + } + if got.authPath != "" { + t.Errorf("the ca subcommand logged in at %q; it must not authenticate", got.authPath) + } +} + +// The positional issue form must keep parsing unchanged now that a subcommand +// sits alongside it. +func TestRunPositionalFormStillParses(t *testing.T) { + srv, got := newVaultStub(t, stubOptions{}) + cfg := approleConfig(t, srv.URL) + + var stdout, stderr bytes.Buffer + if code := run(puppetArgs(cfg), &stdout, &stderr); code != 0 { + t.Fatalf("exit code = %d, stderr = %q", code, stderr.String()) + } + if got.issueBody["common_name"] != "host.example.com" { + t.Errorf("common_name = %v", got.issueBody["common_name"]) + } + if got.caRequested { + t.Error("the issue path must not call the CA chain endpoint") + } +} + +func TestRunCAMountFlagOverridesConfig(t *testing.T) { + srv, got := newVaultStub(t, stubOptions{}) + cfg := writeConfig(t, "vault:\n addr: "+srv.URL+"\n") + + var stdout, stderr bytes.Buffer + if code := run([]string{"ca", "--config", cfg, "--mount", "pki_int"}, &stdout, &stderr); code != 0 { + t.Fatalf("exit code = %d, stderr = %q", code, stderr.String()) + } + if got.caPath != "/v1/pki_int/ca_chain" { + t.Errorf("requested path = %q", got.caPath) + } +} + +func TestRunCASuccessWritesNothingToStderr(t *testing.T) { + srv, _ := newVaultStub(t, stubOptions{}) + cfg := authlessConfig(t, srv.URL) + + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + orig := os.Stderr + os.Stderr = w + + var stdout bytes.Buffer + code := run([]string{"ca", "--config", cfg}, &stdout, os.Stderr) + + os.Stderr = orig + if err := w.Close(); err != nil { + t.Fatalf("close pipe: %v", err) + } + captured, err := io.ReadAll(r) + if err != nil { + t.Fatalf("read pipe: %v", err) + } + + if code != 0 { + t.Fatalf("exit code = %d", code) + } + if len(captured) != 0 { + t.Errorf("stderr must be byte-empty on the ca success path, got %q", captured) + } + if stdout.Len() == 0 { + t.Error("stdout is empty") + } +} + +func TestRunCAFailures(t *testing.T) { + tests := []struct { + name string + opts stubOptions + config func(t *testing.T, addr string) string + args func(cfg string) []string + wantErr string + }{ + { + name: "vault rejects the request", + opts: stubOptions{caStatus: http.StatusForbidden}, + config: func(t *testing.T, addr string) string { return authlessConfig(t, addr) }, + wantErr: "vault returned 403", + }, + { + name: "mount does not exist", + opts: stubOptions{caStatus: http.StatusNotFound}, + config: func(t *testing.T, addr string) string { return authlessConfig(t, addr) }, + wantErr: "vault returned 404", + }, + { + name: "empty body", + opts: stubOptions{caBody: "", caBodySet: true}, + config: func(t *testing.T, addr string) string { return authlessConfig(t, addr) }, + wantErr: "empty CA chain", + }, + { + name: "whitespace only body", + opts: stubOptions{caBody: "\n \n", caBodySet: true}, + config: func(t *testing.T, addr string) string { return authlessConfig(t, addr) }, + wantErr: "empty CA chain", + }, + { + name: "body is not PEM", + opts: stubOptions{caBody: "nope", caBodySet: true}, + config: func(t *testing.T, addr string) string { return authlessConfig(t, addr) }, + wantErr: "not a PEM certificate chain", + }, + { + name: "PEM block is not a certificate", + opts: stubOptions{caBody: "-----BEGIN PUBLIC KEY-----\nAAAA\n-----END PUBLIC KEY-----\n", caBodySet: true}, + config: func(t *testing.T, addr string) string { return authlessConfig(t, addr) }, + wantErr: `not a CERTIFICATE`, + }, + { + name: "PEM certificate is corrupt", + opts: stubOptions{caBody: "-----BEGIN CERTIFICATE-----\nAAAA\n-----END CERTIFICATE-----\n", caBodySet: true}, + config: func(t *testing.T, addr string) string { return authlessConfig(t, addr) }, + wantErr: "parse certificate 1", + }, + { + name: "missing config file", + config: func(t *testing.T, _ string) string { + return filepath.Join(t.TempDir(), "nope.yaml") + }, + wantErr: "open config", + }, + { + name: "config without addr", + config: func(t *testing.T, _ string) string { + return writeConfig(t, "vault:\n mount_point: pki_int\n") + }, + wantErr: "vault.addr is required", + }, + { + name: "no mount anywhere", + config: func(t *testing.T, addr string) string { + return writeConfig(t, "vault:\n addr: "+addr+"\n") + }, + wantErr: "vault.mount_point is required", + }, + { + name: "vault is unreachable", + config: func(t *testing.T, _ string) string { + return authlessConfig(t, "https://127.0.0.1:1", " timeout: 200ms\n") + }, + wantErr: "GET pki_int/ca_chain", + }, + { + name: "ca takes no positional arguments", + config: func(t *testing.T, addr string) string { + return authlessConfig(t, addr) + }, + args: func(cfg string) []string { return []string{"ca", "extra", "--config", cfg} }, + wantErr: "unknown command", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + srv, _ := newVaultStub(t, tc.opts) + cfg := tc.config(t, srv.URL) + args := []string{"ca", "--config", cfg} + if tc.args != nil { + args = tc.args(cfg) + } + + var stdout, stderr bytes.Buffer + code := run(args, &stdout, &stderr) + + if code == 0 { + t.Fatalf("exit code = 0, want non-zero (stdout %q)", stdout.String()) + } + // A half-written PEM would be spliced into the catalog. + if stdout.Len() != 0 { + t.Errorf("stdout must stay empty on failure, got %q", stdout.String()) + } + if !strings.Contains(stderr.String(), tc.wantErr) { + t.Errorf("stderr = %q, want it to mention %q", stderr.String(), tc.wantErr) + } + }) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 905a648..3e3b243 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -85,6 +85,31 @@ func (v VaultConfig) RequestTimeout() (time.Duration, error) { // Load reads, parses, defaults and validates the config file at path. func Load(path string) (*Config, error) { + cfg, err := parse(path) + if err != nil { + return nil, err + } + if err := cfg.Vault.Validate(); err != nil { + return nil, fmt.Errorf("invalid config %q: %w", path, err) + } + return cfg, nil +} + +// LoadCA reads the config for the CA chain path. That endpoint is public, so +// only vault.addr is required: a config carrying no auth method, no role and no +// credentials still loads. +func LoadCA(path string) (*Config, error) { + cfg, err := parse(path) + if err != nil { + return nil, err + } + if err := cfg.Vault.ValidateCA(); err != nil { + return nil, fmt.Errorf("invalid config %q: %w", path, err) + } + return cfg, nil +} + +func parse(path string) (*Config, error) { f, err := os.Open(path) if err != nil { return nil, fmt.Errorf("open config %q: %w", path, err) @@ -102,13 +127,21 @@ func Load(path string) (*Config, error) { if cfg.Vault.JWTPath == "" { cfg.Vault.JWTPath = DefaultJWTPath } - - if err := cfg.Vault.Validate(); err != nil { - return nil, fmt.Errorf("invalid config %q: %w", path, err) - } return &cfg, nil } +// ValidateCA rejects a config that could not reach the CA chain endpoint. It +// deliberately ignores every auth key: the endpoint is unauthenticated. +func (v VaultConfig) ValidateCA() error { + if v.Addr == "" { + return fmt.Errorf("vault.addr is required") + } + if _, err := v.RequestTimeout(); err != nil { + return err + } + return nil +} + // Validate rejects a config that could not produce a certificate, naming the // key at fault. It never falls back from one auth method to another. func (v VaultConfig) Validate() error { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 490f8df..4490a16 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -125,3 +125,63 @@ func TestDefaultPath(t *testing.T) { t.Errorf("DefaultPath() = %q, want the Puppet-managed path", got) } } + +// The CA chain endpoint is unauthenticated, so a config with nothing but addr +// and mount_point has to load. +func TestLoadCAWithoutCredentials(t *testing.T) { + cfg, err := LoadCA(writeConfig(t, "vault:\n addr: https://vault.service.consul:8200\n mount_point: pki_int\n")) + if err != nil { + t.Fatalf("LoadCA: %v", err) + } + if cfg.Vault.Addr != "https://vault.service.consul:8200" { + t.Errorf("addr = %q", cfg.Vault.Addr) + } + if cfg.Vault.MountPoint != "pki_int" { + t.Errorf("mount_point = %q", cfg.Vault.MountPoint) + } + if cfg.Vault.RoleID != "" || cfg.Vault.RoleName != "" { + t.Errorf("LoadCA invented credentials: %+v", cfg.Vault) + } +} + +// Load, by contrast, still demands everything issuance needs. +func TestLoadRejectsWhatLoadCAAccepts(t *testing.T) { + path := writeConfig(t, "vault:\n addr: https://vault.service.consul:8200\n mount_point: pki_int\n") + if _, err := Load(path); err == nil { + t.Fatal("Load accepted a config with no role_name or credentials") + } +} + +func TestLoadCAErrors(t *testing.T) { + tests := []struct { + name string + body string + wantErr string + }{ + {"no addr", "vault:\n mount_point: pki_int\n", "vault.addr is required"}, + {"bad timeout", "vault:\n addr: https://v:8200\n timeout: soon\n", "not a valid duration"}, + {"negative timeout", "vault:\n addr: https://v:8200\n timeout: -5s\n", "must be positive"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := LoadCA(writeConfig(t, tc.body)) + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Errorf("error = %v, want it to mention %q", err, tc.wantErr) + } + }) + } +} + +func TestLoadCAMissingFile(t *testing.T) { + _, err := LoadCA(filepath.Join(t.TempDir(), "absent.yaml")) + if err == nil || !strings.Contains(err.Error(), "open config") { + t.Errorf("error = %v, want an open failure", err) + } +} + +// An unknown auth_method is fatal for issuance but irrelevant to the CA path. +func TestLoadCAIgnoresAuthMethod(t *testing.T) { + if _, err := LoadCA(writeConfig(t, "vault:\n addr: https://v:8200\n auth_method: ldap\n mount_point: pki_int\n")); err != nil { + t.Errorf("LoadCA: %v", err) + } +} diff --git a/internal/pki/pki.go b/internal/pki/pki.go index 2e09543..618481b 100644 --- a/internal/pki/pki.go +++ b/internal/pki/pki.go @@ -1,6 +1,8 @@ package pki import ( + "crypto/x509" + "encoding/pem" "fmt" "strings" @@ -10,10 +12,9 @@ import ( // CertificateResponse is the JSON document printed with --json and consumed by // parsejson() in profiles::pki::vault. Field order fixes the key order. type CertificateResponse struct { - Certificate string `json:"certificate"` - PrivateKey string `json:"private_key"` - FullChain string `json:"full_chain"` - CACertificate string `json:"ca_certificate"` + Certificate string `json:"certificate"` + PrivateKey string `json:"private_key"` + FullChain string `json:"full_chain"` } type issueRequest struct { @@ -48,10 +49,58 @@ func IssueCert(client *vault.Client, mountPoint, roleName, commonName string, al return nil, fmt.Errorf("vault returned no certificate data for %q", commonName) } + // Leaf first: every TLS server that reads full_chain.crt expects the end + // entity certificate ahead of the issuers. return &CertificateResponse{ - Certificate: resp.Data.Certificate, - PrivateKey: resp.Data.PrivateKey, - FullChain: resp.Data.IssuingCA + "\n" + resp.Data.Certificate, - CACertificate: resp.Data.IssuingCA, + Certificate: resp.Data.Certificate, + PrivateKey: resp.Data.PrivateKey, + FullChain: resp.Data.Certificate + "\n" + resp.Data.IssuingCA, }, nil } + +// FetchCAChain returns the mount's CA chain as PEM. /ca_chain is public and +// estate-wide: it needs no token, no role and no per-node context, which is why +// it is fetched here rather than derived from an issue response. It carries the +// root as well as the intermediate, so the result stands alone as a CAfile for +// path builders that insist on a self-signed anchor. +func FetchCAChain(client *vault.Client, mountPoint string) (string, error) { + path := fmt.Sprintf("%s/ca_chain", mountPoint) + body, err := client.Get(path) + if err != nil { + return "", err + } + + // The endpoint advertises application/pkix-cert while serving PEM, so the + // body is parsed rather than sniffed. + chain := strings.TrimSpace(string(body)) + if chain == "" { + return "", fmt.Errorf("GET %s: vault returned an empty CA chain", path) + } + n, err := countCertificates(chain) + if err != nil { + return "", fmt.Errorf("GET %s: %w", path, err) + } + if n == 0 { + return "", fmt.Errorf("GET %s: response is not a PEM certificate chain", path) + } + return chain + "\n", nil +} + +func countCertificates(chain string) (int, error) { + rest := []byte(chain) + count := 0 + for { + var block *pem.Block + block, rest = pem.Decode(rest) + if block == nil { + return count, nil + } + if block.Type != "CERTIFICATE" { + return 0, fmt.Errorf("PEM block %d is a %q, not a CERTIFICATE", count+1, block.Type) + } + if _, err := x509.ParseCertificate(block.Bytes); err != nil { + return 0, fmt.Errorf("parse certificate %d: %w", count+1, err) + } + count++ + } +} diff --git a/internal/pki/pki_test.go b/internal/pki/pki_test.go index e6da5a0..9fb3e50 100644 --- a/internal/pki/pki_test.go +++ b/internal/pki/pki_test.go @@ -1,13 +1,22 @@ package pki_test import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" "encoding/json" + "encoding/pem" "fmt" + "io" + "math/big" "net/http" "net/http/httptest" "reflect" "strings" "testing" + "time" "git.unkin.net/unkin/certmanager/internal/config" "git.unkin.net/unkin/certmanager/internal/pki" @@ -68,14 +77,17 @@ func TestIssueCert(t *testing.T) { } want := &pki.CertificateResponse{ - Certificate: "CERT", - PrivateKey: "KEY", - FullChain: "CA\nCERT", - CACertificate: "CA", + Certificate: "CERT", + PrivateKey: "KEY", + FullChain: "CERT\nCA", } if !reflect.DeepEqual(got, want) { t.Errorf("IssueCert = %+v, want %+v", got, want) } + // Leaf first: asserted as an order, not as membership. + if got.FullChain != got.Certificate+"\n"+"CA" { + t.Errorf("full_chain = %q, want the leaf ahead of the issuing CA", got.FullChain) + } } func TestIssueCertSendsEmptySANFields(t *testing.T) { @@ -124,16 +136,163 @@ func TestIssueCertEmptyPayload(t *testing.T) { func TestCertificateResponseJSONKeyOrder(t *testing.T) { b, err := json.Marshal(&pki.CertificateResponse{ - Certificate: "CERT", - PrivateKey: "KEY", - FullChain: "CA\nCERT", - CACertificate: "CA", + Certificate: "CERT", + PrivateKey: "KEY", + FullChain: "CERT\nCA", }) if err != nil { t.Fatalf("Marshal: %v", err) } - want := `{"certificate":"CERT","private_key":"KEY","full_chain":"CA\nCERT","ca_certificate":"CA"}` + want := `{"certificate":"CERT","private_key":"KEY","full_chain":"CERT\nCA"}` if string(b) != want { t.Errorf("JSON = %s, want %s", b, want) } } + +// newCAServer serves the CA chain endpoint and records what the client sent. +func newCAServer(t *testing.T, h http.HandlerFunc) (*vault.Client, *caCapture) { + t.Helper() + got := &caCapture{} + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got.method = r.Method + got.path = r.URL.Path + _, got.tokenSet = r.Header["X-Vault-Token"] + if strings.HasSuffix(r.URL.Path, "/login") { + got.loggedIn = true + } + h(w, r) + })) + t.Cleanup(srv.Close) + + c, err := vault.NewUnauthenticated(config.VaultConfig{Addr: srv.URL}) + if err != nil { + t.Fatalf("vault.NewUnauthenticated: %v", err) + } + return c, got +} + +type caCapture struct { + method string + path string + tokenSet bool + loggedIn bool +} + +func testChainPEM(t *testing.T) string { + t.Helper() + return testCertPEM(t, "intermediate") + testCertPEM(t, "root") +} + +func testCertPEM(t *testing.T, cn string) string { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generate key: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(time.Now().UnixNano()), + Subject: pkix.Name{CommonName: cn}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + IsCA: true, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("create certificate: %v", err) + } + return string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})) +} + +func TestFetchCAChain(t *testing.T) { + chain := testChainPEM(t) + c, got := newCAServer(t, func(w http.ResponseWriter, _ *http.Request) { + // Vault labels the PEM body application/pkix-cert. + w.Header().Set("Content-Type", "application/pkix-cert") + _, _ = io.WriteString(w, chain) + }) + + out, err := pki.FetchCAChain(c, "pki_int") + if err != nil { + t.Fatalf("FetchCAChain: %v", err) + } + + if got.method != http.MethodGet { + t.Errorf("method = %q, want GET", got.method) + } + if got.path != "/v1/pki_int/ca_chain" { + t.Errorf("path = %q", got.path) + } + if got.tokenSet { + t.Error("X-Vault-Token must not be sent to the CA chain endpoint") + } + if got.loggedIn { + t.Error("the CA chain path must not authenticate") + } + if out != strings.TrimSpace(chain)+"\n" { + t.Errorf("chain = %q, want %q", out, chain) + } + if n := strings.Count(out, "-----BEGIN CERTIFICATE-----"); n != 2 { + t.Errorf("chain carries %d certificates, want 2", n) + } +} + +func TestFetchCAChainFailures(t *testing.T) { + tests := []struct { + name string + handler http.HandlerFunc + wantErr string + }{ + { + name: "non-200", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = io.WriteString(w, `{"errors":["permission denied"]}`) + }, + wantErr: "vault returned 403", + }, + { + name: "empty body", + handler: func(http.ResponseWriter, *http.Request) {}, + wantErr: "empty CA chain", + }, + { + name: "not PEM", + handler: func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, "definitely not a certificate") + }, + wantErr: "not a PEM certificate chain", + }, + { + name: "wrong PEM type", + handler: func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, "-----BEGIN PUBLIC KEY-----\nAAAA\n-----END PUBLIC KEY-----\n") + }, + wantErr: "not a CERTIFICATE", + }, + { + name: "corrupt certificate", + handler: func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, "-----BEGIN CERTIFICATE-----\nAAAA\n-----END CERTIFICATE-----\n") + }, + wantErr: "parse certificate 1", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + c, _ := newCAServer(t, tc.handler) + out, err := pki.FetchCAChain(c, "pki_int") + if err == nil { + t.Fatalf("FetchCAChain succeeded, want an error (got %q)", out) + } + if out != "" { + t.Errorf("a failing FetchCAChain must return no chain, got %q", out) + } + if !strings.Contains(err.Error(), tc.wantErr) { + t.Errorf("error = %v, want it to mention %q", err, tc.wantErr) + } + }) + } +} diff --git a/internal/vault/client.go b/internal/vault/client.go index 250d181..007f26d 100644 --- a/internal/vault/client.go +++ b/internal/vault/client.go @@ -17,6 +17,9 @@ import ( // maxErrorBody caps how much of a Vault error response is quoted back. const maxErrorBody = 2048 +// maxBody caps a GET response; the CA chain is a handful of kilobytes. +const maxBody = 1 << 20 + // Client is an authenticated Vault HTTP client. type Client struct { addr string @@ -28,22 +31,11 @@ type Client struct { // Client. An unknown method, or a failure of the selected method, is an error: // there is no fallback between methods. func New(cfg config.VaultConfig) (*Client, error) { - timeout, err := cfg.RequestTimeout() + c, err := newClient(cfg) if err != nil { return nil, err } - transport := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: cfg.SkipTLSVerify()}, //nolint:gosec // defaults to the Python script's verify=False; flip vault.tls_skip_verify to enable - TLSHandshakeTimeout: timeout, - ResponseHeaderTimeout: timeout, - } - - c := &Client{ - addr: strings.TrimRight(cfg.Addr, "/"), - http: &http.Client{Transport: transport, Timeout: timeout}, - } - switch cfg.AuthMethod { case config.AuthMethodAppRole: err = c.loginAppRole(cfg) @@ -59,6 +51,31 @@ func New(cfg config.VaultConfig) (*Client, error) { return c, nil } +// NewUnauthenticated returns a Client that performs no login and holds no +// token. The PKI CA endpoints are public, so the CA chain is reachable from a +// config that carries no credentials at all. +func NewUnauthenticated(cfg config.VaultConfig) (*Client, error) { + return newClient(cfg) +} + +func newClient(cfg config.VaultConfig) (*Client, error) { + timeout, err := cfg.RequestTimeout() + if err != nil { + return nil, err + } + + transport := &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: cfg.SkipTLSVerify()}, //nolint:gosec // defaults to the Python script's verify=False; flip vault.tls_skip_verify to enable + TLSHandshakeTimeout: timeout, + ResponseHeaderTimeout: timeout, + } + + return &Client{ + addr: strings.TrimRight(cfg.Addr, "/"), + http: &http.Client{Transport: transport, Timeout: timeout}, + }, nil +} + func (c *Client) loginAppRole(cfg config.VaultConfig) error { // The Vault role sets bind_secret_id: false, so role_id alone logs in. payload := map[string]string{"role_id": cfg.RoleID} @@ -105,6 +122,36 @@ func (c *Client) fetchToken(authPath string, payload any) (string, error) { return result.Auth.ClientToken, nil } +// Get sends a GET to a Vault path and returns the raw body undecoded. The +// X-Vault-Token header is omitted entirely when the client holds no token. +func (c *Client) Get(path string) ([]byte, error) { + url := fmt.Sprintf("%s/v1/%s", c.addr, path) + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, url, nil) + if err != nil { + return nil, err + } + if c.token != "" { + req.Header.Set("X-Vault-Token", c.token) + } + + resp, err := c.http.Do(req) + if err != nil { + return nil, fmt.Errorf("GET %s: %w", path, err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + detail, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBody)) + return nil, fmt.Errorf("GET %s: vault returned %d: %s", path, resp.StatusCode, strings.TrimSpace(string(detail))) + } + + body, err := io.ReadAll(io.LimitReader(resp.Body, maxBody)) + if err != nil { + return nil, fmt.Errorf("GET %s: read response: %w", path, err) + } + return body, nil +} + // Post sends an authenticated POST to a Vault path and decodes the response. func (c *Client) Post(path string, payload, out any) error { return c.post(path, c.token, payload, out) diff --git a/internal/vault/client_test.go b/internal/vault/client_test.go index 42c9e0a..d2a8404 100644 --- a/internal/vault/client_test.go +++ b/internal/vault/client_test.go @@ -259,3 +259,70 @@ func TestTLSVerificationCanBeEnabled(t *testing.T) { t.Fatal("New succeeded against a self-signed server with verification enabled") } } + +func TestNewUnauthenticatedDoesNotLogIn(t *testing.T) { + var paths []string + srv := newServer(t, func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + if _, ok := r.Header["X-Vault-Token"]; ok { + t.Errorf("X-Vault-Token sent to %s", r.URL.Path) + } + _, _ = fmt.Fprint(w, "PEM") + }) + + // No auth_method, no role_id, no JWT path: the CA chain endpoint is public. + c, err := vault.NewUnauthenticated(config.VaultConfig{Addr: srv.URL}) + if err != nil { + t.Fatalf("NewUnauthenticated: %v", err) + } + body, err := c.Get("pki_int/ca_chain") + if err != nil { + t.Fatalf("Get: %v", err) + } + if string(body) != "PEM" { + t.Errorf("body = %q", body) + } + if len(paths) != 1 || paths[0] != "/v1/pki_int/ca_chain" { + t.Errorf("requests = %v, want a single GET of the chain", paths) + } +} + +func TestGetSendsTheTokenWhenAuthenticated(t *testing.T) { + var token string + srv := newServer(t, func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/login") { + _, _ = fmt.Fprint(w, `{"auth":{"client_token":"s.token"}}`) + return + } + token = r.Header.Get("X-Vault-Token") + _, _ = fmt.Fprint(w, "PEM") + }) + + c, err := vault.New(baseConfig(srv.URL)) + if err != nil { + t.Fatalf("New: %v", err) + } + if _, err := c.Get("pki_int/ca_chain"); err != nil { + t.Fatalf("Get: %v", err) + } + if token != "s.token" { + t.Errorf("X-Vault-Token = %q", token) + } +} + +func TestGetSurfacesTheStatusAndBody(t *testing.T) { + srv := newServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = fmt.Fprint(w, `{"errors":["unsupported path"]}`) + }) + + c, err := vault.NewUnauthenticated(config.VaultConfig{Addr: srv.URL}) + if err != nil { + t.Fatalf("NewUnauthenticated: %v", err) + } + if _, err := c.Get("pki_int/ca_chain"); err == nil || + !strings.Contains(err.Error(), "vault returned 404") || + !strings.Contains(err.Error(), "unsupported path") { + t.Errorf("error = %v, want the status and Vault's message", err) + } +} diff --git a/test/integration/certmanager_test.go b/test/integration/certmanager_test.go index 7a1fdc0..b3a0799 100644 --- a/test/integration/certmanager_test.go +++ b/test/integration/certmanager_test.go @@ -19,12 +19,18 @@ import ( "path/filepath" "slices" "strings" + "sync" "testing" "time" ) const ( - pkiMount = "pki" + // Dev-mode Vault has no PKI of its own, and a single root mount would make + // the CA chain one self-signed certificate -- which would not exercise the + // intermediate + root shape production serves. So a root mount signs an + // intermediate, and certificates are issued from the intermediate. + rootMount = "pki_root" + pkiMount = "pki_int" pkiRole = "puppet" approlePath = "approle" approleRole = "certmanager" @@ -123,18 +129,59 @@ func vaultReq(t *testing.T, method, path string, payload any, okErrors ...string return out } -// setupVault provisions the PKI mount, issuing role, policy and approle role, -// returning the role_id certmanager will authenticate with. +// setupVault provisions the mounts, issuing role, policy and approle role, +// returning the role_id certmanager will authenticate with. Provisioning runs +// once per process: re-signing the intermediate would add issuers and make the +// mount's CA chain non-deterministic. func setupVault(t *testing.T) string { t.Helper() + setupOnce.Do(func() { setupRoleID = provisionVault(t) }) + if setupRoleID == "" { + t.Fatal("vault provisioning failed") + } + return setupRoleID +} +var ( + setupOnce sync.Once + setupRoleID string +) + +func provisionVault(t *testing.T) string { + t.Helper() + + vaultReq(t, http.MethodPost, "sys/mounts/"+rootMount, + map[string]any{"type": "pki", "config": map[string]any{"max_lease_ttl": "87600h"}}, + "path is already in use") vaultReq(t, http.MethodPost, "sys/mounts/"+pkiMount, map[string]any{"type": "pki", "config": map[string]any{"max_lease_ttl": "87600h"}}, "path is already in use") - vaultReq(t, http.MethodPost, pkiMount+"/root/generate/internal", map[string]any{ - "common_name": certDomain + " integration CA", + root := vaultReq(t, http.MethodPost, rootMount+"/root/generate/internal", map[string]any{ + "common_name": certDomain + " integration root CA", "ttl": "87600h", + "key_type": "rsa", + "key_bits": 2048, + }) + rootPEM := dataString(t, root, "certificate") + + csr := vaultReq(t, http.MethodPost, pkiMount+"/intermediate/generate/internal", map[string]any{ + "common_name": certDomain + " integration intermediate CA", + "key_type": "rsa", + "key_bits": 2048, + }) + + signed := vaultReq(t, http.MethodPost, rootMount+"/root/sign-intermediate", map[string]any{ + "csr": dataString(t, csr, "csr"), + "common_name": certDomain + " integration intermediate CA", + "ttl": "43800h", + "format": "pem", + }) + + // Appending the root to the signed intermediate is what makes /ca_chain + // serve both certificates. + vaultReq(t, http.MethodPost, pkiMount+"/intermediate/set-signed", map[string]any{ + "certificate": dataString(t, signed, "certificate") + "\n" + rootPEM, }) vaultReq(t, http.MethodPost, pkiMount+"/roles/"+pkiRole, map[string]any{ @@ -172,6 +219,16 @@ func setupVault(t *testing.T) string { return roleID } +func dataString(t *testing.T, resp map[string]any, key string) string { + t.Helper() + data, _ := resp["data"].(map[string]any) + v, _ := data[key].(string) + if v == "" { + t.Fatalf("vault returned no data.%s", key) + } + return v +} + // buildBinary compiles certmanager with the config path baked in, so the test // can invoke it with exactly the argv Puppet's generate() uses -- no --config. func buildBinary(t *testing.T, dir, configPath string) string { @@ -263,11 +320,17 @@ func TestIssueCertificateAgainstVault(t *testing.T) { if err := json.Unmarshal([]byte(stdout), &doc); err != nil { t.Fatalf("stdout is not JSON: %v\n%s", err, stdout) } - for _, key := range []string{"certificate", "private_key", "full_chain", "ca_certificate"} { + for _, key := range []string{"certificate", "private_key", "full_chain"} { if doc[key] == "" { t.Errorf("JSON key %q is missing or empty", key) } } + if _, ok := doc["ca_certificate"]; ok { + t.Error("ca_certificate must not be emitted; the CA comes from the ca subcommand") + } + if len(doc) != 3 { + t.Errorf("document has %d keys, want exactly 3: %v", len(doc), keysOf(doc)) + } cert := parseCert(t, doc["certificate"]) if cert.Subject.CommonName != commonName { @@ -290,16 +353,173 @@ func TestIssueCertificateAgainstVault(t *testing.T) { t.Errorf("certificate lifetime %v, want ~%dd", lifetime, expiryDays) } - ca := parseCert(t, doc["ca_certificate"]) - if !ca.IsCA { - t.Errorf("ca_certificate is not a CA certificate") + // full_chain is leaf first: asserted as an order, not as membership. + chain := parseChain(t, doc["full_chain"]) + if len(chain) != 2 { + t.Fatalf("full_chain carries %d certificates, want leaf + issuing CA", len(chain)) } - if err := cert.CheckSignatureFrom(ca); err != nil { - t.Errorf("certificate is not signed by ca_certificate: %v", err) + if !chain[0].Equal(cert) { + t.Errorf("full_chain starts with %q, want the leaf %q", chain[0].Subject.CommonName, commonName) } - if !strings.Contains(doc["full_chain"], strings.TrimSpace(doc["certificate"])) || - !strings.Contains(doc["full_chain"], strings.TrimSpace(doc["ca_certificate"])) { - t.Errorf("full_chain does not contain both the leaf and the issuing CA") + if !chain[1].IsCA { + t.Errorf("full_chain[1] (%q) is not a CA certificate", chain[1].Subject.CommonName) + } + if err := chain[0].CheckSignatureFrom(chain[1]); err != nil { + t.Errorf("full_chain is not leaf-then-issuer: %v", err) + } +} + +// TestCAChainAgainstVault is the reason the CA no longer rides along with +// issuance: this runs with no credentials at all. +func TestCAChainAgainstVault(t *testing.T) { + waitForVault(t) + roleID := setupVault(t) + + dir := t.TempDir() + cfg := writeConfig(t, dir, roleID) + bin := buildBinary(t, dir, cfg) + + code, stdout, stderr := runCertmanager(t, bin, "ca") + if code != 0 { + t.Fatalf("exit code %d, stderr: %s", code, stderr) + } + if stderr != "" { + t.Errorf("stderr must be empty on success, got %q", stderr) + } + + // Puppet reads this through generate() too, so it must be the PEM and + // nothing else. + if !strings.HasPrefix(stdout, "-----BEGIN CERTIFICATE-----") { + t.Fatalf("stdout is not a PEM document: %q", stdout) + } + + chain := parseChain(t, stdout) + if len(chain) != 2 { + t.Fatalf("CA chain carries %d certificates, want intermediate + root", len(chain)) + } + intermediate, root := chain[0], chain[1] + + if !intermediate.IsCA || !root.IsCA { + t.Errorf("both chain entries must be CA certificates: %v, %v", intermediate.IsCA, root.IsCA) + } + // The root anchors the chain: self-issued and self-signed. issuing_ca alone + // would not be, which is what breaks strict path builders. + if root.Subject.String() != root.Issuer.String() { + t.Errorf("chain[1] subject %q != issuer %q, so it is not the root", root.Subject, root.Issuer) + } + if err := root.CheckSignatureFrom(root); err != nil { + t.Errorf("chain[1] is not self-signed: %v", err) + } + if intermediate.Subject.String() == intermediate.Issuer.String() { + t.Errorf("chain[0] %q is self-issued; expected the intermediate", intermediate.Subject) + } + + // Issue a leaf and verify it against the chain the ca subcommand returned. + _, issued, stderr := runCertmanager(t, bin, + commonName, + "--alt-names", shortName+","+altName, + "--ip-sans", ipSAN1+","+ipSAN2, + "--expiry-days", fmt.Sprint(expiryDays), + "--json", + ) + var doc map[string]string + if err := json.Unmarshal([]byte(issued), &doc); err != nil { + t.Fatalf("issue stdout is not JSON: %v (stderr %q)", err, stderr) + } + leaf := parseCert(t, doc["certificate"]) + + if err := leaf.CheckSignatureFrom(intermediate); err != nil { + t.Errorf("chain[0] did not sign the issued leaf: %v", err) + } + + // The openssl verify equivalent: anchor on the root, bridge with the + // intermediate, both taken from the ca output alone. + roots := x509.NewCertPool() + roots.AddCert(root) + inters := x509.NewCertPool() + inters.AddCert(intermediate) + if _, err := leaf.Verify(x509.VerifyOptions{ + Roots: roots, + Intermediates: inters, + DNSName: commonName, + KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, + }); err != nil { + t.Errorf("leaf does not validate against the ca chain: %v", err) + } +} + +// The CA chain endpoint is public, so a config with no auth keys must work. +func TestCAChainNeedsNoCredentials(t *testing.T) { + waitForVault(t) + roleID := setupVault(t) + + dir := t.TempDir() + bin := buildBinary(t, dir, writeConfig(t, dir, roleID)) + + authless := filepath.Join(dir, "authless.yaml") + body := fmt.Sprintf("vault:\n addr: %s\n mount_point: %s\n", vaultAddr(), pkiMount) + if err := os.WriteFile(authless, []byte(body), 0o600); err != nil { + t.Fatalf("write config: %v", err) + } + + code, stdout, stderr := runCertmanager(t, bin, "ca", "--config", authless) + if code != 0 { + t.Fatalf("exit code %d, stderr: %s", code, stderr) + } + if stderr != "" { + t.Errorf("stderr must be empty on success, got %q", stderr) + } + if len(parseChain(t, stdout)) != 2 { + t.Errorf("credential-free ca output is not the two certificate chain: %q", stdout) + } +} + +func TestCAChainFailsCleanlyForAnUnknownMount(t *testing.T) { + waitForVault(t) + roleID := setupVault(t) + + dir := t.TempDir() + bin := buildBinary(t, dir, writeConfig(t, dir, roleID)) + + code, stdout, stderr := runCertmanager(t, bin, "ca", "--mount", "pki_does_not_exist") + if code == 0 { + t.Fatalf("expected a non-zero exit, stdout: %s", stdout) + } + if stdout != "" { + t.Errorf("stdout must be empty on failure, got %q", stdout) + } + if !strings.Contains(stderr, "certmanager:") { + t.Errorf("stderr should carry the cause, got %q", stderr) + } +} + +func keysOf(m map[string]string) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + slices.Sort(out) + return out +} + +func parseChain(t *testing.T, data string) []*x509.Certificate { + t.Helper() + var out []*x509.Certificate + rest := []byte(data) + for { + var block *pem.Block + block, rest = pem.Decode(rest) + if block == nil { + return out + } + if block.Type != "CERTIFICATE" { + t.Fatalf("PEM block %d is a %q", len(out)+1, block.Type) + } + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + t.Fatalf("parse certificate %d: %v", len(out)+1, err) + } + out = append(out, cert) } }