diff --git a/internal/init/ocsp_previous_test.go b/internal/init/ocsp_previous_test.go new file mode 100644 index 0000000..39738c7 --- /dev/null +++ b/internal/init/ocsp_previous_test.go @@ -0,0 +1,360 @@ +package init + +/* +Copyright The CryptOS Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import ( + "context" + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "testing" + "time" + + "golang.org/x/crypto/ocsp" + + "github.com/CryptOS-PKI/cryptos-node/internal/bootstrap" + "github.com/CryptOS-PKI/cryptos-node/internal/config" + "github.com/CryptOS-PKI/cryptos-node/internal/node" + "github.com/CryptOS-PKI/cryptos-node/internal/revocation" + "github.com/CryptOS-PKI/cryptos-node/internal/storage/etcd" +) + +// rotationRig wires an established Intermediate the way run.go does, with a +// real rekeyer and enroller, so a test can issue leaves, re-key and query /ocsp. +type rotationRig struct { + store *node.Store + revStore *revocation.Store + parent *rekeyParent + rekeyer *nodeRekeyer + signer *node.CASigner + responder *ocspResponder + ocspFn func(context.Context, []byte) ([]byte, error) +} + +func newRotationRig(t *testing.T) (*rotationRig, context.Context) { + t.Helper() + srv, err := etcd.Open(t.TempDir()) + if err != nil { + t.Fatalf("etcd.Open: %v", err) + } + t.Cleanup(func() { _ = srv.Close() }) + cli, err := srv.Client() + if err != nil { + t.Fatalf("etcd.Client: %v", err) + } + t.Cleanup(func() { _ = cli.Close() }) + store, err := node.New(cli) + if err != nil { + t.Fatalf("node.New: %v", err) + } + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + t.Cleanup(cancel) + revStore := revocation.NewStore(cli) + parent := newRekeyParent(t) + + key, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader) + if err != nil { + t.Fatalf("GenerateKey: %v", err) + } + blob, err := x509.MarshalECPrivateKey(key) + if err != nil { + t.Fatalf("MarshalECPrivateKey: %v", err) + } + pub, err := x509.MarshalPKIXPublicKey(&key.PublicKey) + if err != nil { + t.Fatalf("MarshalPKIXPublicKey: %v", err) + } + csr, err := x509.CreateCertificateRequest(rand.Reader, &x509.CertificateRequest{Subject: pkix.Name{CommonName: "Child Issuing CA"}}, key) + if err != nil { + t.Fatalf("CreateCertificateRequest: %v", err) + } + if err := store.StageSubordinate(ctx, csr, blob, pub); err != nil { + t.Fatalf("StageSubordinate: %v", err) + } + if err := store.CommitSubordinateCert(ctx, [][]byte{parent.signIntermediate(t, &key.PublicKey, ""), parent.der}); err != nil { + t.Fatalf("CommitSubordinateCert: %v", err) + } + + backend := NewSoftRootBackend() + load := func(ctx context.Context) (crypto.Signer, func(), error) { + priv, pub, _, err := store.RootKeyBlobs(ctx) + if err != nil { + return nil, nil, err + } + s, err := backend.LoadKey(priv, pub) + if err != nil { + return nil, nil, err + } + return s, func() { _ = s.Close() }, nil + } + issuer := func(ctx context.Context) (*x509.Certificate, error) { + id, err := store.Identity(ctx) + if err != nil { + return nil, err + } + return x509.ParseCertificate(id.ChainDer[0]) + } + + trust, err := bootstrap.LoadTrust(certPEM(parent.der), "") + if err != nil { + t.Fatalf("LoadTrust: %v", err) + } + enr, err := node.NewSubordinateEnroller(store, trust) + if err != nil { + t.Fatalf("NewSubordinateEnroller: %v", err) + } + rekeyer, err := newRekeyer(store, backend, rekeyConfig(), enr) + if err != nil { + t.Fatalf("newRekeyer: %v", err) + } + + cfg := &config.Config{PKI: config.PKI{Profiles: []config.CertificateProfile{ + {Name: "leaf", ValidityDays: 30, KeyUsage: []string{"digital_signature"}, ExtKeyUsage: []string{"server_auth"}}, + }}} + signer := node.NewCASigner(load, issuer, func(context.Context) (*config.Config, error) { return cfg, nil }). + WithRecorder(IssuedRecorder(revStore)) + revoker := &nodeRevoker{store: revStore, crlBuilder: revocation.NewCRLBuilder(revStore, time.Hour), load: load, issuer: issuer} + responder := newOCSPResponder(store, load, issuer, 0) + return &rotationRig{ + store: store, + revStore: revStore, + parent: parent, + rekeyer: rekeyer, + signer: signer, + responder: responder, + ocspFn: revoker.ocspFn(revocation.NewOCSPResponder(revStore), responder), + }, ctx +} + +// issueLeaf returns a new leaf and the CA certificate that issued it. +func (r *rotationRig) issueLeaf(t *testing.T, ctx context.Context) (leaf, ca *x509.Certificate) { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader) + if err != nil { + t.Fatalf("GenerateKey: %v", err) + } + csr, err := x509.CreateCertificateRequest(rand.Reader, &x509.CertificateRequest{Subject: pkix.Name{CommonName: "svc.example"}}, key) + if err != nil { + t.Fatalf("CreateCertificateRequest: %v", err) + } + chain, _, err := r.signer.IssueLeafForNames(ctx, csr, "leaf", nil) + if err != nil { + t.Fatalf("IssueLeafForNames: %v", err) + } + if leaf, err = x509.ParseCertificate(chain[0]); err != nil { + t.Fatalf("ParseCertificate leaf: %v", err) + } + if ca, err = x509.ParseCertificate(chain[1]); err != nil { + t.Fatalf("ParseCertificate CA: %v", err) + } + return leaf, ca +} + +func (r *rotationRig) ensureResponder(t *testing.T, ctx context.Context) *x509.Certificate { + t.Helper() + cert, _, err := r.responder.ensure(ctx) + if err != nil { + t.Fatalf("ensure: %v", err) + } + return cert +} + +// reKey runs a full rotation: begin, parent signature, complete. +func (r *rotationRig) reKey(t *testing.T, ctx context.Context) { + t.Helper() + csr, err := r.rekeyer.BeginRotation(ctx) + if err != nil { + t.Fatalf("BeginRotation: %v", err) + } + der := r.parent.signCSR(t, csr, "Child Issuing CA") + if _, err := r.rekeyer.CompleteRotation(ctx, [][]byte{der, r.parent.der}); err != nil { + t.Fatalf("CompleteRotation: %v", err) + } +} + +// query asks /ocsp about leaf, naming issuer in the CertID, and validates the +// response against that same issuer, as a relying party does. +func (r *rotationRig) query(t *testing.T, ctx context.Context, leaf, issuer *x509.Certificate, opts *ocsp.RequestOptions) (*ocsp.Response, error) { + t.Helper() + reqDER, err := ocsp.CreateRequest(leaf, issuer, opts) + if err != nil { + t.Fatalf("ocsp.CreateRequest: %v", err) + } + body, err := r.ocspFn(ctx, reqDER) + if err != nil { + t.Fatalf("ocspFn: %v", err) + } + return ocsp.ParseResponseForCert(body, leaf, issuer) +} + +func TestOCSPAnswersAnOldKeyLeafWithThePreviousResponder(t *testing.T) { + r, ctx := newRotationRig(t) + leaf, oldCA := r.issueLeaf(t, ctx) + oldResponder := r.ensureResponder(t, ctx) + + r.reKey(t, ctx) + + got, err := r.query(t, ctx, leaf, oldCA, nil) + if err != nil { + t.Fatalf("response does not validate against the old Intermediate: %v", err) + } + if got.Status != ocsp.Good { + t.Errorf("status = %d, want Good", got.Status) + } + if got.Certificate == nil || got.Certificate.SerialNumber.Cmp(oldResponder.SerialNumber) != 0 { + t.Errorf("response embeds responder %v, want the retired responder %s", got.Certificate, oldResponder.SerialNumber.Text(16)) + } +} + +func TestOCSPReportsARevokedOldKeyLeaf(t *testing.T) { + r, ctx := newRotationRig(t) + leaf, oldCA := r.issueLeaf(t, ctx) + r.ensureResponder(t, ctx) + r.reKey(t, ctx) + + if _, err := r.revStore.Revoke(ctx, leaf.SerialNumber.Text(16), ocsp.KeyCompromise, time.Now().UTC()); err != nil { + t.Fatalf("Revoke: %v", err) + } + + got, err := r.query(t, ctx, leaf, oldCA, nil) + if err != nil { + t.Fatalf("response does not validate against the old Intermediate: %v", err) + } + if got.Status != ocsp.Revoked || got.RevocationReason != ocsp.KeyCompromise { + t.Errorf("status = %d reason = %d, want Revoked keyCompromise", got.Status, got.RevocationReason) + } +} + +func TestOCSPCurrentKeyRequestsKeepTheCurrentResponder(t *testing.T) { + r, ctx := newRotationRig(t) + r.issueLeaf(t, ctx) + oldResponder := r.ensureResponder(t, ctx) + r.reKey(t, ctx) + + leaf, newCA := r.issueLeaf(t, ctx) + got, err := r.query(t, ctx, leaf, newCA, nil) + if err != nil { + t.Fatalf("response does not validate against the new Intermediate: %v", err) + } + if got.Certificate == nil || got.Certificate.SerialNumber.Cmp(oldResponder.SerialNumber) == 0 { + t.Error("a request naming the current key was answered by the retired responder") + } +} + +// RFC 6960 section 4.2.1: the response names the same CertID as the request, +// including its hash algorithm. +func TestOCSPEchoesTheHashAlgorithmOfTheRequest(t *testing.T) { + r, ctx := newRotationRig(t) + oldLeaf, oldCA := r.issueLeaf(t, ctx) + r.ensureResponder(t, ctx) + r.reKey(t, ctx) + newLeaf, newCA := r.issueLeaf(t, ctx) + + for _, tc := range []struct { + name string + leaf, issuer *x509.Certificate + }{ + {"previous key", oldLeaf, oldCA}, + {"current key", newLeaf, newCA}, + } { + t.Run(tc.name, func(t *testing.T) { + got, err := r.query(t, ctx, tc.leaf, tc.issuer, &ocsp.RequestOptions{Hash: crypto.SHA256}) + if err != nil { + t.Fatalf("response does not validate: %v", err) + } + if got.IssuerHash != crypto.SHA256 { + t.Errorf("response CertID hash = %v, want SHA-256", got.IssuerHash) + } + }) + } +} + +func TestSecondReKeyReplacesThePreviousResponder(t *testing.T) { + r, ctx := newRotationRig(t) + r.ensureResponder(t, ctx) + r.reKey(t, ctx) + + secondLeaf, secondCA := r.issueLeaf(t, ctx) + secondResponder := r.ensureResponder(t, ctx) + r.reKey(t, ctx) + + got, err := r.query(t, ctx, secondLeaf, secondCA, nil) + if err != nil { + t.Fatalf("response does not validate against the second key: %v", err) + } + if got.Certificate == nil || got.Certificate.SerialNumber.Cmp(secondResponder.SerialNumber) != 0 { + t.Error("the slot does not hold the responder retired by the second re-key") + } +} + +// With no OCSP traffic or ticker between two re-keys, the stored responder is +// still the one under the first key. It must not be paired with the second CA +// certificate; the slot keeps the first key's pair, which still matches. +func TestReKeyKeepsTheSlotWhenTheStoredResponderIsStale(t *testing.T) { + r, ctx := newRotationRig(t) + firstLeaf, firstCA := r.issueLeaf(t, ctx) + r.ensureResponder(t, ctx) + r.reKey(t, ctx) + r.reKey(t, ctx) + + got, err := r.query(t, ctx, firstLeaf, firstCA, nil) + if err != nil { + t.Fatalf("response does not validate against the first key: %v", err) + } + if got.Status != ocsp.Good { + t.Errorf("status = %d, want Good", got.Status) + } +} + +func TestExpiredPreviousResponderIsDropped(t *testing.T) { + r, ctx := newRotationRig(t) + retired := r.ensureResponder(t, ctx) + r.reKey(t, ctx) + + if err := r.responder.dropExpiredPrevious(ctx, retired.NotAfter.Add(-time.Minute)); err != nil { + t.Fatalf("dropExpiredPrevious before expiry: %v", err) + } + if _, _, _, ok, err := r.store.PreviousOCSPResponder(ctx); err != nil || !ok { + t.Fatalf("slot before expiry ok=%v err=%v, want it kept", ok, err) + } + + if err := r.responder.dropExpiredPrevious(ctx, retired.NotAfter.Add(time.Minute)); err != nil { + t.Fatalf("dropExpiredPrevious after expiry: %v", err) + } + if _, _, _, ok, err := r.store.PreviousOCSPResponder(ctx); err != nil || ok { + t.Fatalf("slot after expiry ok=%v err=%v, want it empty", ok, err) + } +} + +// An expired slot must not answer even if the ticker has not dropped it yet. +func TestExpiredPreviousResponderDoesNotAnswer(t *testing.T) { + r, ctx := newRotationRig(t) + leaf, oldCA := r.issueLeaf(t, ctx) + retired := r.ensureResponder(t, ctx) + r.reKey(t, ctx) + + reqDER, err := ocsp.CreateRequest(leaf, oldCA, nil) + if err != nil { + t.Fatalf("ocsp.CreateRequest: %v", err) + } + if _, ok := r.responder.previousFor(ctx, reqDER, retired.NotAfter.Add(time.Minute)); ok { + t.Error("an expired previous responder was selected") + } +} diff --git a/internal/init/ocsp_responder.go b/internal/init/ocsp_responder.go index 38765da..467014a 100644 --- a/internal/init/ocsp_responder.go +++ b/internal/init/ocsp_responder.go @@ -28,6 +28,7 @@ import ( "github.com/CryptOS-PKI/cryptos-node/internal/ca" "github.com/CryptOS-PKI/cryptos-node/internal/node" + "github.com/CryptOS-PKI/cryptos-node/internal/revocation" ) // defaultOCSPResponderValidity is the lifetime of a minted delegated OCSP @@ -192,3 +193,75 @@ func (r *ocspResponder) mint(ctx context.Context) (*x509.Certificate, crypto.Sig } return cert, key, nil } + +// previousResponder is the responder a CA re-key retired, with the old CA +// certificate it chains to. +type previousResponder struct { + cert *x509.Certificate + key crypto.Signer + issuer *x509.Certificate +} + +// loadPrevious returns the unexpired contents of the previous-responder slot, +// and the raw certificate for dropping it. ok is false when the slot is empty, +// unusable or expired. It never touches the CA key: that key is gone. +func (r *ocspResponder) loadPrevious(ctx context.Context, now time.Time) (*previousResponder, []byte, bool) { + certDER, keyBlob, caDER, ok, err := r.store.PreviousOCSPResponder(ctx) + if err != nil { + log.Printf("OCSP responder: read previous-responder slot: %v", err) + return nil, nil, false + } + if !ok { + return nil, nil, false + } + cert, err := x509.ParseCertificate(certDER) + if err != nil { + log.Printf("OCSP responder: parse previous responder: %v", err) + return nil, certDER, false + } + if now.After(cert.NotAfter) { + return nil, certDER, false + } + issuer, err := x509.ParseCertificate(caDER) + if err != nil { + log.Printf("OCSP responder: parse previous CA certificate: %v", err) + return nil, certDER, false + } + key, err := parseNodeKey(keyBlob) + if err != nil { + log.Printf("OCSP responder: parse previous responder key: %v", err) + return nil, certDER, false + } + return &previousResponder{cert: cert, key: key, issuer: issuer}, certDER, true +} + +// previousFor returns the retired responder when reqDER names the CA key it +// chains to. A malformed request, or one naming any other issuer, reports +// ok=false and takes the current-responder path. +func (r *ocspResponder) previousFor(ctx context.Context, reqDER []byte, now time.Time) (*previousResponder, bool) { + prev, _, ok := r.loadPrevious(ctx, now) + if !ok { + return nil, false + } + named, err := revocation.RequestNamesIssuer(reqDER, prev.issuer) + if err != nil { + log.Printf("OCSP responder: match request to the previous CA: %v", err) + return nil, false + } + return prev, named +} + +// dropExpiredPrevious empties the previous-responder slot once its responder +// has expired or can no longer be read, so the retired key does not outlive +// its use. +func (r *ocspResponder) dropExpiredPrevious(ctx context.Context, now time.Time) error { + _, certDER, ok := r.loadPrevious(ctx, now) + if ok || certDER == nil { + return nil + } + if err := r.store.DropPreviousOCSPResponder(ctx, certDER); err != nil { + return fmt.Errorf("init: drop previous OCSP responder: %w", err) + } + log.Printf("OCSP responder: dropped the responder kept for the previous CA key") + return nil +} diff --git a/internal/init/revocation.go b/internal/init/revocation.go index 0eb4d76..0e55abf 100644 --- a/internal/init/revocation.go +++ b/internal/init/revocation.go @@ -22,6 +22,7 @@ import ( "encoding/hex" "errors" "fmt" + "log" "time" nodev1 "github.com/CryptOS-PKI/cryptos-node/gen/go/cryptos/node/v1" @@ -225,8 +226,18 @@ func (r *nodeRevoker) caCertFn() func(ctx context.Context) ([]byte, error) { // signing with the delegated responder key rather than the CA key. If the // responder cannot be ensured (CA key unavailable) the request fails; the HTTP // layer maps that to internalError. +// +// A request naming the CA key a re-key retired is answered by the responder kept +// for that key, checked first because it needs neither the CA key nor a current +// responder. Its leaves chain only to the old CA certificate, so an answer from +// the current responder would not verify (#387). func (r *nodeRevoker) ocspFn(resp *revocation.OCSPResponder, responder *ocspResponder) func(ctx context.Context, reqDER []byte) ([]byte, error) { return func(ctx context.Context, reqDER []byte) ([]byte, error) { + now := time.Now().UTC() + if prev, ok := responder.previousFor(ctx, reqDER, now); ok { + log.Printf("OCSP: answering for the previous CA key with responder serial %s", prev.cert.SerialNumber.Text(16)) + return resp.Respond(ctx, reqDER, prev.issuer, prev.cert, prev.key, now) + } responderCert, responderKey, err := responder.ensure(ctx) if err != nil { return nil, fmt.Errorf("init: ensure OCSP responder: %w", err) @@ -235,7 +246,7 @@ func (r *nodeRevoker) ocspFn(resp *revocation.OCSPResponder, responder *ocspResp if err != nil { return nil, fmt.Errorf("init: load issuer for OCSP: %w", err) } - return resp.Respond(ctx, reqDER, issuer, responderCert, responderKey, time.Now().UTC()) + return resp.Respond(ctx, reqDER, issuer, responderCert, responderKey, now) } } diff --git a/internal/init/run.go b/internal/init/run.go index 006d465..d61a1b2 100644 --- a/internal/init/run.go +++ b/internal/init/run.go @@ -885,6 +885,9 @@ func boot(ctx context.Context, shutdown *shutdownRequests) (err error) { if _, _, err := ocspResponderMgr.ensure(ctx); err != nil { log.Printf("OCSP responder: renewal ensure failed: %v", err) } + if err := ocspResponderMgr.dropExpiredPrevious(ctx, time.Now()); err != nil { + log.Printf("OCSP responder: %v", err) + } } check() t := time.NewTicker(30 * time.Second) diff --git a/internal/node/store.go b/internal/node/store.go index bf5759f..099dfbf 100644 --- a/internal/node/store.go +++ b/internal/node/store.go @@ -19,11 +19,13 @@ limitations under the License. import ( "context" "crypto/sha256" + "crypto/x509" "encoding/hex" "encoding/json" "encoding/pem" "errors" "fmt" + "log" "strconv" "time" @@ -566,7 +568,9 @@ func (s *Store) RotationKeyBlobs(ctx context.Context) (private, public []byte, o // transaction is guarded on the rotation key-blob existing so a commit with no // staged rotation does not apply and returns ErrNoRotation, which makes commit // re-entry after a crash idempotent. The old key is discarded; certificates it -// already signed keep validating until they expire. Chain verification (that +// already signed keep validating until they expire. The delegated OCSP +// responder under the old key moves to the previous-responder slot in the same +// transaction, so OCSP keeps working for those certificates. Chain verification (that // the chain roots to the pinned parent anchor and that the leaf carries the // staged rotation key) is the caller's responsibility (the enroller verifies // before calling this). @@ -597,17 +601,24 @@ func (s *Store) CommitRotation(ctx context.Context, chainDER [][]byte) error { return ErrNoRotation } + retire, err := s.retireResponderOps(ctx) + if err != nil { + return fmt.Errorf("node: CommitRotation: %w", err) + } + + ops := []clientv3.Op{ + clientv3.OpPut(etcd.KeyRootKeyBlob, string(keyPriv)), + clientv3.OpPut(etcd.KeyRootKeyPublic, string(keyPub)), + clientv3.OpPut(etcd.KeyIdentityChain, string(chainJSON)), + clientv3.OpPut(etcd.KeyRootCert, string(leafDER)), + clientv3.OpDelete(etcd.KeyRotationCSR), + clientv3.OpDelete(etcd.KeyRotationKeyBlob), + clientv3.OpDelete(etcd.KeyRotationKeyPublic), + } + ops = append(ops, retire...) resp, err := s.cli.Txn(ctx). If(clientv3.Compare(clientv3.CreateRevision(etcd.KeyRotationKeyBlob), "!=", 0)). - Then( - clientv3.OpPut(etcd.KeyRootKeyBlob, string(keyPriv)), - clientv3.OpPut(etcd.KeyRootKeyPublic, string(keyPub)), - clientv3.OpPut(etcd.KeyIdentityChain, string(chainJSON)), - clientv3.OpPut(etcd.KeyRootCert, string(leafDER)), - clientv3.OpDelete(etcd.KeyRotationCSR), - clientv3.OpDelete(etcd.KeyRotationKeyBlob), - clientv3.OpDelete(etcd.KeyRotationKeyPublic), - ). + Then(ops...). Commit() if err != nil { return fmt.Errorf("node: CommitRotation: txn: %w", err) @@ -618,6 +629,53 @@ func (s *Store) CommitRotation(ctx context.Context, chainDER [][]byte) error { return nil } +// retireResponderOps returns the operations that move the stored OCSP responder +// and the CA certificate it chains to into the previous-responder slot. It +// returns none when there is nothing valid to keep: no responder has been +// minted, or the stored one is not signed by the CA being retired, which +// happens when two re-keys land before the responder is re-minted. Pairing that +// responder with this CA certificate would produce responses no client accepts, +// so the slot keeps what it has, a pair that still matches. +func (s *Store) retireResponderOps(ctx context.Context) ([]clientv3.Op, error) { + certDER, keyBlob, _, ok, err := s.OCSPResponder(ctx) + if err != nil { + return nil, fmt.Errorf("read OCSP responder: %w", err) + } + if !ok { + log.Printf("node: re-key: no OCSP responder stored, nothing to keep for the retiring CA key") + return nil, nil + } + caKV, ok, err := s.getKV(ctx, etcd.KeyRootCert) + if err != nil { + return nil, fmt.Errorf("read CA certificate: %w", err) + } + if !ok { + return nil, nil + } + cert, err := x509.ParseCertificate(certDER) + if err != nil { + log.Printf("node: re-key: stored OCSP responder does not parse, not keeping it: %v", err) + return nil, nil + } + caCert, err := x509.ParseCertificate(caKV.Value) + if err != nil { + log.Printf("node: re-key: CA certificate does not parse, not keeping the OCSP responder: %v", err) + return nil, nil + } + if err := cert.CheckSignatureFrom(caCert); err != nil { + log.Printf("node: re-key: OCSP responder (serial %s) is not signed by the retiring CA certificate, not keeping it: %v", + cert.SerialNumber.Text(16), err) + return nil, nil + } + log.Printf("node: re-key: keeping OCSP responder (serial %s) for the retiring CA key until %s", + cert.SerialNumber.Text(16), cert.NotAfter.UTC().Format(time.RFC3339)) + return []clientv3.Op{ + clientv3.OpPut(etcd.KeyOCSPPreviousResponderCert, string(certDER)), + clientv3.OpPut(etcd.KeyOCSPPreviousResponderKeyBlob, string(keyBlob)), + clientv3.OpPut(etcd.KeyOCSPPreviousResponderCACert, string(caKV.Value)), + }, nil +} + // CommitRenewal atomically replaces an established subordinate's CA // certificate with a re-certification of the SAME key. The chain is leaf-first // (chainDER[0] is the renewed certificate). In a single guarded transaction it @@ -788,6 +846,48 @@ func (s *Store) OCSPResponder(ctx context.Context) (certDER, keyBlob, keyPublic true, nil } +// PreviousOCSPResponder returns the responder retired by the last CA re-key: +// its certificate (DER), its private key blob, and the CA certificate (DER) it +// chains to. ok is false when the slot is empty. +func (s *Store) PreviousOCSPResponder(ctx context.Context) (certDER, keyBlob, caCertDER []byte, ok bool, err error) { + certKV, okCert, err := s.getKV(ctx, etcd.KeyOCSPPreviousResponderCert) + if err != nil { + return nil, nil, nil, false, err + } + blobKV, okBlob, err := s.getKV(ctx, etcd.KeyOCSPPreviousResponderKeyBlob) + if err != nil { + return nil, nil, nil, false, err + } + caKV, okCA, err := s.getKV(ctx, etcd.KeyOCSPPreviousResponderCACert) + if err != nil { + return nil, nil, nil, false, err + } + if !okCert || !okBlob || !okCA { + return nil, nil, nil, false, nil + } + return append([]byte(nil), certKV.Value...), + append([]byte(nil), blobKV.Value...), + append([]byte(nil), caKV.Value...), + true, nil +} + +// DropPreviousOCSPResponder empties the previous-responder slot if it still +// holds certDER, so a re-key that filled the slot in the meantime is not undone. +func (s *Store) DropPreviousOCSPResponder(ctx context.Context, certDER []byte) error { + _, err := s.cli.Txn(ctx). + If(clientv3.Compare(clientv3.Value(etcd.KeyOCSPPreviousResponderCert), "=", string(certDER))). + Then( + clientv3.OpDelete(etcd.KeyOCSPPreviousResponderCert), + clientv3.OpDelete(etcd.KeyOCSPPreviousResponderKeyBlob), + clientv3.OpDelete(etcd.KeyOCSPPreviousResponderCACert), + ). + Commit() + if err != nil { + return fmt.Errorf("node: DropPreviousOCSPResponder: txn: %w", err) + } + return nil +} + // getString returns the value at key as a string and whether it exists. func (s *Store) getString(ctx context.Context, key string) (string, bool, error) { kv, ok, err := s.getKV(ctx, key) diff --git a/internal/revocation/ocsp.go b/internal/revocation/ocsp.go index a2a046b..4e49c5c 100644 --- a/internal/revocation/ocsp.go +++ b/internal/revocation/ocsp.go @@ -17,9 +17,12 @@ limitations under the License. */ import ( + "bytes" "context" "crypto" "crypto/x509" + "crypto/x509/pkix" + "encoding/asn1" "fmt" "time" @@ -63,8 +66,11 @@ func (r *OCSPResponder) Respond(ctx context.Context, reqDER []byte, issuer, resp tmpl := ocsp.Response{ SerialNumber: req.SerialNumber, - ThisUpdate: now, - NextUpdate: now.Add(ocspNextUpdate), + // RFC 6960 section 4.2.1: the response names the requested CertID, so it + // is hashed with the algorithm the client chose. + IssuerHash: req.HashAlgorithm, + ThisUpdate: now, + NextUpdate: now.Add(ocspNextUpdate), // Embed the delegated responder certificate so a relying party can // validate that the response signer (responderKey) chains to issuer via // a cert carrying id-kp-OCSPSigning, rather than expecting the CA key to @@ -92,3 +98,27 @@ func (r *OCSPResponder) Respond(ctx context.Context, reqDER []byte, issuer, resp } return respDER, nil } + +// RequestNamesIssuer reports whether the CertID in reqDER names issuer, which +// RFC 6960 section 4.1.1 defines as both the hash of the issuer's DN and the +// hash of its public key (the BIT STRING value, without tag and length). It +// fails on a malformed request. +func RequestNamesIssuer(reqDER []byte, issuer *x509.Certificate) (bool, error) { + req, err := ocsp.ParseRequest(reqDER) + if err != nil { + return false, fmt.Errorf("revocation: RequestNamesIssuer: parse request: %w", err) + } + var spki struct { + Algorithm pkix.AlgorithmIdentifier + PublicKey asn1.BitString + } + if _, err := asn1.Unmarshal(issuer.RawSubjectPublicKeyInfo, &spki); err != nil { + return false, fmt.Errorf("revocation: RequestNamesIssuer: parse issuer public key: %w", err) + } + nameHash := req.HashAlgorithm.New() + nameHash.Write(issuer.RawSubject) + keyHash := req.HashAlgorithm.New() + keyHash.Write(spki.PublicKey.RightAlign()) + return bytes.Equal(nameHash.Sum(nil), req.IssuerNameHash) && + bytes.Equal(keyHash.Sum(nil), req.IssuerKeyHash), nil +} diff --git a/internal/revocation/ocsp_test.go b/internal/revocation/ocsp_test.go index 0205467..95b0091 100644 --- a/internal/revocation/ocsp_test.go +++ b/internal/revocation/ocsp_test.go @@ -24,6 +24,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/asn1" + "math/big" "testing" "time" @@ -141,3 +142,65 @@ func TestOCSPGoodRevokedUnknown(t *testing.T) { t.Fatalf("unknown serial status=%d want %d", parsed.Status, ocsp.Unknown) } } + +func TestRequestNamesIssuer(t *testing.T) { + newCA := func(cn string, key crypto.Signer) *x509.Certificate { + t.Helper() + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: cn}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + IsCA: true, + BasicConstraintsValid: true, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key.Public(), key) + if err != nil { + t.Fatalf("CreateCertificate: %v", err) + } + cert, err := x509.ParseCertificate(der) + if err != nil { + t.Fatalf("ParseCertificate: %v", err) + } + return cert + } + newKey := func() crypto.Signer { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader) + if err != nil { + t.Fatalf("GenerateKey: %v", err) + } + return key + } + key := newKey() + named := newCA("Issuing CA", key) + leaf := &x509.Certificate{SerialNumber: big.NewInt(7)} + + for _, hash := range []crypto.Hash{crypto.SHA1, crypto.SHA256, crypto.SHA384, crypto.SHA512} { + reqDER, err := ocsp.CreateRequest(leaf, named, &ocsp.RequestOptions{Hash: hash}) + if err != nil { + t.Fatalf("ocsp.CreateRequest %v: %v", hash, err) + } + for _, tc := range []struct { + name string + issuer *x509.Certificate + want bool + }{ + {"same name and key", named, true}, + {"same name, other key", newCA("Issuing CA", newKey()), false}, + {"other name, same key", newCA("Other CA", key), false}, + } { + got, err := RequestNamesIssuer(reqDER, tc.issuer) + if err != nil { + t.Fatalf("%v %s: %v", hash, tc.name, err) + } + if got != tc.want { + t.Errorf("%v %s = %v, want %v", hash, tc.name, got, tc.want) + } + } + } + + if _, err := RequestNamesIssuer([]byte("not a request"), named); err == nil { + t.Error("a malformed request did not fail") + } +} diff --git a/internal/storage/etcd/etcd.go b/internal/storage/etcd/etcd.go index 4e5aa7b..581b41e 100644 --- a/internal/storage/etcd/etcd.go +++ b/internal/storage/etcd/etcd.go @@ -111,6 +111,19 @@ const ( // responder key. KeyOCSPResponderKeyPublic = "/cryptos/pki/ocsp-responder/key-public" + // KeyOCSPPreviousResponderCert holds the DER of the responder retired by the + // last CA re-key. It answers OCSP for leaves of the discarded CA key until + // it expires, at most the responder validity after the re-key. + KeyOCSPPreviousResponderCert = "/cryptos/pki/ocsp-responder/previous/cert" + + // KeyOCSPPreviousResponderKeyBlob holds the retired responder's private key + // in the same encoding as KeyOCSPResponderKeyBlob. + KeyOCSPPreviousResponderKeyBlob = "/cryptos/pki/ocsp-responder/previous/key-blob" + + // KeyOCSPPreviousResponderCACert holds the DER of the CA certificate the + // retired responder chains to. + KeyOCSPPreviousResponderCACert = "/cryptos/pki/ocsp-responder/previous/ca-cert" + // PrefixCeremonyManifests stores one signed Ceremony Manifest per // ceremony run, keyed by ceremony ID under this prefix. PrefixCeremonyManifests = "/cryptos/ceremony/manifests/"