Skip to content

Commit febd9f5

Browse files
authored
Bind issuers to only issue for specified profiles (#8409)
Change how the CA determines which issuer to use (the `pickIssuer` helper), to take into account the requested profile alongside the CSR's key algorithm. This simple loop allows us to greatly simplify how the CA has to track its set of issuers, resulting in some wider-reaching changes to the NewCertificateAuthorityImpl constructor, the unit tests, and boulder-ca/main.go. Make the recently-added Issuer.Profiles config field required, and add checks to ensure that all profiles are listed by at least one issuer, and that all profiles listed by an issuer are actually configured. Fixes #8390
1 parent 943372d commit febd9f5

File tree

10 files changed

+304
-248
lines changed

10 files changed

+304
-248
lines changed

ca/ca.go

Lines changed: 70 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"fmt"
1515
"math/big"
1616
mrand "math/rand/v2"
17+
"slices"
1718

1819
ct "github.com/google/certificate-transparency-go"
1920
cttls "github.com/google/certificate-transparency-go/tls"
@@ -69,15 +70,6 @@ type issuanceEventResult struct {
6970
Certificate string `json:",omitempty"`
7071
}
7172

72-
// Two maps of keys to Issuers. Lookup by PublicKeyAlgorithm is useful for
73-
// determining the set of issuers which can sign a given (pre)cert, based on its
74-
// PublicKeyAlgorithm. Lookup by NameID is useful for looking up a specific
75-
// issuer based on the issuer of a given (pre)certificate.
76-
type issuerMaps struct {
77-
byAlg map[x509.PublicKeyAlgorithm][]*issuance.Issuer
78-
byNameID map[issuance.NameID]*issuance.Issuer
79-
}
80-
8173
// caMetrics holds various metrics which are shared between caImpl and crlImpl.
8274
type caMetrics struct {
8375
signatureCount *prometheus.CounterVec
@@ -129,11 +121,11 @@ func (m *caMetrics) noteSignError(err error) {
129121
// certificateAuthorityImpl represents a CA that signs certificates.
130122
type certificateAuthorityImpl struct {
131123
capb.UnsafeCertificateAuthorityServer
132-
sa sapb.StorageAuthorityCertificateClient
133-
sctClient rapb.SCTProviderClient
134-
pa core.PolicyAuthority
135-
issuers issuerMaps
136-
certProfiles map[string]*issuance.Profile
124+
sa sapb.StorageAuthorityCertificateClient
125+
sctClient rapb.SCTProviderClient
126+
pa core.PolicyAuthority
127+
issuers []*issuance.Issuer
128+
profiles map[string]*issuance.Profile
137129

138130
// The prefix is prepended to the serial number.
139131
prefix byte
@@ -147,60 +139,14 @@ type certificateAuthorityImpl struct {
147139

148140
var _ capb.CertificateAuthorityServer = (*certificateAuthorityImpl)(nil)
149141

150-
// makeIssuerMaps processes a list of issuers into a set of maps for easy
151-
// lookup either by key algorithm (useful for picking an issuer for a precert)
152-
// or by unique ID (useful for final certs and CRLs). If two issuers with
153-
// the same unique ID are encountered, an error is returned.
154-
func makeIssuerMaps(issuers []*issuance.Issuer) (issuerMaps, error) {
155-
issuersByAlg := make(map[x509.PublicKeyAlgorithm][]*issuance.Issuer, 2)
156-
issuersByNameID := make(map[issuance.NameID]*issuance.Issuer, len(issuers))
157-
for _, issuer := range issuers {
158-
if _, found := issuersByNameID[issuer.NameID()]; found {
159-
return issuerMaps{}, fmt.Errorf("two issuers with same NameID %d (%s) configured", issuer.NameID(), issuer.Name())
160-
}
161-
issuersByNameID[issuer.NameID()] = issuer
162-
if issuer.IsActive() {
163-
issuersByAlg[issuer.KeyType()] = append(issuersByAlg[issuer.KeyType()], issuer)
164-
}
165-
}
166-
if i, ok := issuersByAlg[x509.ECDSA]; !ok || len(i) == 0 {
167-
return issuerMaps{}, errors.New("no ECDSA issuers configured")
168-
}
169-
if i, ok := issuersByAlg[x509.RSA]; !ok || len(i) == 0 {
170-
return issuerMaps{}, errors.New("no RSA issuers configured")
171-
}
172-
return issuerMaps{issuersByAlg, issuersByNameID}, nil
173-
}
174-
175-
// makeCertificateProfilesMap processes a set of named certificate issuance
176-
// profile configs into a map from name to profile.
177-
func makeCertificateProfilesMap(profiles map[string]*issuance.ProfileConfig) (map[string]*issuance.Profile, error) {
178-
if len(profiles) <= 0 {
179-
return nil, fmt.Errorf("must pass at least one certificate profile")
180-
}
181-
182-
profilesByName := make(map[string]*issuance.Profile, len(profiles))
183-
184-
for name, profileConfig := range profiles {
185-
profile, err := issuance.NewProfile(profileConfig)
186-
if err != nil {
187-
return nil, err
188-
}
189-
190-
profilesByName[name] = profile
191-
}
192-
193-
return profilesByName, nil
194-
}
195-
196142
// NewCertificateAuthorityImpl creates a CA instance that can sign certificates
197143
// from any number of issuance.Issuers and for any number of profiles.
198144
func NewCertificateAuthorityImpl(
199145
sa sapb.StorageAuthorityCertificateClient,
200146
sctService rapb.SCTProviderClient,
201147
pa core.PolicyAuthority,
202-
boulderIssuers []*issuance.Issuer,
203-
certificateProfiles map[string]*issuance.ProfileConfig,
148+
issuers []*issuance.Issuer,
149+
profiles map[string]*issuance.Profile,
204150
serialPrefix byte,
205151
maxNames int,
206152
keyPolicy goodkey.KeyPolicy,
@@ -212,33 +158,57 @@ func NewCertificateAuthorityImpl(
212158
return nil, errors.New("serial prefix must be between 0x01 (1) and 0x7f (127)")
213159
}
214160

215-
if len(boulderIssuers) == 0 {
161+
if len(issuers) == 0 {
216162
return nil, errors.New("must have at least one issuer")
217163
}
218164

219-
certProfiles, err := makeCertificateProfilesMap(certificateProfiles)
220-
if err != nil {
221-
return nil, err
165+
if len(profiles) == 0 {
166+
return nil, errors.New("must have at least one certificate profile")
222167
}
223168

224-
issuers, err := makeIssuerMaps(boulderIssuers)
225-
if err != nil {
226-
return nil, err
169+
issuableKeys := make(map[x509.PublicKeyAlgorithm]bool)
170+
issuableProfiles := make(map[string]bool)
171+
for _, issuer := range issuers {
172+
if issuer.IsActive() && len(issuer.Profiles()) == 0 {
173+
return nil, fmt.Errorf("issuer %q is active but has no profiles", issuer.Name())
174+
}
175+
176+
for _, profile := range issuer.Profiles() {
177+
_, ok := profiles[profile]
178+
if !ok {
179+
return nil, fmt.Errorf("issuer %q lists profile %q, which is not configured", issuer.Name(), profile)
180+
}
181+
issuableProfiles[profile] = true
182+
}
183+
184+
issuableKeys[issuer.KeyType()] = true
185+
}
186+
187+
for profile := range profiles {
188+
if !issuableProfiles[profile] {
189+
return nil, fmt.Errorf("profile %q configured, but no issuer lists it", profile)
190+
}
191+
}
192+
193+
for _, keyAlg := range []x509.PublicKeyAlgorithm{x509.ECDSA, x509.RSA} {
194+
if !issuableKeys[keyAlg] {
195+
return nil, fmt.Errorf("no %s issuers configured", keyAlg)
196+
}
227197
}
228198

229199
return &certificateAuthorityImpl{
230-
sa: sa,
231-
sctClient: sctService,
232-
pa: pa,
233-
issuers: issuers,
234-
certProfiles: certProfiles,
235-
prefix: serialPrefix,
236-
maxNames: maxNames,
237-
keyPolicy: keyPolicy,
238-
log: logger,
239-
metrics: metrics,
240-
tracer: otel.GetTracerProvider().Tracer("github.com/letsencrypt/boulder/ca"),
241-
clk: clk,
200+
sa: sa,
201+
sctClient: sctService,
202+
pa: pa,
203+
issuers: issuers,
204+
profiles: profiles,
205+
prefix: serialPrefix,
206+
maxNames: maxNames,
207+
keyPolicy: keyPolicy,
208+
log: logger,
209+
metrics: metrics,
210+
tracer: otel.GetTracerProvider().Tracer("github.com/letsencrypt/boulder/ca"),
211+
clk: clk,
242212
}, nil
243213
}
244214

@@ -265,7 +235,7 @@ func (ca *certificateAuthorityImpl) IssueCertificate(ctx context.Context, req *c
265235
return nil, errors.New("IssueCertificate called with a nil SCT service")
266236
}
267237

268-
profile, ok := ca.certProfiles[req.CertProfileName]
238+
profile, ok := ca.profiles[req.CertProfileName]
269239
if !ok {
270240
return nil, fmt.Errorf("incapable of using a profile named %q", req.CertProfileName)
271241
}
@@ -282,7 +252,7 @@ func (ca *certificateAuthorityImpl) IssueCertificate(ctx context.Context, req *c
282252
return nil, err
283253
}
284254

285-
issuer, err := ca.pickIssuer(csr.PublicKeyAlgorithm)
255+
issuer, err := ca.pickIssuer(req.CertProfileName, csr.PublicKeyAlgorithm)
286256
if err != nil {
287257
return nil, err
288258
}
@@ -496,13 +466,25 @@ func (ca *certificateAuthorityImpl) IssueCertificate(ctx context.Context, req *c
496466
}
497467

498468
// pickIssuer returns an issuer which is willing to issue certificates for the
499-
// given public key algorithm. If no such issuer exists, it returns
469+
// given profile and public key algorithm. If no such issuer exists, it returns
500470
// an error. If multiple such issuers exist, it selects one at random.
501-
func (ca *certificateAuthorityImpl) pickIssuer(keyAlg x509.PublicKeyAlgorithm) (*issuance.Issuer, error) {
502-
pool, ok := ca.issuers.byAlg[keyAlg]
471+
func (ca *certificateAuthorityImpl) pickIssuer(profileName string, keyAlg x509.PublicKeyAlgorithm) (*issuance.Issuer, error) {
472+
var pool []*issuance.Issuer
473+
for _, issuer := range ca.issuers {
474+
if !issuer.IsActive() {
475+
continue
476+
}
477+
if issuer.KeyType() != keyAlg {
478+
continue
479+
}
480+
if !slices.Contains(issuer.Profiles(), profileName) {
481+
continue
482+
}
483+
pool = append(pool, issuer)
484+
}
503485

504-
if !ok || len(pool) == 0 {
505-
return nil, fmt.Errorf("no issuer found for key algorithm %s", keyAlg)
486+
if len(pool) == 0 {
487+
return nil, fmt.Errorf("no issuer found for profile %q and key algorithm %s", profileName, keyAlg)
506488
}
507489

508490
return pool[mrand.IntN(len(pool))], nil

0 commit comments

Comments
 (0)