Skip to content

Commit ebba443

Browse files
aarongableaarongable
authored
Remove cmd.LoadCert in favor of core.LoadCert (letsencrypt#5165)
Having both of these very similar methods sitting around only serves to increase confusion. This removes the last few places which use `cmd.LoadCert` and replaces them with `core.LoadCert`, and deletes the method itself. Fixes letsencrypt#5163
1 parent 409fe7a commit ebba443

File tree

10 files changed

+58
-72
lines changed

10 files changed

+58
-72
lines changed

‎cmd/boulder-wfe/main.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/letsencrypt/boulder/features"
1717
"github.com/letsencrypt/boulder/goodkey"
1818
bgrpc "github.com/letsencrypt/boulder/grpc"
19+
"github.com/letsencrypt/boulder/issuance"
1920
blog "github.com/letsencrypt/boulder/log"
2021
noncepb "github.com/letsencrypt/boulder/nonce/proto"
2122
rapb "github.com/letsencrypt/boulder/ra/proto"
@@ -157,8 +158,9 @@ func main() {
157158
wfe.DirectoryCAAIdentity = c.WFE.DirectoryCAAIdentity
158159
wfe.DirectoryWebsite = c.WFE.DirectoryWebsite
159160

160-
wfe.IssuerCert, err = cmd.LoadCert(c.Common.IssuerCert)
161-
cmd.FailOnError(err, fmt.Sprintf("Couldn't read issuer cert [%s]", c.Common.IssuerCert))
161+
issuerCert, err := core.LoadCert(c.Common.IssuerCert)
162+
cmd.FailOnError(err, fmt.Sprintf("Couldn't load issuer cert [%s]", c.Common.IssuerCert))
163+
wfe.IssuerCert = &issuance.Certificate{Certificate: issuerCert}
162164

163165
logger.Infof("WFE using key policy: %#v", kp)
164166

‎cmd/boulder-wfe2/main.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/letsencrypt/boulder/features"
2020
"github.com/letsencrypt/boulder/goodkey"
2121
bgrpc "github.com/letsencrypt/boulder/grpc"
22+
"github.com/letsencrypt/boulder/issuance"
2223
blog "github.com/letsencrypt/boulder/log"
2324
noncepb "github.com/letsencrypt/boulder/nonce/proto"
2425
rapb "github.com/letsencrypt/boulder/ra/proto"
@@ -346,8 +347,9 @@ func main() {
346347
wfe.DirectoryWebsite = c.WFE.DirectoryWebsite
347348
wfe.LegacyKeyIDPrefix = c.WFE.LegacyKeyIDPrefix
348349

349-
wfe.IssuerCert, err = cmd.LoadCert(c.Common.IssuerCert)
350-
cmd.FailOnError(err, fmt.Sprintf("Couldn't read issuer cert [%s]", c.Common.IssuerCert))
350+
issuerCert, err := core.LoadCert(c.Common.IssuerCert)
351+
cmd.FailOnError(err, fmt.Sprintf("Couldn't load issuer cert [%s]", c.Common.IssuerCert))
352+
wfe.IssuerCert = &issuance.Certificate{Certificate: issuerCert}
351353

352354
logger.Infof("WFE using key policy: %#v", kp)
353355

‎cmd/shell.go‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ package cmd
33

44
import (
55
"encoding/json"
6-
"encoding/pem"
7-
"errors"
86
"expvar"
97
"fmt"
108
"io/ioutil"
@@ -252,28 +250,6 @@ func FailOnError(err error, msg string) {
252250
}
253251
}
254252

255-
// LoadCert loads a PEM-formatted certificate from the provided path, returning
256-
// it as a byte array, or an error if it couldn't be decoded.
257-
func LoadCert(path string) (cert []byte, err error) {
258-
if path == "" {
259-
err = errors.New("Issuer certificate was not provided in config.")
260-
return
261-
}
262-
pemBytes, err := ioutil.ReadFile(path)
263-
if err != nil {
264-
return
265-
}
266-
267-
block, _ := pem.Decode(pemBytes)
268-
if block == nil || block.Type != "CERTIFICATE" {
269-
err = errors.New("Invalid certificate value returned")
270-
return
271-
}
272-
273-
cert = block.Bytes
274-
return
275-
}
276-
277253
// ReadConfigFile takes a file path as an argument and attempts to
278254
// unmarshal the content of the file into a struct containing a
279255
// configuration of a boulder component.

‎cmd/shell_test.go‎

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -148,36 +148,6 @@ func TestVersionString(t *testing.T) {
148148
test.AssertEquals(t, versionStr, expected)
149149
}
150150

151-
func TestLoadCert(t *testing.T) {
152-
testCases := []struct {
153-
path string
154-
expectedErr string
155-
}{
156-
{
157-
"",
158-
"Issuer certificate was not provided in config.",
159-
},
160-
{
161-
"../does/not/exist",
162-
"open ../does/not/exist: no such file or directory",
163-
},
164-
{
165-
"./testdata/key.pem",
166-
"Invalid certificate value returned",
167-
},
168-
}
169-
170-
for _, tc := range testCases {
171-
_, err := LoadCert(tc.path)
172-
test.AssertError(t, err, fmt.Sprintf("LoadCert(%q) did not error", tc.path))
173-
test.AssertEquals(t, err.Error(), tc.expectedErr)
174-
}
175-
176-
bytes, err := LoadCert("./testdata/cert.pem")
177-
test.AssertNotError(t, err, "LoadCert(\"./testdata/cert.pem\") errored")
178-
test.AssertNotEquals(t, len(bytes), 0)
179-
}
180-
181151
func TestReadConfigFile(t *testing.T) {
182152
err := ReadConfigFile("", nil)
183153
test.AssertError(t, err, "ReadConfigFile('') did not error")

‎core/util.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -279,17 +279,20 @@ func LoadCertBundle(filename string) ([]*x509.Certificate, error) {
279279
}
280280

281281
// LoadCert loads a PEM certificate specified by filename or returns an error
282-
func LoadCert(filename string) (cert *x509.Certificate, err error) {
282+
func LoadCert(filename string) (*x509.Certificate, error) {
283283
certPEM, err := ioutil.ReadFile(filename)
284284
if err != nil {
285-
return
285+
return nil, err
286286
}
287287
block, _ := pem.Decode(certPEM)
288288
if block == nil {
289289
return nil, fmt.Errorf("No data in cert PEM file %s", filename)
290290
}
291-
cert, err = x509.ParseCertificate(block.Bytes)
292-
return
291+
cert, err := x509.ParseCertificate(block.Bytes)
292+
if err != nil {
293+
return nil, err
294+
}
295+
return cert, nil
293296
}
294297

295298
// retryJitter is used to prevent bunched retried queries from falling into lockstep

‎core/util_test.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
package core
22

33
import (
4+
"encoding/asn1"
45
"encoding/json"
56
"fmt"
67
"math"
78
"math/big"
9+
"os"
810
"sort"
911
"strings"
1012
"testing"
@@ -153,6 +155,30 @@ func TestValidSerial(t *testing.T) {
153155
test.AssertEquals(t, isValidSerial, true)
154156
}
155157

158+
func TestLoadCert(t *testing.T) {
159+
var osPathErr *os.PathError
160+
_, err := LoadCert("")
161+
test.AssertError(t, err, "Loading empty path did not error")
162+
test.AssertErrorWraps(t, err, &osPathErr)
163+
164+
_, err = LoadCert("totally/fake/path")
165+
test.AssertError(t, err, "Loading nonexistent path did not error")
166+
test.AssertErrorWraps(t, err, &osPathErr)
167+
168+
_, err = LoadCert("../test/test-ca.der")
169+
test.AssertError(t, err, "Loading non-PEM file did not error")
170+
test.AssertEquals(t, err.Error(), "No data in cert PEM file ../test/test-ca.der")
171+
172+
var asnStructuralErr asn1.StructuralError
173+
_, err = LoadCert("../test/test-ca.key")
174+
test.AssertError(t, err, "Loading non-cert file did not error")
175+
test.AssertErrorWraps(t, err, &asnStructuralErr)
176+
177+
cert, err := LoadCert("../test/test-ca.pem")
178+
test.AssertNotError(t, err, "Failed to load cert file")
179+
test.AssertEquals(t, cert.Subject.CommonName, "happy hacker fake CA")
180+
}
181+
156182
func TestRetryBackoff(t *testing.T) {
157183
assertBetween := func(a, b, c float64) {
158184
t.Helper()

‎wfe/wfe.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/letsencrypt/boulder/goodkey"
2727
bgrpc "github.com/letsencrypt/boulder/grpc"
2828
"github.com/letsencrypt/boulder/identifier"
29+
"github.com/letsencrypt/boulder/issuance"
2930
blog "github.com/letsencrypt/boulder/log"
3031
"github.com/letsencrypt/boulder/metrics/measured_http"
3132
"github.com/letsencrypt/boulder/nonce"
@@ -74,8 +75,8 @@ type WebFrontEndImpl struct {
7475
// URL configuration parameters
7576
BaseURL string
7677

77-
// Issuer certificate (DER) for /acme/issuer-cert
78-
IssuerCert []byte
78+
// Issuer certificate for /acme/issuer-cert
79+
IssuerCert *issuance.Certificate
7980

8081
// URL to the current subscriber agreement (should contain some version identifier)
8182
SubscriberAgreementURL string
@@ -1499,7 +1500,7 @@ func (wfe *WebFrontEndImpl) Issuer(ctx context.Context, logEvent *web.RequestEve
14991500
// TODO Content negotiation
15001501
response.Header().Set("Content-Type", "application/pkix-cert")
15011502
response.WriteHeader(http.StatusOK)
1502-
if _, err := response.Write(wfe.IssuerCert); err != nil {
1503+
if _, err := response.Write(wfe.IssuerCert.Raw); err != nil {
15031504
wfe.log.Warningf("Could not write response: %s", err)
15041505
}
15051506
}

‎wfe/wfe_test.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"github.com/letsencrypt/boulder/features"
3030
"github.com/letsencrypt/boulder/goodkey"
3131
"github.com/letsencrypt/boulder/identifier"
32+
"github.com/letsencrypt/boulder/issuance"
3233
blog "github.com/letsencrypt/boulder/log"
3334
"github.com/letsencrypt/boulder/metrics"
3435
"github.com/letsencrypt/boulder/mocks"
@@ -2131,15 +2132,16 @@ func TestTermsRedirect(t *testing.T) {
21312132

21322133
func TestIssuer(t *testing.T) {
21332134
wfe, _ := setupWFE(t)
2134-
wfe.IssuerCert = []byte{0, 0, 1}
2135+
wfe.IssuerCert = &issuance.Certificate{Certificate: &x509.Certificate{}}
2136+
wfe.IssuerCert.Raw = []byte{0, 0, 1}
21352137

21362138
responseWriter := httptest.NewRecorder()
21372139

21382140
wfe.Issuer(ctx, newRequestEvent(), responseWriter, &http.Request{
21392141
Method: "GET",
21402142
})
21412143
test.AssertEquals(t, responseWriter.Code, http.StatusOK)
2142-
test.Assert(t, bytes.Compare(responseWriter.Body.Bytes(), wfe.IssuerCert) == 0, "Incorrect bytes returned")
2144+
test.Assert(t, bytes.Compare(responseWriter.Body.Bytes(), wfe.IssuerCert.Raw) == 0, "Incorrect bytes returned")
21432145
}
21442146

21452147
func TestGetCertificate(t *testing.T) {

‎wfe2/wfe.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/letsencrypt/boulder/goodkey"
2424
bgrpc "github.com/letsencrypt/boulder/grpc"
2525
"github.com/letsencrypt/boulder/identifier"
26+
"github.com/letsencrypt/boulder/issuance"
2627
blog "github.com/letsencrypt/boulder/log"
2728
"github.com/letsencrypt/boulder/metrics/measured_http"
2829
"github.com/letsencrypt/boulder/nonce"
@@ -76,8 +77,8 @@ type WebFrontEndImpl struct {
7677
clk clock.Clock
7778
stats wfe2Stats
7879

79-
// Issuer certificate (DER) for /acme/issuer-cert
80-
IssuerCert []byte
80+
// Issuer certificate for /acme/issuer-cert
81+
IssuerCert *issuance.Certificate
8182

8283
// certificateChains maps AIA issuer URLs to a slice of []byte containing a leading
8384
// newline and one or more PEM encoded certificates separated by a newline,
@@ -1705,7 +1706,7 @@ func (wfe *WebFrontEndImpl) Issuer(ctx context.Context, logEvent *web.RequestEve
17051706
// TODO Content negotiation
17061707
response.Header().Set("Content-Type", "application/pkix-cert")
17071708
response.WriteHeader(http.StatusOK)
1708-
if _, err := response.Write(wfe.IssuerCert); err != nil {
1709+
if _, err := response.Write(wfe.IssuerCert.Raw); err != nil {
17091710
wfe.log.Warningf("Could not write response: %s", err)
17101711
}
17111712
}

‎wfe2/wfe_test.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/letsencrypt/boulder/features"
3434
"github.com/letsencrypt/boulder/goodkey"
3535
"github.com/letsencrypt/boulder/identifier"
36+
"github.com/letsencrypt/boulder/issuance"
3637
blog "github.com/letsencrypt/boulder/log"
3738
"github.com/letsencrypt/boulder/metrics"
3839
"github.com/letsencrypt/boulder/mocks"
@@ -911,6 +912,7 @@ func TestNonceEndpoint(t *testing.T) {
911912

912913
func TestHTTPMethods(t *testing.T) {
913914
wfe, _ := setupWFE(t)
915+
wfe.IssuerCert = &issuance.Certificate{Certificate: &x509.Certificate{}}
914916
mux := wfe.Handler(metrics.NoopRegisterer)
915917

916918
// NOTE: Boulder's muxer treats HEAD as implicitly allowed if GET is specified
@@ -1744,15 +1746,16 @@ func TestAccount(t *testing.T) {
17441746

17451747
func TestIssuer(t *testing.T) {
17461748
wfe, _ := setupWFE(t)
1747-
wfe.IssuerCert = []byte{0, 0, 1}
1749+
wfe.IssuerCert = &issuance.Certificate{Certificate: &x509.Certificate{}}
1750+
wfe.IssuerCert.Raw = []byte{0, 0, 1}
17481751

17491752
responseWriter := httptest.NewRecorder()
17501753

17511754
wfe.Issuer(ctx, newRequestEvent(), responseWriter, &http.Request{
17521755
Method: "GET",
17531756
})
17541757
test.AssertEquals(t, responseWriter.Code, http.StatusOK)
1755-
test.Assert(t, bytes.Compare(responseWriter.Body.Bytes(), wfe.IssuerCert) == 0, "Incorrect bytes returned")
1758+
test.Assert(t, bytes.Compare(responseWriter.Body.Bytes(), wfe.IssuerCert.Raw) == 0, "Incorrect bytes returned")
17561759
}
17571760

17581761
func TestGetCertificate(t *testing.T) {

0 commit comments

Comments
 (0)