X Tutup
Skip to content

Commit d416862

Browse files
jsharolandshoemaker
jsha
authored and
rolandshoemaker
committed
Fix orphan-finder (letsencrypt#4507)
This creates the correct type of backend service for the OCSP generator. It also adds an invocation of orphan-finder during the integration tests. This also adds a minor safety check to SA that I hit while writing the test. Without this safety check, passing a certificate with no DNSNames to AddCertificate would result in an obscure MariaDB syntax error without enough context to track it down. In normal circumstances this shouldn't be hit, but it will be good to have a solid error message if we hit it in tests sometime. Also, this tweaks the .travis.yml so it explicitly sets BOULDER_CONFIG_DIR to test/config in the default case. Because the docker-compose run command uses -e BOULDER_CONFIG_DIR="${BOULDER_CONFIG_DIR}", we were setting a blank BOULDER_CONFIG_DIR in default case. Since the Python startservers script sets a default if BOULDER_CONFIG_DIR is not set, we haven't noticed this before. But since this test case relies on the actual environment variable, it became an issue. Fixes letsencrypt#4499
1 parent 329e415 commit d416862

File tree

12 files changed

+186
-33
lines changed

12 files changed

+186
-33
lines changed

‎.travis.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ env:
3333
#
3434
# Current Go version build tasks:
3535
#
36-
- RUN="lints integration generate rpm"
36+
- RUN="lints integration generate rpm" BOULDER_CONFIG_DIR="test/config"
3737
# Config changes that have landed in master but not yet been applied to
3838
# production can be made in boulder-config-next.json.
3939
- RUN="integration" BOULDER_CONFIG_DIR="test/config-next"

‎cmd/orphan-finder/main.go‎

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
blog "github.com/letsencrypt/boulder/log"
2727
"github.com/letsencrypt/boulder/metrics"
2828
sapb "github.com/letsencrypt/boulder/sa/proto"
29+
"google.golang.org/grpc"
2930
)
3031

3132
var usageString = `
@@ -61,7 +62,7 @@ type certificateStorage interface {
6162
}
6263

6364
type ocspGenerator interface {
64-
GenerateOCSP(context.Context, core.OCSPSigningRequest) ([]byte, error)
65+
GenerateOCSP(context.Context, *capb.GenerateOCSPRequest, ...grpc.CallOption) (*capb.OCSPResponse, error)
6566
}
6667

6768
// orphanType is a numeric identifier for the type of orphan being processed.
@@ -199,11 +200,7 @@ func storeParsedLogLine(sa certificateStorage, ca ocspGenerator, logger blog.Log
199200
logger.AuditErrf("Couldn't parse regID: %s, [%s]", err, line)
200201
return true, false, typ
201202
}
202-
// generate a fresh OCSP response
203-
ocspResponse, err := ca.GenerateOCSP(ctx, core.OCSPSigningRequest{
204-
CertDER: der,
205-
Status: string(core.OCSPStatusGood),
206-
})
203+
response, err := generateOCSP(ctx, ca, der)
207204
if err != nil {
208205
logger.AuditErrf("Couldn't generate OCSP: %s, [%s]", err, line)
209206
return true, false, typ
@@ -215,13 +212,13 @@ func storeParsedLogLine(sa certificateStorage, ca ocspGenerator, logger blog.Log
215212
issuedDate := cert.NotBefore.Add(backdateDuration)
216213
switch typ {
217214
case certOrphan:
218-
_, err = sa.AddCertificate(ctx, der, regID, ocspResponse, &issuedDate)
215+
_, err = sa.AddCertificate(ctx, der, regID, response, &issuedDate)
219216
case precertOrphan:
220217
issued := issuedDate.UnixNano()
221218
_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
222219
Der: der,
223220
RegID: &regID,
224-
Ocsp: ocspResponse,
221+
Ocsp: response,
225222
Issued: &issued,
226223
})
227224
default:
@@ -235,7 +232,24 @@ func storeParsedLogLine(sa certificateStorage, ca ocspGenerator, logger blog.Log
235232
return true, true, typ
236233
}
237234

238-
func setup(configFile string) (blog.Logger, core.StorageAuthority, core.CertificateAuthority) {
235+
func generateOCSP(ctx context.Context, ca ocspGenerator, certDER []byte) ([]byte, error) {
236+
// generate a fresh OCSP response
237+
statusGood := string(core.OCSPStatusGood)
238+
zeroInt32 := int32(0)
239+
zeroInt64 := int64(0)
240+
ocspResponse, err := ca.GenerateOCSP(ctx, &capb.GenerateOCSPRequest{
241+
CertDER: certDER,
242+
Status: &statusGood,
243+
Reason: &zeroInt32,
244+
RevokedAt: &zeroInt64,
245+
})
246+
if err != nil {
247+
return nil, err
248+
}
249+
return ocspResponse.Response, nil
250+
}
251+
252+
func setup(configFile string) (blog.Logger, core.StorageAuthority, capb.OCSPGeneratorClient) {
239253
configJSON, err := ioutil.ReadFile(configFile)
240254
cmd.FailOnError(err, "Failed to read config file")
241255
var conf config
@@ -255,7 +269,7 @@ func setup(configFile string) (blog.Logger, core.StorageAuthority, core.Certific
255269

256270
caConn, err := bgrpc.ClientSetup(conf.OCSPGeneratorService, tlsConfig, clientMetrics, cmd.Clock())
257271
cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to CA")
258-
cac := bgrpc.NewCertificateAuthorityClient(nil, capb.NewCertificateAuthorityClient(caConn))
272+
cac := capb.NewOCSPGeneratorClient(caConn)
259273

260274
backdateDuration = conf.Backdate.Duration
261275
return logger, sac, cac
@@ -298,6 +312,9 @@ func main() {
298312

299313
var certOrphansFound, certOrphansAdded, precertOrphansFound, precertOrphansAdded int64
300314
for _, line := range strings.Split(string(logData), "\n") {
315+
if line == "" {
316+
continue
317+
}
301318
found, added, typ := storeParsedLogLine(sa, ca, logger, line)
302319
var foundStat, addStat *int64
303320
switch typ {
@@ -334,21 +351,18 @@ func main() {
334351
// Because certificates are backdated we need to add the backdate duration
335352
// to find the true issued time.
336353
issuedDate := cert.NotBefore.Add(1 * backdateDuration)
337-
ocspResponse, err := ca.GenerateOCSP(ctx, core.OCSPSigningRequest{
338-
CertDER: der,
339-
Status: string(core.OCSPStatusGood),
340-
})
354+
response, err := generateOCSP(ctx, ca, der)
341355
cmd.FailOnError(err, "Generating OCSP")
342356

343357
switch typ {
344358
case certOrphan:
345-
_, err = sa.AddCertificate(ctx, der, *regID, ocspResponse, &issuedDate)
359+
_, err = sa.AddCertificate(ctx, der, *regID, response, &issuedDate)
346360
case precertOrphan:
347361
issued := issuedDate.UnixNano()
348362
_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
349363
Der: der,
350364
RegID: regID,
351-
Ocsp: ocspResponse,
365+
Ocsp: response,
352366
Issued: &issued,
353367
})
354368
default:

‎cmd/orphan-finder/main_test.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import (
88
"testing"
99
"time"
1010

11+
"google.golang.org/grpc"
12+
1113
"github.com/jmhodges/clock"
14+
capb "github.com/letsencrypt/boulder/ca/proto"
1215
"github.com/letsencrypt/boulder/core"
1316
corepb "github.com/letsencrypt/boulder/core/proto"
1417
berrors "github.com/letsencrypt/boulder/errors"
@@ -90,8 +93,10 @@ func (m *mockSA) GetPrecertificate(ctx context.Context, req *sapb.Serial) (*core
9093

9194
type mockCA struct{}
9295

93-
func (ca *mockCA) GenerateOCSP(ctx context.Context, xferObj core.OCSPSigningRequest) (ocsp []byte, err error) {
94-
return []byte("HI"), nil
96+
func (ca *mockCA) GenerateOCSP(context.Context, *capb.GenerateOCSPRequest, ...grpc.CallOption) (*capb.OCSPResponse, error) {
97+
return &capb.OCSPResponse{
98+
Response: []byte("HI"),
99+
}, nil
95100
}
96101

97102
func checkNoErrors(t *testing.T) {

‎grpc/ca-wrappers.go‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,15 @@ func (cas *CertificateAuthorityServerWrapper) IssueCertificateForPrecertificate(
112112
return CertToPB(cert), nil
113113
}
114114

115-
func (cas *CertificateAuthorityServerWrapper) GenerateOCSP(ctx context.Context, request *caPB.GenerateOCSPRequest) (*caPB.OCSPResponse, error) {
115+
func (cas *CertificateAuthorityServerWrapper) GenerateOCSP(ctx context.Context, req *caPB.GenerateOCSPRequest) (*caPB.OCSPResponse, error) {
116+
if req.CertDER == nil || req.Status == nil || req.Reason == nil || req.RevokedAt == nil {
117+
return nil, errIncompleteRequest
118+
}
116119
res, err := cas.inner.GenerateOCSP(ctx, core.OCSPSigningRequest{
117-
CertDER: request.CertDER,
118-
Status: *request.Status,
119-
Reason: revocation.Reason(*request.Reason),
120-
RevokedAt: time.Unix(0, *request.RevokedAt),
120+
CertDER: req.CertDER,
121+
Status: *req.Status,
122+
Reason: revocation.Reason(*req.Reason),
123+
RevokedAt: time.Unix(0, *req.RevokedAt),
121124
})
122125
if err != nil {
123126
return nil, err

‎grpc/client.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ import (
1818
// on the provided *tls.Config.
1919
// It dials the remote service and returns a grpc.ClientConn if successful.
2020
func ClientSetup(c *cmd.GRPCClientConfig, tlsConfig *tls.Config, metrics clientMetrics, clk clock.Clock) (*grpc.ClientConn, error) {
21+
if c == nil {
22+
return nil, errors.New("nil gRPC client config provided. JSON config is probably missing a fooService section.")
23+
}
2124
if c.ServerAddress == "" {
2225
return nil, errors.New("ServerAddress must not be empty")
2326
}

‎sa/sa.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -946,6 +946,9 @@ func deleteOrderFQDNSet(
946946
}
947947

948948
func addIssuedNames(db dbExecer, cert *x509.Certificate, isRenewal bool) error {
949+
if len(cert.DNSNames) == 0 {
950+
return berrors.InternalServerError("certificate has no DNSNames")
951+
}
949952
var qmarks []string
950953
var values []interface{}
951954
for _, name := range cert.DNSNames {

‎test/config-next/ca-b.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@
2424
"grpcOCSPGenerator": {
2525
"address": ":9096",
2626
"clientNames": [
27-
"ocsp-updater.boulder"
27+
"ocsp-updater.boulder",
28+
"orphan-finder.boulder"
2829
]
2930
},
3031
"Issuers": [{

‎test/config-next/orphan-finder.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
},
77

88
"tls": {
9-
"caCertFile": "test/grpc-creds/minica.pem",
10-
"certFile": "test/grpc-creds/orphan-finder.boulder/cert.pem",
11-
"keyFile": "test/grpc-creds/orphan-finder.boulder/key.pem"
9+
"caCertFile": "../../test/grpc-creds/minica.pem",
10+
"certFile": "../../test/grpc-creds/orphan-finder.boulder/cert.pem",
11+
"keyFile": "../../test/grpc-creds/orphan-finder.boulder/key.pem"
1212
},
1313

1414
"ocspGeneratorService": {

‎test/config/ca-a.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@
2323
"grpcOCSPGenerator": {
2424
"address": ":9096",
2525
"clientNames": [
26-
"ocsp-updater.boulder"
26+
"ocsp-updater.boulder",
27+
"orphan-finder.boulder"
2728
]
2829
},
2930
"Issuers": [{

‎test/config/ca-b.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@
2323
"grpcOCSPGenerator": {
2424
"address": ":9096",
2525
"clientNames": [
26-
"ocsp-updater.boulder"
26+
"ocsp-updater.boulder",
27+
"orphan-finder.boulder"
2728
]
2829
},
2930
"Issuers": [{

0 commit comments

Comments
 (0)
X Tutup