Skip to content

Commit bfd3f83

Browse files
aarongableaarongable
authored
Remove CFSSL issuance path (letsencrypt#5347)
Make the `NonCFSSLSigner` code path the only code path through the CA. Remove all code related to the old, CFSSL-based code path. Update tests to supply (or mock) issuers of the new kind. Remove or simplify a few tests that were testing for behavior only exhibited by the old code path, such as incrementing certain metrics. Remove code from `//cmd/` for initializing the CFSSL library. Finally, mark the `NonCFSSLSigner` feature flag itself as deprecated. Delete the portions of the vendored CFSSL code which were only used by these deleted code paths. This does not remove the CFSSL library entirely, the rest of the cleanup will follow shortly. Part of letsencrypt#5115
1 parent 26c65e7 commit bfd3f83

File tree

19 files changed

+244
-3648
lines changed

19 files changed

+244
-3648
lines changed

‎ca/ca.go‎

Lines changed: 31 additions & 345 deletions
Large diffs are not rendered by default.

‎ca/ca_test.go‎

Lines changed: 192 additions & 543 deletions
Large diffs are not rendered by default.

‎ca/testdata/tls_feature_unknown.der.csr‎

-644 Bytes
Binary file not shown.

‎cmd/boulder-ca/main.go‎

Lines changed: 5 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,18 @@
11
package main
22

33
import (
4-
"crypto"
5-
"crypto/x509"
6-
"encoding/json"
74
"flag"
85
"fmt"
9-
"io/ioutil"
106
"os"
117
"sync"
128

139
"github.com/beeker1121/goque"
14-
cfsslConfig "github.com/cloudflare/cfssl/config"
15-
"github.com/cloudflare/cfssl/helpers"
16-
pkcs11key "github.com/letsencrypt/pkcs11key/v4"
1710
"google.golang.org/grpc/health"
1811
healthpb "google.golang.org/grpc/health/grpc_health_v1"
1912

2013
"github.com/letsencrypt/boulder/ca"
2114
capb "github.com/letsencrypt/boulder/ca/proto"
2215
"github.com/letsencrypt/boulder/cmd"
23-
"github.com/letsencrypt/boulder/core"
2416
"github.com/letsencrypt/boulder/features"
2517
"github.com/letsencrypt/boulder/goodkey"
2618
bgrpc "github.com/letsencrypt/boulder/grpc"
@@ -45,30 +37,17 @@ type config struct {
4537

4638
SAService *cmd.GRPCClientConfig
4739

48-
// CFSSL contains CFSSL-specific configs as specified by that library.
49-
CFSSL cfsslConfig.Config
50-
// RSAProfile and ECDSAProfile name which of the profiles specified in the
51-
// CFSSL config should be used when issuing RSA and ECDSA certs, respectively.
52-
RSAProfile string
53-
ECDSAProfile string
54-
// Issuers contains configuration information for each issuer cert and key
55-
// this CA knows about. The first in the list is used as the default.
56-
// Only used by CFSSL.
57-
Issuers []IssuerConfig
58-
59-
// Issuance contains all information necessary to load and initialize non-CFSSL issuers.
40+
// Issuance contains all information necessary to load and initialize issuers.
6041
Issuance struct {
6142
Profile issuance.ProfileConfig
6243
Issuers []issuance.IssuerConfig
6344
IgnoredLints []string
6445
}
6546

66-
// How long issued certificates are valid for, should match expiry field
67-
// in cfssl config.
47+
// How long issued certificates are valid for.
6848
Expiry cmd.ConfigDuration
6949

70-
// How far back certificates should be backdated, should match backdate
71-
// field in cfssl config.
50+
// How far back certificates should be backdated.
7251
Backdate cmd.ConfigDuration
7352

7453
// What digits we should prepend to serials after randomly generating them.
@@ -125,91 +104,6 @@ type config struct {
125104
Syslog cmd.SyslogConfig
126105
}
127106

128-
// IssuerConfig contains info about an issuer: private key and issuer cert.
129-
// It should contain either a File path to a PEM-format private key,
130-
// or a PKCS11Config defining how to load a module for an HSM. Used by CFSSL.
131-
type IssuerConfig struct {
132-
// A file from which a pkcs11key.Config will be read and parsed, if present
133-
ConfigFile string
134-
File string
135-
PKCS11 *pkcs11key.Config
136-
CertFile string
137-
// Number of sessions to open with the HSM. For maximum performance,
138-
// this should be equal to the number of cores in the HSM. Defaults to 1.
139-
NumSessions int
140-
}
141-
142-
func loadCFSSLIssuers(configs []IssuerConfig) ([]ca.Issuer, error) {
143-
var issuers []ca.Issuer
144-
for _, issuerConfig := range configs {
145-
signer, cert, err := loadCFSSLIssuer(issuerConfig)
146-
cmd.FailOnError(err, "Couldn't load private key")
147-
issuers = append(issuers, ca.Issuer{
148-
Signer: signer,
149-
Cert: cert,
150-
})
151-
}
152-
return issuers, nil
153-
}
154-
155-
func loadCFSSLIssuer(issuerConfig IssuerConfig) (crypto.Signer, *issuance.Certificate, error) {
156-
cert, err := issuance.LoadCertificate(issuerConfig.CertFile)
157-
if err != nil {
158-
return nil, nil, err
159-
}
160-
161-
signer, err := loadCFSSLSigner(issuerConfig, cert.Certificate)
162-
if err != nil {
163-
return nil, nil, err
164-
}
165-
166-
if !core.KeyDigestEquals(signer.Public(), cert.PublicKey) {
167-
return nil, nil, fmt.Errorf("Issuer key did not match issuer cert %s", issuerConfig.CertFile)
168-
}
169-
return signer, cert, err
170-
}
171-
172-
func loadCFSSLSigner(issuerConfig IssuerConfig, cert *x509.Certificate) (crypto.Signer, error) {
173-
if issuerConfig.File != "" {
174-
keyBytes, err := ioutil.ReadFile(issuerConfig.File)
175-
if err != nil {
176-
return nil, fmt.Errorf("Could not read key file %s", issuerConfig.File)
177-
}
178-
179-
signer, err := helpers.ParsePrivateKeyPEM(keyBytes)
180-
if err != nil {
181-
return nil, err
182-
}
183-
return signer, nil
184-
}
185-
186-
var pkcs11Config *pkcs11key.Config
187-
if issuerConfig.ConfigFile != "" {
188-
contents, err := ioutil.ReadFile(issuerConfig.ConfigFile)
189-
if err != nil {
190-
return nil, err
191-
}
192-
pkcs11Config = new(pkcs11key.Config)
193-
err = json.Unmarshal(contents, pkcs11Config)
194-
if err != nil {
195-
return nil, err
196-
}
197-
} else {
198-
pkcs11Config = issuerConfig.PKCS11
199-
}
200-
if pkcs11Config.Module == "" ||
201-
pkcs11Config.TokenLabel == "" ||
202-
pkcs11Config.PIN == "" {
203-
return nil, fmt.Errorf("Missing a field in pkcs11Config %#v", pkcs11Config)
204-
}
205-
numSessions := issuerConfig.NumSessions
206-
if numSessions <= 0 {
207-
numSessions = 1
208-
}
209-
return pkcs11key.NewPool(numSessions, pkcs11Config.Module,
210-
pkcs11Config.TokenLabel, pkcs11Config.PIN, cert.PublicKey)
211-
}
212-
213107
func loadBoulderIssuers(profileConfig issuance.ProfileConfig, issuerConfigs []issuance.IssuerConfig, ignoredLints []string) ([]*issuance.Issuer, error) {
214108
issuers := make([]*issuance.Issuer, 0, len(issuerConfigs))
215109
for _, issuerConfig := range issuerConfigs {
@@ -285,15 +179,9 @@ func main() {
285179
err = pa.SetHostnamePolicyFile(c.CA.HostnamePolicyFile)
286180
cmd.FailOnError(err, "Couldn't load hostname policy file")
287181

288-
var cfsslIssuers []ca.Issuer
289182
var boulderIssuers []*issuance.Issuer
290-
if features.Enabled(features.NonCFSSLSigner) {
291-
boulderIssuers, err = loadBoulderIssuers(c.CA.Issuance.Profile, c.CA.Issuance.Issuers, c.CA.Issuance.IgnoredLints)
292-
cmd.FailOnError(err, "Couldn't load issuers")
293-
} else {
294-
cfsslIssuers, err = loadCFSSLIssuers(c.CA.Issuers)
295-
cmd.FailOnError(err, "Couldn't load issuers")
296-
}
183+
boulderIssuers, err = loadBoulderIssuers(c.CA.Issuance.Profile, c.CA.Issuance.Issuers, c.CA.Issuance.IgnoredLints)
184+
cmd.FailOnError(err, "Couldn't load issuers")
297185

298186
tlsConfig, err := c.CA.TLS.Load()
299187
cmd.FailOnError(err, "TLS config")
@@ -318,10 +206,6 @@ func main() {
318206
cai, err := ca.NewCertificateAuthorityImpl(
319207
sa,
320208
pa,
321-
c.CA.CFSSL,
322-
c.CA.RSAProfile,
323-
c.CA.ECDSAProfile,
324-
cfsslIssuers,
325209
boulderIssuers,
326210
c.CA.ECDSAAllowedAccounts,
327211
c.CA.Expiry.Duration,

‎cmd/boulder-ca/main_test.go‎

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1 @@
11
package main
2-
3-
import (
4-
"testing"
5-
)
6-
7-
func TestLoadIssuerSuccess(t *testing.T) {
8-
signer, cert, err := loadCFSSLIssuer(IssuerConfig{
9-
File: "../../test/test-ca.key",
10-
CertFile: "../../test/test-ca2.pem",
11-
})
12-
if err != nil {
13-
t.Fatal(err)
14-
}
15-
if signer == nil {
16-
t.Fatal("loadIssuer returned nil signer")
17-
}
18-
if cert == nil {
19-
t.Fatal("loadIssuer returned nil cert")
20-
}
21-
}
22-
23-
func TestLoadIssuerBadKey(t *testing.T) {
24-
_, _, err := loadCFSSLIssuer(IssuerConfig{
25-
File: "/dev/null",
26-
CertFile: "../../test/test-ca2.pem",
27-
})
28-
if err == nil {
29-
t.Fatal("loadIssuer succeeded when loading key from /dev/null")
30-
}
31-
}
32-
33-
func TestLoadIssuerBadCert(t *testing.T) {
34-
_, _, err := loadCFSSLIssuer(IssuerConfig{
35-
File: "../../test/test-ca.key",
36-
CertFile: "/dev/null",
37-
})
38-
if err == nil {
39-
t.Fatal("loadIssuer succeeded when loading key from /dev/null")
40-
}
41-
}

‎cmd/shell.go‎

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020

2121
"google.golang.org/grpc/grpclog"
2222

23-
cfsslLog "github.com/cloudflare/cfssl/log"
2423
"github.com/go-sql-driver/mysql"
2524
"github.com/prometheus/client_golang/prometheus"
2625
"github.com/prometheus/client_golang/prometheus/promhttp"
@@ -50,20 +49,6 @@ func (m mysqlLogger) Print(v ...interface{}) {
5049
m.AuditErrf("[mysql] %s", fmt.Sprint(v...))
5150
}
5251

53-
// cfsslLogger provides two additional methods that are expected by CFSSL's
54-
// logger but not supported by Boulder's Logger.
55-
type cfsslLogger struct {
56-
blog.Logger
57-
}
58-
59-
func (cl cfsslLogger) Crit(msg string) {
60-
cl.AuditErr(msg)
61-
}
62-
63-
func (cl cfsslLogger) Emerg(msg string) {
64-
cl.AuditErr(msg)
65-
}
66-
6752
type grpcLogger struct {
6853
blog.Logger
6954
}
@@ -146,7 +131,7 @@ func (lw logWriter) Write(p []byte) (n int, err error) {
146131
// server on the provided port to report the stats and provide pprof profiling
147132
// handlers. NewLogger and newStatsRegistry will call os.Exit on errors.
148133
// Also sets the constructed AuditLogger as the default logger, and configures
149-
// the cfssl, mysql, and grpc packages to use our logger.
134+
// the mysql and grpc packages to use our logger.
150135
// This must be called before any gRPC code is called, because gRPC's SetLogger
151136
// doesn't use any locking.
152137
func StatsAndLogging(logConf SyslogConfig, addr string) (prometheus.Registerer, blog.Logger) {
@@ -170,13 +155,6 @@ func NewLogger(logConf SyslogConfig) blog.Logger {
170155
FailOnError(err, "Could not connect to Syslog")
171156

172157
_ = blog.Set(logger)
173-
// We set the cfssl logging level to Debug as it
174-
// won't actually call logging methods for any
175-
// level less than what is set. We will ignore
176-
// any logging we don't care about at the syslog
177-
// level, so this doesn't cause extraneous logging.
178-
cfsslLog.Level = cfsslLog.LevelDebug
179-
cfsslLog.SetLogger(cfsslLogger{logger})
180158
_ = mysql.SetLogger(mysqlLogger{logger})
181159
grpclog.SetLoggerV2(grpcLogger{logger})
182160
log.SetOutput(logWriter{logger})

‎cmd/shell_test.go‎

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -91,38 +91,6 @@ func TestMysqlLogger(t *testing.T) {
9191
}
9292
}
9393

94-
func TestCfsslLogger(t *testing.T) {
95-
log := blog.UseMock()
96-
cLog := cfsslLogger{log}
97-
98-
testCases := []struct {
99-
msg, expected string
100-
}{
101-
{
102-
"",
103-
"ERR: [AUDIT] ",
104-
},
105-
{
106-
"Test",
107-
"ERR: [AUDIT] Test",
108-
},
109-
}
110-
111-
for _, tc := range testCases {
112-
// cfsslLogger proxies blog.AuditLogger to provide Crit() and Emerg()
113-
// methods that are expected by CFSSL's logger
114-
cLog.Crit(tc.msg)
115-
cLog.Emerg(tc.msg)
116-
logged := log.GetAll()
117-
// Calling Crit and Emerg should produce two AuditErr outputs matching the
118-
// testCase expected output
119-
test.AssertEquals(t, len(logged), 2)
120-
test.AssertEquals(t, logged[0], tc.expected)
121-
test.AssertEquals(t, logged[1], tc.expected)
122-
log.Clear()
123-
}
124-
}
125-
12694
func TestCaptureStdlibLog(t *testing.T) {
12795
logger := blog.UseMock()
12896
oldDest := log.Writer()

‎features/featureflag_string.go‎

Lines changed: 14 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎features/features.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ const (
1414
// Deprecated features, these can be removed once stripped from production configs
1515
PrecertificateRevocation
1616
StripDefaultSchemePort
17+
NonCFSSLSigner
1718

1819
// Currently in-use features
1920
// Check CAA and respect validationmethods parameter.
@@ -46,9 +47,6 @@ const (
4647
// FasterNewOrdersRateLimit enables use of a separate table for counting the
4748
// new orders rate limit.
4849
FasterNewOrdersRateLimit
49-
// NonCFSSLSigner enables usage of our own certificate signer instead of the
50-
// CFSSL signer.
51-
NonCFSSLSigner
5250
// ECDSAForAll enables all accounts, regardless of their presence in the CA's
5351
// ecdsaAllowedAccounts config value, to get issuance from ECDSA issuers.
5452
ECDSAForAll

0 commit comments

Comments
 (0)