diff --git a/CHANGELOG.md b/CHANGELOG.md index f898f4bd..0d9583f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,25 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. --- +## [Unreleased] + +### Added + +- Display issuer-validity truncation warnings on stderr after successful native + CA issuance, renewal, rekey, admin login, and SSH-associated X.509 identity + operations, including offline X.509 operations and renewal daemons, without + changing successful exit status + ([smallstep/certificates#381](https://github.com/smallstep/certificates/issues/381)). + +### Fixed + +- Keep certificate warnings nonfatal when stderr is a broken pipe, without + changing process-wide signal handling + ([smallstep/certificates#560](https://github.com/smallstep/certificates/issues/560)). +- Display X.509 identity validity warnings during implicit SSH proxy login and + automatic login after an unauthorized response + ([smallstep/certificates#560](https://github.com/smallstep/certificates/issues/560)). + ## [0.31.0] - 2026-09-25 ### Added diff --git a/command/ca/ca.go b/command/ca/ca.go index bd5e68c5..a6fb3b35 100644 --- a/command/ca/ca.go +++ b/command/ca/ca.go @@ -21,6 +21,12 @@ func init() { authority, retrieve the root of trust, sign and renew certificates, and create and manage provisioners. +Native CA commands print issuer-validity truncation warnings to stderr after +successfully writing or installing the issued credentials, without changing +successful exit status. Administrative commands also report these warnings +when login creates a new in-memory admin identity; existing admin credentials +do not trigger issuance warnings. + ## EXAMPLES Create the configuration for a new certificate authority: diff --git a/command/ca/certificate.go b/command/ca/certificate.go index 74ea4db9..424fdd6c 100644 --- a/command/ca/certificate.go +++ b/command/ca/certificate.go @@ -35,6 +35,11 @@ func certificateCommand() cli.Command { [**--context**=]`, Description: `**step ca certificate** command generates a new certificate pair +For native CA issuance, including **--offline**, issuer-validity truncation +warnings are printed to stderr after the certificate is written. These warnings +do not change successful exit status. ACME issuance does not provide native CA +warning metadata. + ## POSITIONAL ARGUMENTS diff --git a/command/ca/rekey.go b/command/ca/rekey.go index aa80dcac..7796a620 100644 --- a/command/ca/rekey.go +++ b/command/ca/rekey.go @@ -43,6 +43,10 @@ to disk - either overwriting and positional arguments or using new files when the **--out-cert**= and **--out-key**= flags are used. +Issuer-validity truncation warnings are printed to stderr after the certificate +and key are written, including daemon rekeys. These warnings do not change +successful exit status. + With the **--daemon** flag the command will periodically update the given certificate. By default, it will rekey the certificate before 2/3 of the validity period of the certificate has elapsed. A random jitter is used to avoid multiple diff --git a/command/ca/renew.go b/command/ca/renew.go index ba2b5783..e8ed61f7 100644 --- a/command/ca/renew.go +++ b/command/ca/renew.go @@ -55,12 +55,19 @@ func renewCertificateCommand() cli.Command { certificate authority) and writes the new certificate to disk - either overwriting or using a new file when the **--out**= flag is used. +Issuer-validity truncation warnings are printed to stderr after the renewed +certificate is written, including offline and daemon renewals. These warnings +do not change successful exit status. + With the **--daemon** flag the command will periodically update the given certificate. By default, it will renew the certificate before 2/3 of the validity period of the certificate has elapsed. A random jitter is used to avoid multiple instances running at the same time. The amount of time between renewal and certificate expiration can be configured using the **--expires-in** flag, or a fixed period can be set with the **--renew-period** flag. +If issuer validity shortens the certificate lifetime below a fixed renewal +period, the existing scheduling policy renews immediately. Renewal preserves +the previous certificate's duration, including an earlier truncated duration. The **--daemon** flag can be combined with **--pid**, **--signal**, or **--exec** to provide certificate reloads on your services. @@ -503,6 +510,7 @@ func (r *renewer) Renew(outFile string) (resp *api.SignResponse, err error) { return nil, errs.FileError(err, outFile) } + cautils.PrintCertificateWarnings(os.Stderr, resp.Warnings) return resp, nil } @@ -540,6 +548,7 @@ func (r *renewer) Rekey(priv any, outCert, outKey string, writePrivateKey bool) } } + cautils.PrintCertificateWarnings(os.Stderr, resp.Warnings) return resp, nil } diff --git a/command/ca/sign.go b/command/ca/sign.go index 5129aa89..7e8e8018 100644 --- a/command/ca/sign.go +++ b/command/ca/sign.go @@ -34,6 +34,11 @@ func signCertificateCommand() cli.Command { [**--root**=] [**--context**=]`, Description: `**step ca sign** command signs the given csr and generates a new certificate. +For native CA issuance, including **--offline**, issuer-validity truncation +warnings are printed to stderr after the certificate is written. These warnings +do not change successful exit status. ACME issuance does not provide native CA +warning metadata. + ## POSITIONAL ARGUMENTS diff --git a/command/ssh/certificate.go b/command/ssh/certificate.go index ab89bf84..cddb0509 100644 --- a/command/ssh/certificate.go +++ b/command/ssh/certificate.go @@ -55,6 +55,11 @@ certificate using [step certificates](https://github.com/smallstep/certificates) With a certificate clients or servers may trust only the CA key and verify its signature on a certificate rather than trusting many user/host keys. +When the CA also issues an X.509 identity, issuer-validity truncation warnings +are printed to stderr after the credentials are written. These warnings do not +change successful exit status or the SSH certificate's validity. Offline SSH +issuance does not issue an X.509 identity. + Note that not all the provisioner types will be able to generate user and host certificates. Currently JWK provisioners can generate both, but with an OIDC provisioner you will only be able to generate user certificates unless you are @@ -545,6 +550,7 @@ func certificateAction(ctx *cli.Context) error { ui.PrintSelected("Add User Certificate", baseName+"-provisioner-cert.pub") } + cautils.PrintCertificateWarnings(os.Stderr, resp.Warnings) return nil } diff --git a/command/ssh/login.go b/command/ssh/login.go index 567bcff3..59ac17a8 100644 --- a/command/ssh/login.go +++ b/command/ssh/login.go @@ -2,6 +2,7 @@ package ssh import ( "crypto" + "os" "time" "github.com/pkg/errors" @@ -41,6 +42,11 @@ certificate. This certificate will be automatically added to the SSH agent. With a certificate servers may trust only the CA key and verify its signature on a certificate rather than trusting many user keys. +When the CA also issues an X.509 identity, issuer-validity truncation warnings +are printed to stderr after the identity is installed. These warnings do not +change successful exit status or the SSH certificate's validity. Offline SSH +issuance does not issue an X.509 identity. + ## POSITIONAL ARGUMENTS @@ -298,5 +304,6 @@ func loginAction(ctx *cli.Context) error { } } + cautils.PrintCertificateWarnings(os.Stderr, resp.Warnings) return nil } diff --git a/command/ssh/proxycommand.go b/command/ssh/proxycommand.go index 0a0293b3..3c9d5a47 100644 --- a/command/ssh/proxycommand.go +++ b/command/ssh/proxycommand.go @@ -40,7 +40,11 @@ func proxycommandCommand() cli.Command { and proxies the ssh connection according to its configuration. This command is used in the ssh client config with keyword. -This command will add the user to the ssh-agent if necessary. +This command will add the user to the ssh-agent if necessary. When the CA also +issues an X.509 identity, issuer-validity truncation warnings are printed to +stderr after the identity is installed. These warnings do not change successful +exit status or the SSH certificate's validity. Offline SSH issuance does not +issue an X.509 identity. ## POSITIONAL ARGUMENTS @@ -212,7 +216,12 @@ func doLoginIfNeeded(ctx *cli.Context, subject string) error { } // Add certificate and private key to agent - return agent.AddCertificate(subject, resp.Certificate.Certificate, priv) + if err := agent.AddCertificate(subject, resp.Certificate.Certificate, priv); err != nil { + return err + } + + cautils.PrintCertificateWarnings(os.Stderr, resp.Warnings) + return nil } func getBastion(ctx *cli.Context, user, host string) (*api.SSHBastionResponse, error) { diff --git a/command/ssh/rekey.go b/command/ssh/rekey.go index a384f9a4..50c59b8b 100644 --- a/command/ssh/rekey.go +++ b/command/ssh/rekey.go @@ -37,6 +37,11 @@ an existing SSH Certificate and key pair to authenticate and templatize the request. It writes the new certificate to disk - either overwriting or using new files when the **--out**= flag is used. +When the CA also renews an X.509 identity, issuer-validity truncation warnings +are printed to stderr after the credentials are written. These warnings do not +change successful exit status or the SSH certificate's validity. Offline SSH +rekey does not renew an X.509 identity. + ## POSITIONAL ARGUMENTS @@ -194,5 +199,6 @@ func rekeyAction(ctx *cli.Context) error { ui.PrintSelected("Public Key", newPubFile) ui.PrintSelected("Certificate", newCertFile) + cautils.PrintCertificateWarnings(os.Stderr, resp.Warnings) return nil } diff --git a/command/ssh/renew.go b/command/ssh/renew.go index 1498b038..b3be8089 100644 --- a/command/ssh/renew.go +++ b/command/ssh/renew.go @@ -35,6 +35,11 @@ It writes the new certificate to disk - either overwriting or using a new file when the **--out**= flag is used. This command cannot be used to renew SSH User Certificates. +When the CA also renews an X.509 identity, issuer-validity truncation warnings +are printed to stderr after the credentials are written. These warnings do not +change successful exit status or the SSH certificate's validity. Offline SSH +renewal does not renew an X.509 identity. + ## POSITIONAL ARGUMENTS @@ -141,5 +146,6 @@ func renewAction(ctx *cli.Context) error { ui.PrintSelected("Certificate", outFile) + cautils.PrintCertificateWarnings(os.Stderr, resp.Warnings) return nil } diff --git a/command/ssh/ssh.go b/command/ssh/ssh.go index 24a4e8d8..8c4b3dba 100644 --- a/command/ssh/ssh.go +++ b/command/ssh/ssh.go @@ -2,6 +2,7 @@ package ssh import ( "net/http" + "os" "strings" "github.com/pkg/errors" @@ -30,6 +31,11 @@ func init() { UsageText: "step ssh [arguments] [global-flags] [subcommand-flags]", Description: `**step ssh** command group provides facilities to sign SSH certificates. +When automatic login issues an X.509 identity, issuer-validity truncation +warnings are printed to stderr after the identity is installed, including login +while recovering from an unauthorized response. Warning output does not change +successful exit status or SSH certificate validity. + ## EXAMPLES Generate a new SSH key pair and user certificate: @@ -247,6 +253,7 @@ func loginOnUnauthorized(ctx *cli.Context) (ca.RetryFunc, error) { agent.AddCertificate(jwt.Payload.Email, resp.Certificate.Certificate, priv) } + cautils.PrintCertificateWarnings(os.Stderr, resp.Warnings) return true }, nil } diff --git a/integration/issuer_validity_ssh_login_test.go b/integration/issuer_validity_ssh_login_test.go new file mode 100644 index 00000000..fd6d3716 --- /dev/null +++ b/integration/issuer_validity_ssh_login_test.go @@ -0,0 +1,154 @@ +package integration + +import ( + "encoding/json" + "encoding/pem" + "net" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "testing" + "time" + + "github.com/smallstep/certificates/authority" + "github.com/smallstep/certificates/authority/config" + "github.com/smallstep/certificates/authority/provisioner" + "github.com/stretchr/testify/require" + "go.step.sm/crypto/jose" + "golang.org/x/crypto/ssh/agent" +) + +func TestIssuerValiditySSHProxycommandLogin(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("the real SSH agent fixture uses a Unix socket") + } + + previousVersion := authority.GlobalVersion + authority.GlobalVersion.RequireClientAuthentication = true + t.Cleanup(func() { authority.GlobalVersion = previousVersion }) + f := newIssuerValidityCLI(t) + home := t.TempDir() + // Unix socket paths are limited to 104 bytes on macOS. + t.Chdir(t.TempDir()) + socket := "agent.sock" + listener, err := net.Listen("unix", socket) + require.NoError(t, err) + t.Cleanup(func() { listener.Close() }) + keyring := agent.NewKeyring() + go serveIssuerValidityAgent(listener, keyring) + t.Setenv("SSH_AUTH_SOCK", socket) + + proxyListener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { proxyListener.Close() }) + go func() { + conn, err := proxyListener.Accept() + if err == nil { + conn.Close() + } + }() + + host, port, err := net.SplitHostPort(proxyListener.Addr().String()) + require.NoError(t, err) + args := []string{ + "ssh", "proxycommand", "ssh.example", host, port, + "--provisioner", "issuer-validity", "--provisioner-password-file", f.passwordFile, + } + stdout, stderr, err := f.run(t, home, args...) + require.NoError(t, err, "%s", stderr) + assertCLIWarning(t, stdout, stderr, 1) + + f.assertLeaf(t, filepath.Join(home, "identity", "identity.crt"), f.issuer.NotAfter) + + keys, err := keyring.List() + require.NoError(t, err) + require.Len(t, keys, 1) +} + +func TestIssuerValiditySSHUnauthorizedLogin(t *testing.T) { + // Complete a real OIDC device flow with a local provider. The CA verifies the + // signed ID token and issues the identity; no issuance metadata is mocked. + key, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0) + require.NoError(t, err) + signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.ES256, Key: key}, + new(jose.SignerOptions).WithType("JWT").WithHeader("kid", key.KeyID)) + require.NoError(t, err) + mux := http.NewServeMux() + provider := httptest.NewTLSServer(mux) + t.Cleanup(provider.Close) + providerRoot := filepath.Join(t.TempDir(), "provider.crt") + require.NoError(t, os.WriteFile(providerRoot, + pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: provider.Certificate().Raw}), 0600)) + t.Setenv("STEP_TEST_OIDC_ROOT", providerRoot) + now := time.Now() + token, err := jose.Signed(signer).Claims(jose.Claims{ + Issuer: provider.URL, Subject: "user", Audience: []string{"ssh-client"}, + IssuedAt: jose.NewNumericDate(now), Expiry: jose.NewNumericDate(now.Add(time.Minute)), + }).Claims(map[string]any{"email": "user@example.com", "email_verified": true}).CompactSerialize() + require.NoError(t, err) + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]string{ + "issuer": provider.URL, "jwks_uri": provider.URL + "/keys", + "device_authorization_endpoint": provider.URL + "/device", + "token_endpoint": provider.URL + "/token", + }) + }) + mux.HandleFunc("/keys", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(jose.JSONWebKeySet{Keys: []jose.JSONWebKey{key.Public()}}) + }) + mux.HandleFunc("/device", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "device_code": "test-device", "user_code": "test-code", + "verification_uri": provider.URL + "/verify", "interval": 1, "expires_in": 20, + }) + }) + mux.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) { + if r.FormValue("device_code") != "test-device" { + http.Error(w, "invalid device code", http.StatusBadRequest) + return + } + _ = json.NewEncoder(w).Encode(map[string]string{"id_token": token, "access_token": "test-access", "token_type": "Bearer"}) + }) + enableSSH := true + oidc := &provisioner.OIDC{ + Type: "OIDC", Name: "identity", ClientID: "ssh-client", + ConfigurationEndpoint: provider.URL + "/.well-known/openid-configuration", + Claims: &provisioner.Claims{ + EnableSSHCA: &enableSSH, + DefaultUserSSHDur: &provisioner.Duration{Duration: 72 * time.Hour}, + MaxUserSSHDur: &provisioner.Duration{Duration: 168 * time.Hour}, + DefaultTLSDur: &provisioner.Duration{Duration: 72 * time.Hour}, + MaxTLSDur: &provisioner.Duration{Duration: 168 * time.Hour}, + }, + } + previousVersion := authority.GlobalVersion + authority.GlobalVersion.RequireClientAuthentication = true + t.Cleanup(func() { authority.GlobalVersion = previousVersion }) + f := newIssuerValidityCLI(t, func(cfg *config.Config) { + cfg.AuthorityConfig.Provisioners = append(cfg.AuthorityConfig.Provisioners, oidc) + cfg.FederatedRoots = append(cfg.FederatedRoots, providerRoot) + }) + home := t.TempDir() + // Agent installation is deliberately optional on unauthorized recovery. + t.Setenv("SSH_AUTH_SOCK", filepath.Join(home, "missing-agent.sock")) + stdout, stderr, err := f.run(t, home, "ssh", "hosts", "--console") + require.NoError(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + require.Contains(t, stdout, "host.example") + assertCLIWarning(t, stdout, stderr, 1) + f.assertLeaf(t, filepath.Join(home, "identity", "identity.crt"), f.issuer.NotAfter) +} + +func serveIssuerValidityAgent(listener net.Listener, keyring agent.Agent) { + for { + conn, err := listener.Accept() + if err != nil { + return + } + go func() { + defer conn.Close() + _ = agent.ServeAgent(keyring, conn) + }() + } +} diff --git a/integration/issuer_validity_test.go b/integration/issuer_validity_test.go new file mode 100644 index 00000000..1a7c098f --- /dev/null +++ b/integration/issuer_validity_test.go @@ -0,0 +1,654 @@ +package integration + +import ( + "bytes" + "context" + "crypto" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/json" + "encoding/pem" + "flag" + "io" + "math/big" + "net" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "sync" + "testing" + "time" + + "github.com/go-chi/chi/v5" + "github.com/stretchr/testify/require" + "github.com/urfave/cli" + "go.step.sm/crypto/jose" + "go.step.sm/crypto/keyutil" + "go.step.sm/crypto/minica" + "go.step.sm/crypto/pemutil" + "golang.org/x/crypto/ssh" + "golang.org/x/crypto/ssh/agent" + + "github.com/smallstep/certificates/api" + "github.com/smallstep/certificates/authority" + "github.com/smallstep/certificates/authority/config" + "github.com/smallstep/certificates/authority/provisioner" + "github.com/smallstep/certificates/ca" + "github.com/smallstep/certificates/cas/apiv1" + "github.com/smallstep/certificates/errs" + "github.com/smallstep/cli/utils/cautils" +) + +// The existing integration TestMain installs the real step entrypoint as an +// executable. These tests use that process and real signed credentials, not a +// mocked client or response transport. +func TestIssuerValidityNativeCommands(t *testing.T) { + f := newIssuerValidityCLI(t) + for _, offline := range []bool{false, true} { + name := "online" + if offline { + name = "offline" + } + t.Run(name, func(t *testing.T) { + home := t.TempDir() + for _, operation := range []string{"certificate", "sign"} { + t.Run(operation, func(t *testing.T) { + certFile := filepath.Join(t.TempDir(), "leaf.crt") + keyFile := filepath.Join(t.TempDir(), "leaf.key") + args := []string{"ca", operation} + if operation == "certificate" { + args = append(args, "native.example", certFile, keyFile) + } else { + key, err := keyutil.GenerateDefaultSigner() + require.NoError(t, err) + csr, err := x509.CreateCertificateRequest(rand.Reader, &x509.CertificateRequest{ + Subject: pkix.Name{CommonName: "native.example"}, DNSNames: []string{"native.example"}, + }, key) + require.NoError(t, err) + csrFile := filepath.Join(t.TempDir(), "leaf.csr") + require.NoError(t, os.WriteFile(csrFile, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csr}), 0600)) + args = append(args, csrFile, certFile) + } + args = append(args, "--not-after", "96h") + if offline { + args = append(args, f.offlineFlags()...) + } else { + args = append(args, "--token", f.signToken(t, "native.example")) + } + stdout, stderr, err := f.run(t, home, args...) + require.NoError(t, err, "%s", stderr) + assertCLIWarning(t, stdout, stderr, 1) + f.assertLeaf(t, certFile, f.issuer.NotAfter) + }) + } + for _, operation := range []string{"renew", "token-renew", "rekey"} { + t.Run(operation, func(t *testing.T) { + certFile, keyFile, oldLeaf := f.oldCredential(t, f.now, f.now.Add(32*time.Hour)) + command := operation + if command == "token-renew" { + command = "renew" + } + args := []string{"ca", command, certFile, keyFile, "--force"} + if operation == "token-renew" { + args = append(args, "--mtls=false") + } + if offline { + args = append(args, "--offline", "--ca-config", f.configFile) + } + stdout, stderr, err := f.run(t, home, args...) + require.NoError(t, err, "%s", stderr) + assertCLIWarning(t, stdout, stderr, 1) + leaf := f.assertLeaf(t, certFile, f.issuer.NotAfter) + pair, err := tls.LoadX509KeyPair(certFile, keyFile) + require.NoError(t, err) + require.True(t, keyutil.Equal(leaf.PublicKey, pair.PrivateKey.(crypto.Signer).Public())) + if operation == "rekey" { + require.False(t, keyutil.Equal(oldLeaf.PublicKey, leaf.PublicKey)) + } else { + require.True(t, keyutil.Equal(oldLeaf.PublicKey, leaf.PublicKey)) + } + }) + } + // Warning-free replies remain silent. A rejected request must not leave + // certificate output or produce a success warning. + for _, test := range []struct { + name, duration string + failure bool + }{ + {"uncapped", "1h", false}, + {"invalid duration", "200h", true}, + } { + t.Run(test.name, func(t *testing.T) { + certFile := filepath.Join(t.TempDir(), "leaf.crt") + args := []string{"ca", "certificate", "native.example", certFile, filepath.Join(t.TempDir(), "leaf.key"), "--not-after", test.duration} + if offline { + args = append(args, f.offlineFlags()...) + } else { + args = append(args, "--token", f.signToken(t, "native.example")) + } + stdout, stderr, err := f.run(t, home, args...) + assertCLIWarning(t, stdout, stderr, 0) + if test.failure { + require.Error(t, err) + _, statErr := os.Stat(certFile) + require.True(t, os.IsNotExist(statErr)) + } else { + require.NoError(t, err, "%s", stderr) + bundle, err := pemutil.ReadCertificateBundle(certFile) + require.NoError(t, err) + leaf := bundle[0] + require.True(t, leaf.NotAfter.Before(f.issuer.NotAfter)) + require.NoError(t, leaf.CheckSignatureFrom(f.issuer)) + } + }) + } + }) + } +} + +func TestIssuerValiditySSHCommands(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("the real SSH agent fixture uses a Unix socket") + } + f := newIssuerValidityCLI(t) + home := t.TempDir() + // Keep the socket address below macOS's 104-byte Unix socket limit. + t.Chdir(t.TempDir()) + socket := "agent.sock" + listener, err := net.Listen("unix", socket) + require.NoError(t, err) + t.Cleanup(func() { listener.Close() }) + keyring := agent.NewKeyring() + go func() { + for { + conn, err := listener.Accept() + if err != nil { + return + } + go func() { + defer conn.Close() + _ = agent.ServeAgent(keyring, conn) + }() + } + }() + t.Setenv("SSH_AUTH_SOCK", socket) + previousVersion := authority.GlobalVersion + authority.GlobalVersion.RequireClientAuthentication = true + t.Cleanup(func() { authority.GlobalVersion = previousVersion }) + + for _, operation := range []string{"certificate", "login"} { + t.Run(operation, func(t *testing.T) { + args := []string{"ssh", operation, "ssh.example"} + keyFile := filepath.Join(t.TempDir(), "ssh-key") + if operation == "certificate" { + args = append(args, keyFile, "--host", "--no-agent", "--no-password", "--insecure") + } + certType := provisioner.SSHUserCert + if operation == "certificate" { + certType = provisioner.SSHHostCert + } + args = append(args, "--force", "--token", f.sshToken(t, certType), "--not-after", "72h") + stdout, stderr, err := f.run(t, home, args...) + require.NoError(t, err, "%s", stderr) + assertCLIWarning(t, stdout, stderr, 1) + f.assertLeaf(t, filepath.Join(home, "identity", "identity.crt"), f.issuer.NotAfter) + if operation == "certificate" { + cert := readCLISSHCertificate(t, keyFile+"-cert.pub") + require.Greater(t, time.Unix(int64(cert.ValidBefore), 0).Unix(), f.issuer.NotAfter.Unix()) + } else { + keys, err := keyring.List() + require.NoError(t, err) + require.Len(t, keys, 1) + pub, err := ssh.ParsePublicKey(keys[0].Blob) + require.NoError(t, err) + cert := pub.(*ssh.Certificate) + require.Greater(t, time.Unix(int64(cert.ValidBefore), 0).Unix(), f.issuer.NotAfter.Unix()) + } + }) + } + + for _, operation := range []string{"renew", "rekey"} { + t.Run(operation, func(t *testing.T) { + keyFile := filepath.Join(t.TempDir(), "ssh-key") + _, stderr, err := f.run(t, home, "ssh", "certificate", "ssh.example", keyFile, "--host", "--no-agent", "--no-password", "--insecure", "--token", f.sshToken(t, provisioner.SSHHostCert), "--not-after", "72h", "--force") + require.NoError(t, err, "%s", stderr) + old := readCLISSHCertificate(t, keyFile+"-cert.pub") + args := []string{"ssh", operation, keyFile + "-cert.pub", keyFile, "--force"} + if operation == "rekey" { + args = append(args, "--no-password", "--insecure") + } + stdout, stderr, err := f.run(t, home, args...) + require.NoError(t, err, "%s", stderr) + assertCLIWarning(t, stdout, stderr, 1) + f.assertLeaf(t, filepath.Join(home, "identity", "identity.crt"), f.issuer.NotAfter) + cert := readCLISSHCertificate(t, keyFile+"-cert.pub") + require.Equal(t, old.ValidBefore-old.ValidAfter, cert.ValidBefore-cert.ValidAfter) + require.Greater(t, time.Unix(int64(cert.ValidBefore), 0).Unix(), f.issuer.NotAfter.Unix()) + if operation == "rekey" { + require.NotEqual(t, old.Key.Marshal(), cert.Key.Marshal()) + } else { + require.Equal(t, old.Key.Marshal(), cert.Key.Marshal()) + } + }) + } + + t.Run("online SSH without an identity remains silent", func(t *testing.T) { + authority.GlobalVersion.RequireClientAuthentication = false + defer func() { authority.GlobalVersion.RequireClientAuthentication = true }() + home := t.TempDir() + keyFile := filepath.Join(t.TempDir(), "ssh-key") + stdout, stderr, err := f.run(t, home, "ssh", "certificate", "ssh.example", keyFile, + "--host", "--no-agent", "--no-password", "--insecure", "--not-after", "72h", "--token", f.sshToken(t, provisioner.SSHHostCert)) + require.NoError(t, err, "%s", stderr) + assertCLIWarning(t, stdout, stderr, 0) + _, err = os.Stat(filepath.Join(home, "identity", "identity.crt")) + require.True(t, os.IsNotExist(err)) + }) + + t.Run("offline SSH remains identity free", func(t *testing.T) { + home := t.TempDir() + keyFile := filepath.Join(t.TempDir(), "ssh-key") + args := []string{"ssh", "certificate", "ssh.example", keyFile, "--host", "--no-agent", "--no-password", "--insecure", "--not-after", "72h"} + args = append(args, f.offlineFlags()...) + stdout, stderr, err := f.run(t, home, args...) + require.NoError(t, err, "%s", stderr) + assertCLIWarning(t, stdout, stderr, 0) + _, err = os.Stat(filepath.Join(home, "identity", "identity.crt")) + require.True(t, os.IsNotExist(err)) + for _, operation := range []string{"renew", "rekey"} { + args = []string{"ssh", operation, keyFile + "-cert.pub", keyFile, "--force", "--offline", "--ca-config", f.configFile} + if operation == "rekey" { + args = append(args, "--no-password", "--insecure") + } + stdout, stderr, err = f.run(t, home, args...) + require.NoError(t, err, "%s", stderr) + assertCLIWarning(t, stdout, stderr, 0) + _, err = os.Stat(filepath.Join(home, "identity", "identity.crt")) + require.True(t, os.IsNotExist(err)) + } + args = []string{"ssh", "login", "ssh.example", "--force", "--not-after", "72h"} + args = append(args, f.offlineFlags()...) + stdout, stderr, err = f.run(t, home, args...) + require.NoError(t, err, "%s", stderr) + assertCLIWarning(t, stdout, stderr, 0) + _, err = os.Stat(filepath.Join(home, "identity", "identity.crt")) + require.True(t, os.IsNotExist(err)) + }) +} + +func readCLISSHCertificate(t *testing.T, path string) *ssh.Certificate { + t.Helper() + data, err := os.ReadFile(path) + require.NoError(t, err) + pub, _, _, _, err := ssh.ParseAuthorizedKey(data) + require.NoError(t, err) + cert, ok := pub.(*ssh.Certificate) + require.True(t, ok) + return cert +} + +func TestIssuerValidityAdminClient(t *testing.T) { + f := newIssuerValidityCLI(t) + t.Setenv("STEPPATH", t.TempDir()) + set := flag.NewFlagSet("admin", flag.ContinueOnError) + for name, value := range map[string]string{ + "ca-url": f.server.URL, "root": f.rootFile, "admin-subject": "admin.example", + "admin-provisioner": "issuer-validity", "provisioner-password-file": f.passwordFile, + "admin-cert": "", "admin-key": "", "password-file": "", + } { + set.String(name, value, "") + } + ctx := cli.NewContext(cli.NewApp(), set, nil) + stderr, err := captureCLIStderr(t, func() error { + _, err := cautils.NewAdminClient(ctx) + return err + }) + require.NoError(t, err) + assertCLIWarning(t, "", stderr, 1) + certFile, keyFile, _ := f.oldCredential(t, f.now, f.now.Add(time.Hour)) + require.NoError(t, ctx.Set("admin-cert", certFile)) + require.NoError(t, ctx.Set("admin-key", keyFile)) + stderr, err = captureCLIStderr(t, func() error { + _, err := cautils.NewAdminClient(ctx) + return err + }) + require.NoError(t, err) + assertCLIWarning(t, "", stderr, 0) +} + +func captureCLIStderr(t *testing.T, fn func() error) (string, error) { + t.Helper() + file, err := os.CreateTemp(t.TempDir(), "stderr") + require.NoError(t, err) + original := os.Stderr + os.Stderr = file + defer func() { os.Stderr = original; file.Close() }() + operationErr := fn() + _, err = file.Seek(0, io.SeekStart) + require.NoError(t, err) + data, err := io.ReadAll(file) + require.NoError(t, err) + return string(data), operationErr +} + +func assertCLIWarning(t *testing.T, stdout, stderr string, count int) { + t.Helper() + require.NotContains(t, stdout, "WARNING:") + require.Equal(t, count, strings.Count(stderr, "WARNING:"), "%s", stderr) + require.Equal(t, count, strings.Count(stderr, "[issuer_validity_truncated]"), "%s", stderr) +} + +func TestIssuerValidityUnavailableStderr(t *testing.T) { + f := newIssuerValidityCLI(t) + certFile, keyFile := filepath.Join(t.TempDir(), "leaf.crt"), filepath.Join(t.TempDir(), "leaf.key") + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, "step", "ca", "certificate", "native.example", certFile, keyFile, + "--not-after", "96h", "--token", f.signToken(t, "native.example"), "--ca-url", f.server.URL, "--root", f.rootFile) + cmd.Env = append(os.Environ(), "STEPPATH="+t.TempDir()) + // A read-only descriptor makes writes to the child's actual stderr fail. + stderr, err := os.Open(os.DevNull) + require.NoError(t, err) + defer stderr.Close() + var stdout bytes.Buffer + cmd.Stdout, cmd.Stderr = &stdout, stderr + require.NoError(t, cmd.Run(), "%s", stdout.String()) + require.NotContains(t, stdout.String(), "WARNING:") + leaf := f.assertLeaf(t, certFile, f.issuer.NotAfter) + pair, err := tls.LoadX509KeyPair(certFile, keyFile) + require.NoError(t, err) + require.True(t, keyutil.Equal(leaf.PublicKey, pair.PrivateKey.(crypto.Signer).Public())) +} + +func TestIssuerValidityFailedCredentialOutput(t *testing.T) { + f := newIssuerValidityCLI(t) + for _, operation := range []string{"certificate", "renew", "rekey"} { + t.Run(operation, func(t *testing.T) { + missingFile := filepath.Join(t.TempDir(), "missing", "leaf.crt") + var args []string + if operation == "certificate" { + args = []string{"ca", "certificate", "native.example", missingFile, filepath.Join(t.TempDir(), "leaf.key"), + "--not-after", "96h", "--token", f.signToken(t, "native.example")} + } else { + certFile, keyFile, _ := f.oldCredential(t, f.now, f.now.Add(32*time.Hour)) + args = []string{"ca", operation, certFile, keyFile, "--force"} + if operation == "renew" { + args = append(args, "--out", missingFile) + } else { + args = append(args, "--out-cert", missingFile, "--out-key", filepath.Join(t.TempDir(), "new.key")) + } + } + stdout, stderr, err := f.run(t, t.TempDir(), args...) + require.Error(t, err) + assertCLIWarning(t, stdout, stderr, 0) + _, err = os.Stat(missingFile) + require.True(t, os.IsNotExist(err)) + }) + } +} + +func TestIssuerValidityDaemonImmediateRenewal(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("the daemon fixture terminates with a Unix interrupt signal") + } + f := newIssuerValidityCLI(t) + certFile, keyFile, old := f.oldCredential(t, f.now.Add(-4*time.Hour), f.now.Add(28*time.Hour)) + // The fixed period is valid for the original 32h leaf, but is longer than + // the 30h issuer-capped renewal. The existing daemon consequently schedules + // another renewal immediately instead of changing the configured period. + require.Greater(t, old.NotAfter.Sub(old.NotBefore), 31*time.Hour) + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, "step", "ca", "renew", certFile, keyFile, + "--daemon", "--renew-period", "31h", "--ca-url", f.server.URL, "--root", f.rootFile) + cmd.Env = append(os.Environ(), "STEPPATH="+t.TempDir()) + stdout := &issuerValidityOutput{done: make(chan struct{}), needle: "next in 0s", count: 2} + stderr := &issuerValidityOutput{} + cmd.Stdout, cmd.Stderr = stdout, stderr + require.NoError(t, cmd.Start()) + finished := make(chan error, 1) + go func() { finished <- cmd.Wait() }() + select { + case <-stdout.done: + require.NoError(t, cmd.Process.Signal(os.Interrupt)) + require.NoError(t, <-finished) + case err := <-finished: + t.Fatalf("daemon exited before immediate rescheduling: %v\n%s\n%s", err, stdout.String(), stderr.String()) + case <-ctx.Done(): + _ = cmd.Process.Kill() + <-finished + t.Fatalf("daemon did not renew immediately: %s\n%s", stdout.String(), stderr.String()) + } + require.NotContains(t, stdout.String(), "WARNING:") + require.Contains(t, stderr.String(), "[issuer_validity_truncated]") + f.assertLeaf(t, certFile, f.issuer.NotAfter) +} + +type issuerValidityOutput struct { + sync.Mutex + buffer bytes.Buffer + done chan struct{} + needle string + count int + closed bool +} + +func (w *issuerValidityOutput) Write(p []byte) (int, error) { + w.Lock() + defer w.Unlock() + n, err := w.buffer.Write(p) + if w.done != nil && !w.closed && strings.Count(w.buffer.String(), w.needle) >= w.count { + close(w.done) + w.closed = true + } + return n, err +} + +func (w *issuerValidityOutput) String() string { + w.Lock() + defer w.Unlock() + return w.buffer.String() +} + +type issuerValidityCLI struct { + now time.Time + root, issuer, oldIssuer *x509.Certificate + issuerKey crypto.Signer + jwk *jose.JSONWebKey + server *httptest.Server + rootFile, configFile, passwordFile string +} + +func newIssuerValidityCLI(t *testing.T, configure ...func(*config.Config)) *issuerValidityCLI { + t.Helper() + now := time.Now().UTC().Truncate(time.Second) + mini, err := minica.New(minica.WithName("issuer-validity CLI")) + require.NoError(t, err) + rootTemplate := *mini.Root + rootTemplate.NotBefore, rootTemplate.NotAfter = now.Add(-time.Hour), now.Add(36*time.Hour) + rootDER, err := x509.CreateCertificate(rand.Reader, &rootTemplate, &rootTemplate, mini.RootSigner.Public(), mini.RootSigner) + require.NoError(t, err) + root, err := x509.ParseCertificate(rootDER) + require.NoError(t, err) + issuerTemplate := *mini.Intermediate + issuerTemplate.NotBefore, issuerTemplate.NotAfter = now.Add(-24*time.Hour), now.Add(48*time.Hour) + oldDER, err := x509.CreateCertificate(rand.Reader, &issuerTemplate, root, mini.Signer.Public(), mini.RootSigner) + require.NoError(t, err) + oldIssuer, err := x509.ParseCertificate(oldDER) + require.NoError(t, err) + issuerTemplate.SerialNumber = big.NewInt(3) + issuerTemplate.NotAfter = now.Add(30 * time.Hour) + issuerDER, err := x509.CreateCertificate(rand.Reader, &issuerTemplate, root, mini.Signer.Public(), mini.RootSigner) + require.NoError(t, err) + issuer, err := x509.ParseCertificate(issuerDER) + require.NoError(t, err) + jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0) + require.NoError(t, err) + publicJWK := jwk.Public() + encrypted, err := jose.EncryptJWK(jwk, []byte("issuer-validity-test-password")) + require.NoError(t, err) + encryptedKey, err := encrypted.CompactSerialize() + require.NoError(t, err) + dir := t.TempDir() + write := func(name string, value any) string { + path := filepath.Join(dir, name) + _, err := pemutil.Serialize(value, pemutil.ToFile(path, 0600)) + require.NoError(t, err) + return path + } + enableSSH := true + claims := &provisioner.Claims{ + MinTLSDur: &provisioner.Duration{Duration: 5 * time.Minute}, + DefaultTLSDur: &provisioner.Duration{Duration: 72 * time.Hour}, + MaxTLSDur: &provisioner.Duration{Duration: 168 * time.Hour}, + DefaultUserSSHDur: &provisioner.Duration{Duration: 72 * time.Hour}, + MaxUserSSHDur: &provisioner.Duration{Duration: 168 * time.Hour}, + DefaultHostSSHDur: &provisioner.Duration{Duration: 72 * time.Hour}, + EnableSSHCA: &enableSSH, + } + rootFile := write("root.crt", root) + cfg := &config.Config{ + Root: []string{rootFile}, IntermediateCert: write("issuer.crt", issuer), IntermediateKey: write("issuer.key", mini.Signer), + Address: "127.0.0.1:0", DNSNames: []string{"127.0.0.1", "localhost"}, + SSH: &config.SSHConfig{HostKey: write("ssh-host.key", mini.Signer), UserKey: write("ssh-user.key", mini.Signer)}, + AuthorityConfig: &config.AuthConfig{ + DisableIssuedAtCheck: true, + Provisioners: provisioner.List{ + &provisioner.JWK{Type: "JWK", Name: "issuer-validity", Key: &publicJWK, EncryptedKey: encryptedKey, Claims: claims}, + &provisioner.SSHPOP{Type: "SSHPOP", Name: "sshpop", Claims: claims}, + }, + }, + } + for _, fn := range configure { + fn(cfg) + } + auth, err := authority.New(cfg, authority.WithQuietInit(), + authority.WithSSHGetHosts(func(_ context.Context, cert *x509.Certificate) ([]config.Host, error) { + if cert == nil { + return nil, errs.Unauthorized("client identity required") + } + return []config.Host{{Hostname: "host.example"}}, nil + }), + ) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, auth.Shutdown()) }) + mini.Root, mini.Intermediate = root, issuer + serverKey, err := keyutil.GenerateDefaultSigner() + require.NoError(t, err) + serverCert, err := mini.Sign(&x509.Certificate{ + Subject: pkix.Name{CommonName: "localhost"}, DNSNames: []string{"localhost"}, IPAddresses: []net.IP{net.ParseIP("127.0.0.1")}, + PublicKey: serverKey.Public(), NotBefore: now.Add(-time.Minute), NotAfter: now.Add(24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + }) + require.NoError(t, err) + pool := x509.NewCertPool() + pool.AddCert(root) + router := chi.NewRouter() + api.Route(router) + server := httptest.NewUnstartedServer(router) + server.Config.BaseContext = func(net.Listener) context.Context { return authority.NewContext(context.Background(), auth) } + server.TLS = &tls.Config{ + MinVersion: tls.VersionTLS12, ClientAuth: tls.VerifyClientCertIfGiven, ClientCAs: pool, + Certificates: []tls.Certificate{{Certificate: [][]byte{serverCert.Raw, issuer.Raw}, PrivateKey: serverKey}}, + } + server.StartTLS() + t.Cleanup(server.Close) + configFile := filepath.Join(dir, "ca.json") + data, err := json.Marshal(cfg) + require.NoError(t, err) + require.NoError(t, os.WriteFile(configFile, data, 0600)) + passwordFile := filepath.Join(dir, "password") + require.NoError(t, os.WriteFile(passwordFile, []byte("issuer-validity-test-password"), 0600)) + return &issuerValidityCLI{now: now, root: root, issuer: issuer, oldIssuer: oldIssuer, issuerKey: mini.Signer, jwk: jwk, + server: server, rootFile: rootFile, configFile: configFile, passwordFile: passwordFile} +} + +func (f *issuerValidityCLI) signToken(t *testing.T, subject string) string { + t.Helper() + token, err := cautils.NewTokenGenerator(f.jwk.KeyID, "issuer-validity", f.server.URL+"/sign", "", time.Time{}, time.Time{}, f.jwk).SignToken(subject, []string{subject}) + require.NoError(t, err) + return token +} + +func (f *issuerValidityCLI) sshToken(t *testing.T, certType string) string { + t.Helper() + token, err := cautils.NewTokenGenerator(f.jwk.KeyID, "issuer-validity", f.server.URL+"/ssh/sign", "", time.Time{}, time.Time{}, f.jwk). + SignSSHToken("ssh.example", certType, []string{"ssh.example"}, provisioner.TimeDuration{}, provisioner.TimeDuration{}) + require.NoError(t, err) + return token +} + +func (f *issuerValidityCLI) offlineFlags() []string { + return []string{"--offline", "--ca-config", f.configFile, "--provisioner", "issuer-validity", "--provisioner-password-file", f.passwordFile} +} + +func (f *issuerValidityCLI) run(t *testing.T, home string, args ...string) (string, string, error) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + args = append(args, "--ca-url", f.server.URL, "--root", f.rootFile) + cmd := exec.CommandContext(ctx, "step", args...) + cmd.Env = append(os.Environ(), "STEPPATH="+home) + var stdout, stderr bytes.Buffer + cmd.Stdout, cmd.Stderr = &stdout, &stderr + err := cmd.Run() + return stdout.String(), stderr.String(), err +} + +func (f *issuerValidityCLI) assertLeaf(t *testing.T, path string, expiry time.Time) *x509.Certificate { + t.Helper() + bundle, err := pemutil.ReadCertificateBundle(path) + require.NoError(t, err) + leaf := bundle[0] + require.Equal(t, expiry, leaf.NotAfter) + require.NoError(t, leaf.CheckSignatureFrom(f.issuer)) + roots, intermediates := x509.NewCertPool(), x509.NewCertPool() + roots.AddCert(f.root) + intermediates.AddCert(f.issuer) + _, err = leaf.Verify(x509.VerifyOptions{Roots: roots, Intermediates: intermediates, CurrentTime: f.now, KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}}) + require.NoError(t, err) + return leaf +} + +func (f *issuerValidityCLI) oldCredential(t *testing.T, notBefore, notAfter time.Time) (string, string, *x509.Certificate) { + t.Helper() + // Issue a genuine provisioner-bearing leaf with the old, longer-lived + // issuer, then use it against the same-key replacement issuer. + pub := f.jwk.Public() + cfg := &config.Config{DNSNames: []string{"127.0.0.1"}, AuthorityConfig: &config.AuthConfig{ + DisableIssuedAtCheck: true, Backdate: &provisioner.Duration{}, + Provisioners: provisioner.List{&provisioner.JWK{Type: "JWK", Name: "issuer-validity", Key: &pub, Claims: &provisioner.Claims{ + MaxTLSDur: &provisioner.Duration{Duration: 168 * time.Hour}, + }}}, + }} + auth, err := authority.NewEmbedded(authority.WithConfig(cfg), authority.WithX509RootCerts(f.root), authority.WithX509Signer(f.oldIssuer, f.issuerKey), authority.WithQuietInit()) + require.NoError(t, err) + defer auth.Shutdown() + ott, err := cautils.NewTokenGenerator(f.jwk.KeyID, "issuer-validity", "https://127.0.0.1/sign", "", time.Time{}, time.Time{}, f.jwk).SignToken("native.example", []string{"native.example"}) + require.NoError(t, err) + req, key, err := ca.CreateSignRequest(ott) + require.NoError(t, err) + ctx := provisioner.NewContextWithMethod(context.Background(), provisioner.SignMethod) + ctx = apiv1.WithCertificateWarningHandler(ctx, func(w apiv1.CertificateWarning) { t.Fatalf("unexpected old-issuer warning: %s", w.Code) }) + opts, err := auth.Authorize(ctx, ott) + require.NoError(t, err) + chain, err := auth.SignWithContext(ctx, req.CsrPEM.CertificateRequest, provisioner.SignOptions{ + NotBefore: provisioner.NewTimeDuration(notBefore), NotAfter: provisioner.NewTimeDuration(notAfter), + }, opts...) + require.NoError(t, err) + dir := t.TempDir() + certFile, keyFile := filepath.Join(dir, "old.crt"), filepath.Join(dir, "old.key") + var bundle []byte + for _, cert := range chain { + bundle = append(bundle, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw})...) + } + require.NoError(t, os.WriteFile(certFile, bundle, 0600)) + _, err = pemutil.Serialize(key, pemutil.ToFile(keyFile, 0600)) + require.NoError(t, err) + return certFile, keyFile, chain[0] +} diff --git a/integration/shared_test.go b/integration/shared_test.go index ec9c2010..7ccdd36d 100644 --- a/integration/shared_test.go +++ b/integration/shared_test.go @@ -1,6 +1,10 @@ package integration import ( + "crypto/tls" + "crypto/x509" + "net/http" + "os" "testing" "github.com/rogpeppe/go-internal/testscript" @@ -10,6 +14,23 @@ import ( func TestMain(m *testing.M) { testscript.Main(m, map[string]func(){ - "step": cmd.Run, // main entrypoint name + "step": func() { + // Trust only the local OIDC fixture in these test subprocesses, + // without modifying the machine's trust store. + if root := os.Getenv("STEP_TEST_OIDC_ROOT"); root != "" { + pem, err := os.ReadFile(root) + if err != nil { + panic(err) + } + pool := x509.NewCertPool() + if !pool.AppendCertsFromPEM(pem) { + panic("invalid OIDC fixture root") + } + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.TLSClientConfig = &tls.Config{MinVersion: tls.VersionTLS12, RootCAs: pool} + http.DefaultTransport = transport + } + cmd.Run() + }, }) } diff --git a/utils/cautils/certificate_flow.go b/utils/cautils/certificate_flow.go index aa8ffac8..1b67b305 100644 --- a/utils/cautils/certificate_flow.go +++ b/utils/cautils/certificate_flow.go @@ -288,7 +288,11 @@ func (f *CertificateFlow) Sign(ctx *cli.Context, tok string, csr api.Certificate } data = append(data, pem.EncodeToMemory(pemblk)...) } - return fileutil.WriteFile(crtFile, data, 0o600) + if err := fileutil.WriteFile(crtFile, data, 0o600); err != nil { + return err + } + PrintCertificateWarnings(os.Stderr, resp.Warnings) + return nil } // CreateSignRequest is a helper function that given an x509 OTT returns a diff --git a/utils/cautils/certificate_warnings.go b/utils/cautils/certificate_warnings.go new file mode 100644 index 00000000..c11bb950 --- /dev/null +++ b/utils/cautils/certificate_warnings.go @@ -0,0 +1,39 @@ +package cautils + +import ( + "io" + "strings" + "unicode" + + "github.com/smallstep/certificates/cas/apiv1" +) + +// PrintCertificateWarnings writes CA warnings as literal, single-line terminal +// data. A warning output failure never changes successful certificate issuance. +func PrintCertificateWarnings(w io.Writer, warnings []apiv1.CertificateWarning) { + for _, warning := range warnings { + message := sanitizeCertificateWarning(warning.Message) + code := sanitizeCertificateWarning(warning.Code) + if message == "" { + if code == "" { + message = "CA returned an unspecified warning" + } else { + message = "CA returned warning " + code + } + } + if code != "" { + message += " [" + code + "]" + } + // Do not cause callers to retry issuance if stderr is unavailable. + writeCertificateWarning(w, "WARNING: "+message+"\n") + } +} + +func sanitizeCertificateWarning(text string) string { + return strings.Map(func(r rune) rune { + if unicode.IsControl(r) || unicode.In(r, unicode.Cf, unicode.Zl, unicode.Zp) { + return ' ' + } + return r + }, text) +} diff --git a/utils/cautils/certificate_warnings_other.go b/utils/cautils/certificate_warnings_other.go new file mode 100644 index 00000000..9c48646b --- /dev/null +++ b/utils/cautils/certificate_warnings_other.go @@ -0,0 +1,9 @@ +//go:build !unix + +package cautils + +import "io" + +func writeCertificateWarning(w io.Writer, line string) { + _, _ = io.WriteString(w, line) +} diff --git a/utils/cautils/certificate_warnings_test.go b/utils/cautils/certificate_warnings_test.go new file mode 100644 index 00000000..a0e9da29 --- /dev/null +++ b/utils/cautils/certificate_warnings_test.go @@ -0,0 +1,59 @@ +package cautils + +import ( + "bytes" + "errors" + "testing" + + "github.com/smallstep/certificates/cas/apiv1" + "github.com/stretchr/testify/require" +) + +func TestCertificateWarnings(t *testing.T) { + t.Run("ordered literal terminal data", func(t *testing.T) { + var output bytes.Buffer + PrintCertificateWarnings(&output, []apiv1.CertificateWarning{ + {Message: "first\n\r\t\x1b[31m\x00\x7f\u0085\u009b\u2028\u2029\u202e\u200d café", Code: "future\n\x1b\u2028\u202e"}, + {Message: `{{ "literal" | red }} %s`, Code: "unknown_code"}, + {Message: "message only"}, + }) + require.Equal(t, "WARNING: first [31m café [future ]\n"+ + "WARNING: {{ \"literal\" | red }} %s [unknown_code]\n"+ + "WARNING: message only\n", output.String()) + }) + + t.Run("empty fields have useful fallbacks", func(t *testing.T) { + var output bytes.Buffer + PrintCertificateWarnings(&output, []apiv1.CertificateWarning{ + {Code: "future_code"}, + {}, + }) + require.Equal(t, "WARNING: CA returned warning future_code [future_code]\n"+ + "WARNING: CA returned an unspecified warning\n", output.String()) + }) + + t.Run("warning free responses are silent", func(t *testing.T) { + var output bytes.Buffer + PrintCertificateWarnings(&output, nil) + PrintCertificateWarnings(&output, []apiv1.CertificateWarning{}) + require.Empty(t, output.String()) + }) + + t.Run("writer failures do not stop later warnings", func(t *testing.T) { + writer := &failingCertificateWarningWriter{} + PrintCertificateWarnings(writer, []apiv1.CertificateWarning{ + {Message: "first"}, + {Message: "second"}, + }) + require.Equal(t, []string{"WARNING: first\n", "WARNING: second\n"}, writer.attempts) + }) +} + +type failingCertificateWarningWriter struct { + attempts []string +} + +func (w *failingCertificateWarningWriter) Write(p []byte) (int, error) { + w.attempts = append(w.attempts, string(p)) + return 0, errors.New("stderr unavailable") +} diff --git a/utils/cautils/certificate_warnings_unix.go b/utils/cautils/certificate_warnings_unix.go new file mode 100644 index 00000000..bc435e6e --- /dev/null +++ b/utils/cautils/certificate_warnings_unix.go @@ -0,0 +1,43 @@ +//go:build unix + +package cautils + +import ( + "io" + "os" + + "golang.org/x/sys/unix" +) + +func writeCertificateWarning(w io.Writer, line string) { + file, ok := w.(*os.File) + if !ok { + _, _ = io.WriteString(w, line) + return + } + + // os.File.Write raises SIGPIPE on stdout/stderr after EPIPE. Use the raw + // connection instead so a failed warning cannot terminate issuance, without + // changing the process-wide signal disposition or owning the descriptor. + conn, err := file.SyscallConn() + if err != nil { + return + } + data := []byte(line) + _ = conn.Write(func(fd uintptr) bool { + for len(data) > 0 { + n, err := unix.Write(int(fd), data) + switch err { + case unix.EINTR: + continue + case unix.EAGAIN: + return false + } + if err != nil || n == 0 { + return true + } + data = data[n:] + } + return true + }) +} diff --git a/utils/cautils/certificate_warnings_unix_test.go b/utils/cautils/certificate_warnings_unix_test.go new file mode 100644 index 00000000..bbae134f --- /dev/null +++ b/utils/cautils/certificate_warnings_unix_test.go @@ -0,0 +1,37 @@ +//go:build unix + +package cautils + +import ( + "bytes" + "context" + "fmt" + "os" + "os/exec" + "testing" + "time" + + "github.com/smallstep/certificates/cas/apiv1" + "github.com/stretchr/testify/require" +) + +func TestCertificateWarningsBrokenStderr(t *testing.T) { + if os.Getenv("STEP_TEST_WARNING_BROKEN_STDERR") == "1" { + PrintCertificateWarnings(os.Stderr, []apiv1.CertificateWarning{{Message: "shortened validity"}}) + fmt.Println("credential handling continued") + os.Exit(0) + } + + reader, writer, err := os.Pipe() + require.NoError(t, err) + require.NoError(t, reader.Close()) + t.Cleanup(func() { writer.Close() }) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, os.Args[0], "-test.run=^TestCertificateWarningsBrokenStderr$") + cmd.Env = append(os.Environ(), "STEP_TEST_WARNING_BROKEN_STDERR=1") + var stdout bytes.Buffer + cmd.Stdout, cmd.Stderr = &stdout, writer + require.NoError(t, cmd.Run()) + require.Equal(t, "credential handling continued\n", stdout.String()) +} diff --git a/utils/cautils/client.go b/utils/cautils/client.go index f1801cc7..c309f65b 100644 --- a/utils/cautils/client.go +++ b/utils/cautils/client.go @@ -14,6 +14,7 @@ import ( "github.com/smallstep/certificates/api" "github.com/smallstep/certificates/authority/provisioner" "github.com/smallstep/certificates/ca" + "github.com/smallstep/certificates/cas/apiv1" "github.com/smallstep/certificates/pki" "github.com/smallstep/cli-utils/errs" "github.com/smallstep/cli-utils/ui" @@ -117,6 +118,7 @@ func NewAdminClient(ctx *cli.Context, opts ...ca.ClientOption) (*ca.AdminClient, adminKeyFile = ctx.String("admin-key") adminCert []*x509.Certificate adminKey any + warnings []apiv1.CertificateWarning ) if adminCertFile != "" || adminKeyFile != "" { if adminCertFile == "" { @@ -194,11 +196,19 @@ func NewAdminClient(ctx *cli.Context, opts ...ca.ClientOption) (*ca.AdminClient, for i, c := range signResponse.CertChainPEM { adminCert[i] = c.Certificate } + warnings = signResponse.Warnings } // Create online client opts = append([]ca.ClientOption{ca.WithRootFile(root), ca.WithAdminX5C(adminCert, adminKey, ctx.String("password-file"))}, opts...) - return ca.NewAdminClient(caURL, opts...) + client, err := ca.NewAdminClient(caURL, opts...) + if err != nil { + return nil, err + } + if len(warnings) > 0 { + PrintCertificateWarnings(os.Stderr, warnings) + } + return client, nil } diff --git a/utils/cautils/issuer_validity_test.go b/utils/cautils/issuer_validity_test.go new file mode 100644 index 00000000..a3f21ab8 --- /dev/null +++ b/utils/cautils/issuer_validity_test.go @@ -0,0 +1,139 @@ +package cautils + +import ( + "bytes" + "crypto" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "log" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/require" + "go.step.sm/crypto/jose" + "go.step.sm/crypto/keyutil" + "go.step.sm/crypto/minica" + + "github.com/smallstep/certificates/api" + "github.com/smallstep/certificates/authority" + "github.com/smallstep/certificates/authority/config" + "github.com/smallstep/certificates/authority/provisioner" + "github.com/smallstep/certificates/ca" + "github.com/smallstep/cli/token" +) + +func TestIssuerValidityOfflineWarnings(t *testing.T) { + mini, err := minica.New(minica.WithName("offline warning test")) + require.NoError(t, err) + jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0) + require.NoError(t, err) + publicJWK := jwk.Public() + cfg := &config.Config{ + DNSNames: []string{"127.0.0.1"}, + AuthorityConfig: &config.AuthConfig{ + DisableIssuedAtCheck: true, + Provisioners: provisioner.List{&provisioner.JWK{ + Type: "JWK", Name: "offline", Key: &publicJWK, + Claims: &provisioner.Claims{MaxTLSDur: &provisioner.Duration{Duration: 72 * time.Hour}}, + }}, + }, + } + selectedIssuer := mini.Intermediate + auth, err := authority.NewEmbedded(authority.WithConfig(cfg), authority.WithX509RootCerts(mini.Root), + authority.WithX509SignerFunc(func() ([]*x509.Certificate, crypto.Signer, error) { + return []*x509.Certificate{selectedIssuer}, mini.Signer, nil + }), authority.WithX509IntermediateCerts(mini.Intermediate), authority.WithQuietInit()) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, auth.Shutdown()) }) + client := &OfflineCA{authority: auth, config: *cfg} + generator := NewTokenGenerator(jwk.KeyID, "offline", "https://127.0.0.1/sign", "", time.Time{}, time.Time{}, jwk) + newRequest := func(expiry time.Time) (*api.SignRequest, any) { + ott, err := generator.SignToken("offline.example", []string{"offline.example"}) + require.NoError(t, err) + request, key, err := ca.CreateSignRequest(ott) + require.NoError(t, err) + request.NotAfter = api.NewTimeDuration(expiry) + return request, key + } + now := time.Now().UTC().Truncate(time.Second) + request, oldKey := newRequest(now.Add(18 * time.Hour)) + old, err := client.Sign(request) + require.NoError(t, err) + require.Empty(t, old.Warnings) + issuerTemplate := *mini.Intermediate + issuerTemplate.NotAfter = now.Add(12 * time.Hour) + der, err := x509.CreateCertificate(rand.Reader, &issuerTemplate, mini.Root, mini.Signer.Public(), mini.RootSigner) + require.NoError(t, err) + selectedIssuer, err = x509.ParseCertificate(der) + require.NoError(t, err) + + var logs bytes.Buffer + previousLogger := log.Writer() + log.SetOutput(&logs) + t.Cleanup(func() { log.SetOutput(previousLogger) }) + transport := &http.Transport{TLSClientConfig: &tls.Config{ + MinVersion: tls.VersionTLS12, + Certificates: []tls.Certificate{{Certificate: [][]byte{old.ServerPEM.Raw, old.CaPEM.Raw}, PrivateKey: oldKey}}, + }} + for _, operation := range []string{"sign", "renew", "token-renew", "rekey"} { + t.Run(operation, func(t *testing.T) { + var response *api.SignResponse + expectedKey := old.ServerPEM.PublicKey + var err error + switch operation { + case "sign": + request, key := newRequest(now.Add(36 * time.Hour)) + expectedKey = key.(crypto.Signer).Public() + response, err = client.Sign(request) + case "renew": + response, err = client.Renew(transport) + case "token-renew": + claims, tokenErr := token.NewClaims( + token.WithAudience(client.GetCaURL()+"/renew"), + token.WithIssuer("step-ca-client/1.0"), + token.WithSubject(old.ServerPEM.Subject.CommonName), + token.WithX5CInsecureCerts([]*x509.Certificate{old.ServerPEM.Certificate, old.CaPEM.Certificate}, oldKey), + ) + require.NoError(t, tokenErr) + ott, tokenErr := claims.Sign("", oldKey) + require.NoError(t, tokenErr) + response, err = client.RenewWithToken(ott) + case "rekey": + key, keyErr := keyutil.GenerateDefaultSigner() + require.NoError(t, keyErr) + expectedKey = key.Public() + csrDER, csrErr := x509.CreateCertificateRequest(rand.Reader, &x509.CertificateRequest{}, key) + require.NoError(t, csrErr) + csr, parseErr := x509.ParseCertificateRequest(csrDER) + require.NoError(t, parseErr) + response, err = client.Rekey(&api.RekeyRequest{CsrPEM: api.NewCertificateRequest(csr)}, transport) + } + require.NoError(t, err) + require.True(t, keyutil.Equal(expectedKey, response.ServerPEM.PublicKey)) + require.Equal(t, selectedIssuer.NotAfter, response.ServerPEM.NotAfter) + require.NoError(t, response.ServerPEM.CheckSignatureFrom(selectedIssuer)) + require.Len(t, response.Warnings, 1) + warning := response.Warnings[0] + require.Equal(t, "issuer_validity_truncated", warning.Code) + require.Equal(t, response.ServerPEM.NotAfter, warning.EffectiveNotAfter) + require.Equal(t, selectedIssuer.NotAfter, warning.LimitingCertificate.NotAfter) + require.Greater(t, warning.RequestedNotAfter.Unix(), warning.EffectiveNotAfter.Unix()) + // The warning belongs to this operation, not the singleton OfflineCA. + request, _ := newRequest(now.Add(time.Hour)) + uncapped, err := client.Sign(request) + require.NoError(t, err) + require.Empty(t, uncapped.Warnings) + }) + } + require.NotContains(t, logs.String(), "WARNING:") + + t.Run("failed operation has no success warning", func(t *testing.T) { + request, _ := newRequest(now.Add(100 * time.Hour)) + response, err := client.Sign(request) + require.Error(t, err) + require.Nil(t, response) + require.NotContains(t, logs.String(), "WARNING:") + }) +} diff --git a/utils/cautils/offline.go b/utils/cautils/offline.go index ae1588b6..a9b5d96e 100644 --- a/utils/cautils/offline.go +++ b/utils/cautils/offline.go @@ -19,6 +19,7 @@ import ( "github.com/smallstep/certificates/authority" "github.com/smallstep/certificates/authority/config" "github.com/smallstep/certificates/authority/provisioner" + "github.com/smallstep/certificates/cas/apiv1" "go.step.sm/crypto/pemutil" "go.step.sm/crypto/x509util" @@ -195,6 +196,10 @@ func (c *OfflineCA) Version() (*api.VersionResponse, error) { // intermediate. func (c *OfflineCA) Sign(req *api.SignRequest) (*api.SignResponse, error) { ctx := provisioner.NewContextWithMethod(context.Background(), provisioner.SignMethod) + var warnings []apiv1.CertificateWarning + ctx = apiv1.WithCertificateWarningHandler(ctx, func(w apiv1.CertificateWarning) { + warnings = append(warnings, w) + }) opts, err := c.authority.Authorize(ctx, req.OTT) if err != nil { return nil, err @@ -218,6 +223,7 @@ func (c *OfflineCA) Sign(req *api.SignRequest) (*api.SignResponse, error) { CaPEM: caPEM, CertChainPEM: certChainPEM, TLSOptions: c.authority.GetTLSOptions(), + Warnings: warnings, }, nil } @@ -232,7 +238,11 @@ func (c *OfflineCA) Renew(rt http.RoundTripper) (*api.SignResponse, error) { return nil, errors.Wrap(err, "error parsing certificate") } // renew cert using authority - certChain, err := c.authority.Renew(peer) + var warnings []apiv1.CertificateWarning + ctx := apiv1.WithCertificateWarningHandler(context.Background(), func(w apiv1.CertificateWarning) { + warnings = append(warnings, w) + }) + certChain, err := c.authority.RenewContext(ctx, peer, nil) if err != nil { return nil, err } @@ -246,6 +256,7 @@ func (c *OfflineCA) Renew(rt http.RoundTripper) (*api.SignResponse, error) { CaPEM: caPEM, CertChainPEM: certChainPEM, TLSOptions: c.authority.GetTLSOptions(), + Warnings: warnings, }, nil } @@ -253,11 +264,15 @@ func (c *OfflineCA) Renew(rt http.RoundTripper) (*api.SignResponse, error) { // method. It returns an api.SignResponse with the requested certificate and the // intermediate. func (c *OfflineCA) RenewWithToken(ott string) (*api.SignResponse, error) { - cert, err := c.authority.AuthorizeRenewToken(context.Background(), ott) + var warnings []apiv1.CertificateWarning + ctx := apiv1.WithCertificateWarningHandler(context.Background(), func(w apiv1.CertificateWarning) { + warnings = append(warnings, w) + }) + cert, err := c.authority.AuthorizeRenewToken(ctx, ott) if err != nil { return nil, err } - certChain, err := c.authority.Renew(cert) + certChain, err := c.authority.RenewContext(ctx, cert, nil) if err != nil { return nil, err } @@ -271,6 +286,7 @@ func (c *OfflineCA) RenewWithToken(ott string) (*api.SignResponse, error) { CaPEM: caPEM, CertChainPEM: certChainPEM, TLSOptions: c.authority.GetTLSOptions(), + Warnings: warnings, }, nil } @@ -323,7 +339,11 @@ func (c *OfflineCA) Rekey(req *api.RekeyRequest, rt http.RoundTripper) (*api.Sig } // rekey cert using authority - certChain, err := c.authority.Rekey(peer, req.CsrPEM.PublicKey) + var warnings []apiv1.CertificateWarning + ctx := apiv1.WithCertificateWarningHandler(context.Background(), func(w apiv1.CertificateWarning) { + warnings = append(warnings, w) + }) + certChain, err := c.authority.RenewContext(ctx, peer, req.CsrPEM.PublicKey) if err != nil { return nil, err } @@ -337,6 +357,7 @@ func (c *OfflineCA) Rekey(req *api.RekeyRequest, rt http.RoundTripper) (*api.Sig CaPEM: caPEM, CertChainPEM: certChainPEM, TLSOptions: c.authority.GetTLSOptions(), + Warnings: warnings, }, nil }