Skip to content

Commit 5f7f4d1

Browse files
author
Daniel
committed
Merge remote-tracking branch 'le/master' into cpu-finalize-errors
2 parents c126845 + 67ae7f7 commit 5f7f4d1

23 files changed

Lines changed: 667 additions & 204 deletions

File tree

CODEOWNERS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* @letsencrypt/boulder-reviewers

cmd/cert-checker/main.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ const (
4040
// forbiddenDomains array
4141
var forbiddenDomainPatterns = []*regexp.Regexp{
4242
regexp.MustCompile(`^\s*$`),
43-
regexp.MustCompile(`\.mil$`),
4443
regexp.MustCompile(`\.local$`),
4544
regexp.MustCompile(`^localhost$`),
4645
regexp.MustCompile(`\.localhost$`),

cmd/cert-checker/main_test.go

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -172,16 +172,14 @@ func TestCheckCert(t *testing.T) {
172172
problems := checker.checkCert(cert)
173173

174174
problemsMap := map[string]int{
175-
"Stored digest doesn't match certificate digest": 1,
176-
"Stored serial doesn't match certificate serial": 1,
177-
"Stored expiration doesn't match certificate NotAfter": 1,
178-
"Certificate doesn't have basic constraints set": 1,
179-
"Certificate has a validity period longer than 2160h0m0s": 1,
180-
"Stored issuance date is outside of 6 hour window of certificate NotBefore": 1,
181-
"Certificate has incorrect key usage extensions": 1,
182-
"Certificate has common name >64 characters long (65)": 1,
183-
"Policy Authority was willing to issue but domain 'foodnotbombs.mil' " +
184-
"matches forbiddenDomains entry \"\\\\.mil$\"": 1,
175+
"Stored digest doesn't match certificate digest": 1,
176+
"Stored serial doesn't match certificate serial": 1,
177+
"Stored expiration doesn't match certificate NotAfter": 1,
178+
"Certificate doesn't have basic constraints set": 1,
179+
"Certificate has a validity period longer than 2160h0m0s": 1,
180+
"Stored issuance date is outside of 6 hour window of certificate NotBefore": 1,
181+
"Certificate has incorrect key usage extensions": 1,
182+
"Certificate has common name >64 characters long (65)": 1,
185183
"Policy Authority isn't willing to issue for '*.foodnotbombs.mil': Wildcard names not supported": 1,
186184
}
187185
for _, p := range problems {
@@ -194,7 +192,7 @@ func TestCheckCert(t *testing.T) {
194192
for k := range problemsMap {
195193
t.Errorf("Expected problem but didn't find it: '%s'.", k)
196194
}
197-
test.AssertEquals(t, len(problems), 10)
195+
test.AssertEquals(t, len(problems), 9)
198196

199197
// Same settings as above, but the stored serial number in the DB is invalid.
200198
cert.Serial = "not valid"
@@ -361,10 +359,6 @@ func TestIsForbiddenDomain(t *testing.T) {
361359
// Whitespace only
362360
{Name: "", Expected: true},
363361
{Name: " ", Expected: true},
364-
// Anything .mil
365-
{Name: "foodnotbombs.mil", Expected: true},
366-
{Name: "www.foodnotbombs.mil", Expected: true},
367-
{Name: ".mil", Expected: true},
368362
// Anything .local
369363
{Name: "yokel.local", Expected: true},
370364
{Name: "off.on.remote.local", Expected: true},

cmd/ocsp-responder/main.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,12 @@ func (src *DBSource) Response(req *ocsp.Request) ([]byte, http.Header, error) {
9090
"SELECT ocspResponse, ocspLastUpdated FROM certificateStatus WHERE serial = :serial",
9191
map[string]interface{}{"serial": serialString},
9292
)
93-
if err != nil && err != sql.ErrNoRows {
94-
src.log.AuditErr(fmt.Sprintf("Failed to retrieve response from certificateStatus table: %s", err))
93+
if err == sql.ErrNoRows {
94+
return nil, nil, cfocsp.ErrNotFound
9595
}
9696
if err != nil {
97-
return nil, nil, cfocsp.ErrNotFound
97+
src.log.AuditErr(fmt.Sprintf("Looking up OCSP response: %s", err))
98+
return nil, nil, err
9899
}
99100
if response.OCSPLastUpdated.IsZero() {
100101
src.log.Debug(fmt.Sprintf("OCSP Response not sent (ocspLastUpdated is zero) for CA=%s, Serial=%s", hex.EncodeToString(src.caKeyHash), serialString))

cmd/ocsp-responder/main_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,9 @@ func TestErrorLog(t *testing.T) {
134134
test.AssertNotError(t, err, "Failed to parse OCSP request")
135135

136136
_, _, err = src.Response(ocspReq)
137-
test.AssertEquals(t, err, cfocsp.ErrNotFound)
137+
test.AssertEquals(t, err.Error(), "Failure!")
138138

139-
test.AssertEquals(t, len(mockLog.GetAllMatching("Failed to retrieve response from certificateStatus table")), 1)
139+
test.AssertEquals(t, len(mockLog.GetAllMatching("Looking up OCSP response")), 1)
140140
}
141141

142142
func mustRead(path string) []byte {

cmd/ocsp-updater/main_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
blog "github.com/letsencrypt/boulder/log"
2222
"github.com/letsencrypt/boulder/metrics"
2323
"github.com/letsencrypt/boulder/publisher/mock_publisher"
24+
pubpb "github.com/letsencrypt/boulder/publisher/proto"
2425
"github.com/letsencrypt/boulder/revocation"
2526
"github.com/letsencrypt/boulder/sa"
2627
"github.com/letsencrypt/boulder/sa/satest"
@@ -109,6 +110,10 @@ func (p *mockPub) SubmitToSingleCT(_ context.Context, _, logPublicKey string, _
109110
return err
110111
}
111112

113+
func (p *mockPub) SubmitToSingleCTWithResult(_ context.Context, _ *pubpb.Request) (*pubpb.Result, error) {
114+
return nil, nil
115+
}
116+
112117
var log = blog.UseMock()
113118

114119
const (

core/interfaces.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
caPB "github.com/letsencrypt/boulder/ca/proto"
1313
corepb "github.com/letsencrypt/boulder/core/proto"
14+
pubpb "github.com/letsencrypt/boulder/publisher/proto"
1415
rapb "github.com/letsencrypt/boulder/ra/proto"
1516
"github.com/letsencrypt/boulder/revocation"
1617
sapb "github.com/letsencrypt/boulder/sa/proto"
@@ -134,6 +135,7 @@ type StorageGetter interface {
134135
GetOrder(ctx context.Context, req *sapb.OrderRequest) (*corepb.Order, error)
135136
GetOrderForNames(ctx context.Context, req *sapb.GetOrderForNamesRequest) (*corepb.Order, error)
136137
GetOrderAuthorizations(ctx context.Context, req *sapb.GetOrderAuthorizationsRequest) (map[string]*Authorization, error)
138+
GetValidOrderAuthorizations(ctx context.Context, req *sapb.GetValidOrderAuthorizationsRequest) (map[string]*Authorization, error)
137139
CountInvalidAuthorizations(ctx context.Context, req *sapb.CountInvalidAuthorizationsRequest) (count *sapb.Count, err error)
138140
GetAuthorizations(ctx context.Context, req *sapb.GetAuthorizationsRequest) (*sapb.Authorizations, error)
139141
}
@@ -170,4 +172,5 @@ type StorageAuthority interface {
170172
type Publisher interface {
171173
SubmitToCT(ctx context.Context, der []byte) error
172174
SubmitToSingleCT(ctx context.Context, logURL, logPublicKey string, der []byte) error
175+
SubmitToSingleCTWithResult(ctx context.Context, req *pubpb.Request) (*pubpb.Result, error)
173176
}

core/objects.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,3 +591,11 @@ type Order struct {
591591
Authorizations []Authorization
592592
Status AcmeStatus
593593
}
594+
595+
// SCTDER is a convenience type that helps differentiate what the
596+
// underlying byte slice contains
597+
type SCTDER []byte
598+
599+
// CertDER is a convenience type that helps differentiate what the
600+
// underlying byte slice contains
601+
type CertDER []byte

ctpolicy/ctpolicy.go

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package ctpolicy
2+
3+
import (
4+
"context"
5+
"errors"
6+
"fmt"
7+
8+
"github.com/letsencrypt/boulder/cmd"
9+
"github.com/letsencrypt/boulder/core"
10+
blog "github.com/letsencrypt/boulder/log"
11+
pubpb "github.com/letsencrypt/boulder/publisher/proto"
12+
)
13+
14+
// CTPolicy is used to hold information about SCTs required from various
15+
// groupings
16+
type CTPolicy struct {
17+
pub core.Publisher
18+
groups [][]cmd.LogDescription
19+
log blog.Logger
20+
}
21+
22+
// New creates a new CTPolicy struct
23+
func New(pub core.Publisher, groups [][]cmd.LogDescription, log blog.Logger) *CTPolicy {
24+
return &CTPolicy{
25+
pub: pub,
26+
groups: groups,
27+
log: log,
28+
}
29+
}
30+
31+
type result struct {
32+
sct core.SCTDER
33+
err error
34+
}
35+
36+
// race submits an SCT to each log in a group and waits for the first response back,
37+
// once it has the first SCT it cancels all of the other submissions and returns.
38+
// It allows up to len(group)-1 of the submissions to fail as we only care about
39+
// getting a single SCT.
40+
func (ctp *CTPolicy) race(ctx context.Context, cert core.CertDER, group []cmd.LogDescription) (core.SCTDER, error) {
41+
results := make(chan result, len(group))
42+
subCtx, cancel := context.WithCancel(ctx)
43+
defer cancel()
44+
for _, l := range group {
45+
go func(l cmd.LogDescription) {
46+
sct, err := ctp.pub.SubmitToSingleCTWithResult(subCtx, &pubpb.Request{
47+
LogURL: &l.URI,
48+
LogPublicKey: &l.Key,
49+
Der: cert,
50+
})
51+
if err != nil {
52+
// Only log the error if it is not a result of canceling subCtx
53+
if err != context.Canceled {
54+
ctp.log.Warning(fmt.Sprintf("ct submission to %q failed: %s", l.URI, err))
55+
}
56+
results <- result{err: err}
57+
return
58+
}
59+
results <- result{sct: sct.Sct}
60+
}(l)
61+
}
62+
63+
for i := 0; i < len(group); i++ {
64+
select {
65+
case <-ctx.Done():
66+
return nil, ctx.Err()
67+
case res := <-results:
68+
if res.sct != nil {
69+
// Return the very first SCT we get back and cancel any other
70+
// in progress work.
71+
cancel()
72+
return res.sct, nil
73+
}
74+
// We will continue waiting for an SCT until we've seen the same number
75+
// of errors as there are logs in the group as we may still get a SCT
76+
// back from another log.
77+
}
78+
}
79+
return nil, errors.New("all submissions for group failed")
80+
}
81+
82+
// GetSCTs attempts to retrieve a SCT from each configured grouping of logs and returns
83+
// the set of SCTs to the caller.
84+
func (ctp *CTPolicy) GetSCTs(ctx context.Context, cert core.CertDER) ([]core.SCTDER, error) {
85+
results := make(chan result, len(ctp.groups))
86+
subCtx, cancel := context.WithCancel(ctx)
87+
defer cancel()
88+
for _, g := range ctp.groups {
89+
go func(g []cmd.LogDescription) {
90+
sct, err := ctp.race(subCtx, cert, g)
91+
// Only one of these will be non-nil
92+
results <- result{sct: sct, err: err}
93+
}(g)
94+
}
95+
96+
var ret []core.SCTDER
97+
for i := 0; i < len(ctp.groups); i++ {
98+
res := <-results
99+
// If any one group fails to get a SCT then we fail out immediately
100+
// cancel any other in progress work as we can't continue
101+
if res.err != nil {
102+
cancel()
103+
return nil, res.err
104+
}
105+
ret = append(ret, res.sct)
106+
}
107+
return ret, nil
108+
}

ctpolicy/ctpolicy_test.go

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
package ctpolicy
2+
3+
import (
4+
"context"
5+
"errors"
6+
"testing"
7+
"time"
8+
9+
"github.com/letsencrypt/boulder/cmd"
10+
"github.com/letsencrypt/boulder/core"
11+
blog "github.com/letsencrypt/boulder/log"
12+
pubpb "github.com/letsencrypt/boulder/publisher/proto"
13+
"github.com/letsencrypt/boulder/test"
14+
)
15+
16+
type mockPub struct {
17+
}
18+
19+
func (mp *mockPub) SubmitToCT(ctx context.Context, der []byte) error {
20+
return nil
21+
}
22+
func (mp *mockPub) SubmitToSingleCT(ctx context.Context, logURL, logPublicKey string, der []byte) error {
23+
return nil
24+
}
25+
func (mp *mockPub) SubmitToSingleCTWithResult(_ context.Context, _ *pubpb.Request) (*pubpb.Result, error) {
26+
return &pubpb.Result{Sct: []byte{0}}, nil
27+
}
28+
29+
type alwaysFail struct {
30+
mockPub
31+
}
32+
33+
func (mp *alwaysFail) SubmitToSingleCTWithResult(_ context.Context, _ *pubpb.Request) (*pubpb.Result, error) {
34+
return nil, errors.New("BAD")
35+
}
36+
37+
func TestGetSCTs(t *testing.T) {
38+
expired, cancel := context.WithDeadline(context.Background(), time.Now())
39+
defer cancel()
40+
testCases := []struct {
41+
name string
42+
mock core.Publisher
43+
groups [][]cmd.LogDescription
44+
ctx context.Context
45+
result []core.SCTDER
46+
err error
47+
}{
48+
{
49+
name: "basic success case",
50+
mock: &mockPub{},
51+
groups: [][]cmd.LogDescription{
52+
{
53+
{URI: "abc", Key: "def"},
54+
{URI: "ghi", Key: "jkl"},
55+
},
56+
{
57+
{URI: "abc", Key: "def"},
58+
{URI: "ghi", Key: "jkl"},
59+
},
60+
},
61+
ctx: context.Background(),
62+
result: []core.SCTDER{[]byte{0}, []byte{0}},
63+
},
64+
{
65+
name: "basic failure case",
66+
mock: &alwaysFail{},
67+
groups: [][]cmd.LogDescription{
68+
{
69+
{URI: "abc", Key: "def"},
70+
{URI: "ghi", Key: "jkl"},
71+
},
72+
{
73+
{URI: "abc", Key: "def"},
74+
{URI: "ghi", Key: "jkl"},
75+
},
76+
},
77+
ctx: context.Background(),
78+
err: errors.New("all submissions for group failed"),
79+
},
80+
{
81+
name: "parent context timeout failure case",
82+
mock: &alwaysFail{},
83+
groups: [][]cmd.LogDescription{
84+
{
85+
{URI: "abc", Key: "def"},
86+
{URI: "ghi", Key: "jkl"},
87+
},
88+
{
89+
{URI: "abc", Key: "def"},
90+
{URI: "ghi", Key: "jkl"},
91+
},
92+
},
93+
ctx: expired,
94+
err: context.DeadlineExceeded,
95+
},
96+
}
97+
98+
for _, tc := range testCases {
99+
t.Run(tc.name, func(t *testing.T) {
100+
ctp := New(tc.mock, tc.groups, blog.NewMock())
101+
ret, err := ctp.GetSCTs(tc.ctx, []byte{0})
102+
if tc.result != nil {
103+
test.AssertDeepEquals(t, ret, tc.result)
104+
} else if tc.err != nil {
105+
test.AssertDeepEquals(t, err, tc.err)
106+
}
107+
})
108+
}
109+
}

0 commit comments

Comments
 (0)