Skip to content

Commit cd78c62

Browse files
authored
fix(ocsp): renew the delegated responder certificate after a CA re-key (#386)
A CA key rotation swaps the CA key and certificate but left the stored delegated OCSP responder in place. It was still fresh, so ensure kept serving it, and responses for leaves under the new key were signed by a responder that chains to the discarded key. loadStored now also checks the stored responder is signed by the current CA certificate and re-mints it when it isn't. The renewal ticker picks that up within 30 seconds of submit-rotation, or the first OCSP request does. Closes #383 Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> Co-authored-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>
1 parent 9ecfc83 commit cd78c62

2 files changed

Lines changed: 84 additions & 2 deletions

File tree

‎internal/init/ocsp_responder.go‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"crypto/x509/pkix"
2424
"encoding/asn1"
2525
"fmt"
26+
"log"
2627
"time"
2728

2829
"github.com/CryptOS-PKI/cryptos-node/internal/ca"
@@ -100,8 +101,14 @@ func (r *ocspResponder) ensure(ctx context.Context) (*x509.Certificate, crypto.S
100101
}
101102

102103
// loadStored returns the persisted responder cert+key when one exists, parses,
103-
// and is still outside the renewal window. Any failure (absent, unparsable,
104-
// expiring) reports ok=false so the caller re-mints.
104+
// was signed by the current CA certificate, and is still outside the renewal
105+
// window. Any failure (absent, unparsable, signed by another key, expiring)
106+
// reports ok=false so the caller re-mints.
107+
//
108+
// The signature check is what renews the responder after a CA re-key: the
109+
// rotation swaps the CA key and certificate but leaves the stored responder,
110+
// still fresh, signed by the discarded key, and a client checking against the
111+
// new CA certificate rejects responses it signs (#383).
105112
//
106113
// The window is half the lifetime a re-mint could actually get: ca.Sign caps
107114
// the responder at the issuer's notAfter, so once the issuer has less than the
@@ -118,6 +125,11 @@ func (r *ocspResponder) loadStored(ctx context.Context) (*x509.Certificate, cryp
118125
}
119126
achievable := r.validity
120127
if issuer, err := r.issuer(ctx); err == nil && issuer != nil {
128+
if err := cert.CheckSignatureFrom(issuer); err != nil {
129+
log.Printf("OCSP responder: stored certificate (serial %s) is not signed by the current CA certificate, re-minting: %v",
130+
cert.SerialNumber.Text(16), err)
131+
return nil, nil, false
132+
}
121133
achievable = min(achievable, time.Until(issuer.NotAfter))
122134
}
123135
remaining := time.Until(cert.NotAfter)

‎internal/init/ocsp_responder_test.go‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,13 @@ import (
2525
"crypto/rsa"
2626
"crypto/x509"
2727
"crypto/x509/pkix"
28+
"math/big"
2829
"sync"
2930
"testing"
3031
"time"
3132

33+
"golang.org/x/crypto/ocsp"
34+
3235
"github.com/CryptOS-PKI/cryptos-node/internal/ca"
3336
"github.com/CryptOS-PKI/cryptos-node/internal/node"
3437
"github.com/CryptOS-PKI/cryptos-node/internal/storage/etcd"
@@ -176,6 +179,73 @@ func TestOCSPResponderEnsureReusesFresh(t *testing.T) {
176179
}
177180
}
178181

182+
// After a CA re-key the stored responder is still fresh, but it was signed by
183+
// the discarded key. Responses for the new key's leaves have to chain to the
184+
// new CA certificate, so the responder must be re-minted under the new key
185+
// (#383).
186+
func TestOCSPResponderRemintsAfterCAReKey(t *testing.T) {
187+
f, mgr, ctx := newOCSPResponderFixture(t, 7*24*time.Hour)
188+
189+
before, _, err := mgr.ensure(ctx)
190+
if err != nil {
191+
t.Fatalf("ensure before re-key: %v", err)
192+
}
193+
194+
newKey, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader)
195+
if err != nil {
196+
t.Fatalf("GenerateKey new CA key: %v", err)
197+
}
198+
p := ca.Profile{
199+
Subject: f.issuer.Subject,
200+
NotBefore: time.Now().Add(-time.Minute),
201+
NotAfter: time.Now().Add(5 * 365 * 24 * time.Hour),
202+
IsCA: true,
203+
KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign,
204+
}
205+
der, _, err := ca.Sign(p, newKey.Public(), nil, newKey)
206+
if err != nil {
207+
t.Fatalf("ca.Sign re-keyed issuer: %v", err)
208+
}
209+
newIssuer, err := x509.ParseCertificate(der)
210+
if err != nil {
211+
t.Fatalf("ParseCertificate re-keyed issuer: %v", err)
212+
}
213+
f.issuer, f.caKey = newIssuer, newKey
214+
215+
after, afterKey, err := mgr.ensure(ctx)
216+
if err != nil {
217+
t.Fatalf("ensure after re-key: %v", err)
218+
}
219+
if after.SerialNumber.Cmp(before.SerialNumber) == 0 {
220+
t.Fatal("ensure kept the responder signed by the old CA key")
221+
}
222+
if err := after.CheckSignatureFrom(newIssuer); err != nil {
223+
t.Fatalf("responder does not chain to the re-keyed CA: %v", err)
224+
}
225+
226+
respDER, err := ocsp.CreateResponse(newIssuer, after, ocsp.Response{
227+
Status: ocsp.Good,
228+
SerialNumber: big.NewInt(42),
229+
ThisUpdate: time.Now(),
230+
NextUpdate: time.Now().Add(time.Hour),
231+
Certificate: after,
232+
}, afterKey)
233+
if err != nil {
234+
t.Fatalf("CreateResponse: %v", err)
235+
}
236+
if _, err := ocsp.ParseResponse(respDER, newIssuer); err != nil {
237+
t.Fatalf("a client checking against the re-keyed CA rejects the response: %v", err)
238+
}
239+
240+
again, _, err := mgr.ensure(ctx)
241+
if err != nil {
242+
t.Fatalf("ensure after re-mint: %v", err)
243+
}
244+
if again.SerialNumber.Cmp(after.SerialNumber) != 0 {
245+
t.Error("ensure re-minted the responder again after it was renewed under the new key")
246+
}
247+
}
248+
179249
func TestOCSPResponderEnsureRenewsWithinWindow(t *testing.T) {
180250
f, mgr, ctx := newOCSPResponderFixture(t, 7*24*time.Hour)
181251

0 commit comments

Comments
 (0)