Skip to content

Commit bc6074f

Browse files
committed
Change api of functions Authority.Sign, Authority.Renew
Returns certificate chain instead of 2 members. Implements smallstep#126
1 parent e2858e1 commit bc6074f

File tree

10 files changed

+109
-61
lines changed

10 files changed

+109
-61
lines changed

acme/common.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212

1313
// SignAuthority is the interface implemented by a CA authority.
1414
type SignAuthority interface {
15-
Sign(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) (*x509.Certificate, *x509.Certificate, error)
15+
Sign(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) ([]*x509.Certificate, error)
1616
LoadProvisionerByID(string) (provisioner.Interface, error)
1717
}
1818

acme/order.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ func (o *order) finalize(db nosql.DB, csr *x509.CertificateRequest, auth SignAut
274274
}
275275

276276
// Create and store a new certificate.
277-
leaf, inter, err := auth.Sign(csr, provisioner.Options{
277+
certChain, err := auth.Sign(csr, provisioner.Options{
278278
NotBefore: provisioner.NewTimeDuration(o.NotBefore),
279279
NotAfter: provisioner.NewTimeDuration(o.NotAfter),
280280
}, signOps...)
@@ -285,8 +285,8 @@ func (o *order) finalize(db nosql.DB, csr *x509.CertificateRequest, auth SignAut
285285
cert, err := newCert(db, CertOptions{
286286
AccountID: o.AccountID,
287287
OrderID: o.ID,
288-
Leaf: leaf,
289-
Intermediates: []*x509.Certificate{inter},
288+
Leaf: certChain[0],
289+
Intermediates: certChain[1:],
290290
})
291291
if err != nil {
292292
return nil, err

acme/order_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -789,19 +789,19 @@ func TestOrderUpdateStatus(t *testing.T) {
789789
}
790790

791791
type mockSignAuth struct {
792-
sign func(csr *x509.CertificateRequest, signOpts provisioner.Options, extraOpts ...provisioner.SignOption) (*x509.Certificate, *x509.Certificate, error)
792+
sign func(csr *x509.CertificateRequest, signOpts provisioner.Options, extraOpts ...provisioner.SignOption) ([]*x509.Certificate, error)
793793
loadProvisionerByID func(string) (provisioner.Interface, error)
794794
ret1, ret2 interface{}
795795
err error
796796
}
797797

798-
func (m *mockSignAuth) Sign(csr *x509.CertificateRequest, signOpts provisioner.Options, extraOpts ...provisioner.SignOption) (*x509.Certificate, *x509.Certificate, error) {
798+
func (m *mockSignAuth) Sign(csr *x509.CertificateRequest, signOpts provisioner.Options, extraOpts ...provisioner.SignOption) ([]*x509.Certificate, error) {
799799
if m.sign != nil {
800800
return m.sign(csr, signOpts, extraOpts...)
801801
} else if m.err != nil {
802-
return nil, nil, m.err
802+
return nil, m.err
803803
}
804-
return m.ret1.(*x509.Certificate), m.ret2.(*x509.Certificate), m.err
804+
return []*x509.Certificate{m.ret1.(*x509.Certificate), m.ret2.(*x509.Certificate)}, m.err
805805
}
806806

807807
func (m *mockSignAuth) LoadProvisionerByID(id string) (provisioner.Interface, error) {
@@ -1082,9 +1082,9 @@ func TestOrderFinalize(t *testing.T) {
10821082
res: clone,
10831083
csr: csr,
10841084
sa: &mockSignAuth{
1085-
sign: func(csr *x509.CertificateRequest, pops provisioner.Options, signOps ...provisioner.SignOption) (*x509.Certificate, *x509.Certificate, error) {
1085+
sign: func(csr *x509.CertificateRequest, pops provisioner.Options, signOps ...provisioner.SignOption) ([]*x509.Certificate, error) {
10861086
assert.Equals(t, len(signOps), 4)
1087-
return crt, inter, nil
1087+
return []*x509.Certificate{crt, inter}, nil
10881088
},
10891089
},
10901090
db: &db.MockNoSQLDB{

api/api.go

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ type Authority interface {
3333
AuthorizeSign(ott string) ([]provisioner.SignOption, error)
3434
GetTLSOptions() *tlsutil.TLSOptions
3535
Root(shasum string) (*x509.Certificate, error)
36-
Sign(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) (*x509.Certificate, *x509.Certificate, error)
37-
Renew(peer *x509.Certificate) (*x509.Certificate, *x509.Certificate, error)
36+
Sign(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) ([]*x509.Certificate, error)
37+
Renew(peer *x509.Certificate) ([]*x509.Certificate, error)
3838
LoadProvisionerByCertificate(*x509.Certificate) (provisioner.Interface, error)
3939
LoadProvisionerByID(string) (provisioner.Interface, error)
4040
GetProvisioners(cursor string, limit int) (provisioner.List, string, error)
@@ -211,10 +211,11 @@ func (s *SignRequest) Validate() error {
211211

212212
// SignResponse is the response object of the certificate signature request.
213213
type SignResponse struct {
214-
ServerPEM Certificate `json:"crt"`
215-
CaPEM Certificate `json:"ca"`
216-
TLSOptions *tlsutil.TLSOptions `json:"tlsOptions,omitempty"`
217-
TLS *tls.ConnectionState `json:"-"`
214+
ServerPEM Certificate `json:"crt"`
215+
CaPEM Certificate `json:"ca"`
216+
CertChainPEM []Certificate `json:"certChain"`
217+
TLSOptions *tlsutil.TLSOptions `json:"tlsOptions,omitempty"`
218+
TLS *tls.ConnectionState `json:"-"`
218219
}
219220

220221
// RootsResponse is the response object of the roots request.
@@ -275,6 +276,14 @@ func (h *caHandler) Root(w http.ResponseWriter, r *http.Request) {
275276
JSON(w, &RootResponse{RootPEM: Certificate{cert}})
276277
}
277278

279+
func certChainToPEM(certChain []*x509.Certificate) []Certificate {
280+
certChainPEM := make([]Certificate, 0, len(certChain))
281+
for _, c := range certChain {
282+
certChainPEM = append(certChainPEM, Certificate{c})
283+
}
284+
return certChainPEM
285+
}
286+
278287
// Sign is an HTTP handler that reads a certificate request and an
279288
// one-time-token (ott) from the body and creates a new certificate with the
280289
// information in the certificate request.
@@ -302,17 +311,22 @@ func (h *caHandler) Sign(w http.ResponseWriter, r *http.Request) {
302311
return
303312
}
304313

305-
cert, root, err := h.Authority.Sign(body.CsrPEM.CertificateRequest, opts, signOpts...)
314+
certChain, err := h.Authority.Sign(body.CsrPEM.CertificateRequest, opts, signOpts...)
306315
if err != nil {
307316
WriteError(w, Forbidden(err))
308317
return
309318
}
310-
311-
logCertificate(w, cert)
319+
certChainPEM := certChainToPEM(certChain)
320+
var caPEM Certificate
321+
if len(certChainPEM) > 0 {
322+
caPEM = certChainPEM[1]
323+
}
324+
logCertificate(w, certChain[0])
312325
JSONStatus(w, &SignResponse{
313-
ServerPEM: Certificate{cert},
314-
CaPEM: Certificate{root},
315-
TLSOptions: h.Authority.GetTLSOptions(),
326+
ServerPEM: certChainPEM[0],
327+
CaPEM: caPEM,
328+
CertChainPEM: certChainPEM,
329+
TLSOptions: h.Authority.GetTLSOptions(),
316330
}, http.StatusCreated)
317331
}
318332

@@ -324,17 +338,23 @@ func (h *caHandler) Renew(w http.ResponseWriter, r *http.Request) {
324338
return
325339
}
326340

327-
cert, root, err := h.Authority.Renew(r.TLS.PeerCertificates[0])
341+
certChain, err := h.Authority.Renew(r.TLS.PeerCertificates[0])
328342
if err != nil {
329343
WriteError(w, Forbidden(err))
330344
return
331345
}
346+
certChainPEM := certChainToPEM(certChain)
347+
var caPEM Certificate
348+
if len(certChainPEM) > 0 {
349+
caPEM = certChainPEM[1]
350+
}
332351

333-
logCertificate(w, cert)
352+
logCertificate(w, certChain[0])
334353
JSONStatus(w, &SignResponse{
335-
ServerPEM: Certificate{cert},
336-
CaPEM: Certificate{root},
337-
TLSOptions: h.Authority.GetTLSOptions(),
354+
ServerPEM: certChainPEM[0],
355+
CaPEM: caPEM,
356+
CertChainPEM: certChainPEM,
357+
TLSOptions: h.Authority.GetTLSOptions(),
338358
}, http.StatusCreated)
339359
}
340360

api/api_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -501,10 +501,10 @@ type mockAuthority struct {
501501
authorizeSign func(ott string) ([]provisioner.SignOption, error)
502502
getTLSOptions func() *tlsutil.TLSOptions
503503
root func(shasum string) (*x509.Certificate, error)
504-
sign func(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) (*x509.Certificate, *x509.Certificate, error)
504+
sign func(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) ([]*x509.Certificate, error)
505505
signSSH func(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error)
506506
signSSHAddUser func(key ssh.PublicKey, cert *ssh.Certificate) (*ssh.Certificate, error)
507-
renew func(cert *x509.Certificate) (*x509.Certificate, *x509.Certificate, error)
507+
renew func(cert *x509.Certificate) ([]*x509.Certificate, error)
508508
loadProvisionerByCertificate func(cert *x509.Certificate) (provisioner.Interface, error)
509509
loadProvisionerByID func(provID string) (provisioner.Interface, error)
510510
getProvisioners func(nextCursor string, limit int) (provisioner.List, string, error)
@@ -540,11 +540,11 @@ func (m *mockAuthority) Root(shasum string) (*x509.Certificate, error) {
540540
return m.ret1.(*x509.Certificate), m.err
541541
}
542542

543-
func (m *mockAuthority) Sign(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) (*x509.Certificate, *x509.Certificate, error) {
543+
func (m *mockAuthority) Sign(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) ([]*x509.Certificate, error) {
544544
if m.sign != nil {
545545
return m.sign(cr, opts, signOpts...)
546546
}
547-
return m.ret1.(*x509.Certificate), m.ret2.(*x509.Certificate), m.err
547+
return []*x509.Certificate{m.ret1.(*x509.Certificate), m.ret2.(*x509.Certificate)}, m.err
548548
}
549549

550550
func (m *mockAuthority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error) {
@@ -561,11 +561,11 @@ func (m *mockAuthority) SignSSHAddUser(key ssh.PublicKey, cert *ssh.Certificate)
561561
return m.ret1.(*ssh.Certificate), m.err
562562
}
563563

564-
func (m *mockAuthority) Renew(cert *x509.Certificate) (*x509.Certificate, *x509.Certificate, error) {
564+
func (m *mockAuthority) Renew(cert *x509.Certificate) ([]*x509.Certificate, error) {
565565
if m.renew != nil {
566566
return m.renew(cert)
567567
}
568-
return m.ret1.(*x509.Certificate), m.ret2.(*x509.Certificate), m.err
568+
return []*x509.Certificate{m.ret1.(*x509.Certificate), m.ret2.(*x509.Certificate)}, m.err
569569
}
570570

571571
func (m *mockAuthority) GetProvisioners(nextCursor string, limit int) (provisioner.List, string, error) {
@@ -724,8 +724,8 @@ func Test_caHandler_Sign(t *testing.T) {
724724
t.Fatal(err)
725725
}
726726

727-
expected1 := []byte(`{"crt":"` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n","ca":"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"}`)
728-
expected2 := []byte(`{"crt":"` + strings.Replace(stepCertPEM, "\n", `\n`, -1) + `\n","ca":"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"}`)
727+
expected1 := []byte(`{"crt":"` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n","ca":"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n","certChain":["` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n","` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"]}`)
728+
expected2 := []byte(`{"crt":"` + strings.Replace(stepCertPEM, "\n", `\n`, -1) + `\n","ca":"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n","certChain":["` + strings.Replace(stepCertPEM, "\n", `\n`, -1) + `\n","` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"]}`)
729729

730730
tests := []struct {
731731
name string
@@ -798,7 +798,7 @@ func Test_caHandler_Renew(t *testing.T) {
798798
{"renew error", cs, nil, nil, fmt.Errorf("an error"), http.StatusForbidden},
799799
}
800800

801-
expected := []byte(`{"crt":"` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n","ca":"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"}`)
801+
expected := []byte(`{"crt":"` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n","ca":"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n","certChain":["` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n","` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"]}`)
802802

803803
for _, tt := range tests {
804804
t.Run(tt.name, func(t *testing.T) {

authority/tls.go

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func withDefaultASN1DN(def *x509util.ASN1DN) x509util.WithOption {
5656
}
5757

5858
// Sign creates a signed certificate from a certificate signing request.
59-
func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Options, extraOpts ...provisioner.SignOption) (*x509.Certificate, *x509.Certificate, error) {
59+
func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Options, extraOpts ...provisioner.SignOption) ([]*x509.Certificate, error) {
6060
var (
6161
errContext = apiCtx{"csr": csr, "signOptions": signOpts}
6262
mods = []x509util.WithOption{withDefaultASN1DN(a.config.AuthorityConfig.Template)}
@@ -69,66 +69,66 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Opti
6969
certValidators = append(certValidators, k)
7070
case provisioner.CertificateRequestValidator:
7171
if err := k.Valid(csr); err != nil {
72-
return nil, nil, &apiError{errors.Wrap(err, "sign"), http.StatusUnauthorized, errContext}
72+
return nil, &apiError{errors.Wrap(err, "sign"), http.StatusUnauthorized, errContext}
7373
}
7474
case provisioner.ProfileModifier:
7575
mods = append(mods, k.Option(signOpts))
7676
default:
77-
return nil, nil, &apiError{errors.Errorf("sign: invalid extra option type %T", k),
77+
return nil, &apiError{errors.Errorf("sign: invalid extra option type %T", k),
7878
http.StatusInternalServerError, errContext}
7979
}
8080
}
8181

8282
if err := csr.CheckSignature(); err != nil {
83-
return nil, nil, &apiError{errors.Wrap(err, "sign: invalid certificate request"),
83+
return nil, &apiError{errors.Wrap(err, "sign: invalid certificate request"),
8484
http.StatusBadRequest, errContext}
8585
}
8686

8787
leaf, err := x509util.NewLeafProfileWithCSR(csr, issIdentity.Crt, issIdentity.Key, mods...)
8888
if err != nil {
89-
return nil, nil, &apiError{errors.Wrapf(err, "sign"), http.StatusInternalServerError, errContext}
89+
return nil, &apiError{errors.Wrapf(err, "sign"), http.StatusInternalServerError, errContext}
9090
}
9191

9292
for _, v := range certValidators {
9393
if err := v.Valid(leaf.Subject()); err != nil {
94-
return nil, nil, &apiError{errors.Wrap(err, "sign"), http.StatusUnauthorized, errContext}
94+
return nil, &apiError{errors.Wrap(err, "sign"), http.StatusUnauthorized, errContext}
9595
}
9696
}
9797

9898
crtBytes, err := leaf.CreateCertificate()
9999
if err != nil {
100-
return nil, nil, &apiError{errors.Wrap(err, "sign: error creating new leaf certificate"),
100+
return nil, &apiError{errors.Wrap(err, "sign: error creating new leaf certificate"),
101101
http.StatusInternalServerError, errContext}
102102
}
103103

104104
serverCert, err := x509.ParseCertificate(crtBytes)
105105
if err != nil {
106-
return nil, nil, &apiError{errors.Wrap(err, "sign: error parsing new leaf certificate"),
106+
return nil, &apiError{errors.Wrap(err, "sign: error parsing new leaf certificate"),
107107
http.StatusInternalServerError, errContext}
108108
}
109109

110110
caCert, err := x509.ParseCertificate(issIdentity.Crt.Raw)
111111
if err != nil {
112-
return nil, nil, &apiError{errors.Wrap(err, "sign: error parsing intermediate certificate"),
112+
return nil, &apiError{errors.Wrap(err, "sign: error parsing intermediate certificate"),
113113
http.StatusInternalServerError, errContext}
114114
}
115115

116116
if err = a.db.StoreCertificate(serverCert); err != nil {
117117
if err != db.ErrNotImplemented {
118-
return nil, nil, &apiError{errors.Wrap(err, "sign: error storing certificate in db"),
118+
return nil, &apiError{errors.Wrap(err, "sign: error storing certificate in db"),
119119
http.StatusInternalServerError, errContext}
120120
}
121121
}
122122

123-
return serverCert, caCert, nil
123+
return []*x509.Certificate{serverCert, caCert}, nil
124124
}
125125

126126
// Renew creates a new Certificate identical to the old certificate, except
127127
// with a validity window that begins 'now'.
128-
func (a *Authority) Renew(oldCert *x509.Certificate) (*x509.Certificate, *x509.Certificate, error) {
128+
func (a *Authority) Renew(oldCert *x509.Certificate) ([]*x509.Certificate, error) {
129129
// Check step provisioner extensions
130130
if err := a.authorizeRenewal(oldCert); err != nil {
131-
return nil, nil, err
131+
return nil, err
132132
}
133133

134134
// Issuer
@@ -181,26 +181,26 @@ func (a *Authority) Renew(oldCert *x509.Certificate) (*x509.Certificate, *x509.C
181181
leaf, err := x509util.NewLeafProfileWithTemplate(newCert,
182182
issIdentity.Crt, issIdentity.Key)
183183
if err != nil {
184-
return nil, nil, &apiError{err, http.StatusInternalServerError, apiCtx{}}
184+
return nil, &apiError{err, http.StatusInternalServerError, apiCtx{}}
185185
}
186186
crtBytes, err := leaf.CreateCertificate()
187187
if err != nil {
188-
return nil, nil, &apiError{errors.Wrap(err, "error renewing certificate from existing server certificate"),
188+
return nil, &apiError{errors.Wrap(err, "error renewing certificate from existing server certificate"),
189189
http.StatusInternalServerError, apiCtx{}}
190190
}
191191

192192
serverCert, err := x509.ParseCertificate(crtBytes)
193193
if err != nil {
194-
return nil, nil, &apiError{errors.Wrap(err, "error parsing new server certificate"),
194+
return nil, &apiError{errors.Wrap(err, "error parsing new server certificate"),
195195
http.StatusInternalServerError, apiCtx{}}
196196
}
197197
caCert, err := x509.ParseCertificate(issIdentity.Crt.Raw)
198198
if err != nil {
199-
return nil, nil, &apiError{errors.Wrap(err, "error parsing intermediate certificate"),
199+
return nil, &apiError{errors.Wrap(err, "error parsing intermediate certificate"),
200200
http.StatusInternalServerError, apiCtx{}}
201201
}
202202

203-
return serverCert, caCert, nil
203+
return []*x509.Certificate{serverCert, caCert}, nil
204204
}
205205

206206
// RevokeOptions are the options for the Revoke API.

authority/tls_test.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ ttnEF4Rq8zqzr4fbv+AF451Mx36AkfgZr9XWGzxidrH+fBCNWXWNR+ymhrL6UFTG
276276
t.Run(name, func(t *testing.T) {
277277
tc := genTestCase(t)
278278

279-
leaf, intermediate, err := tc.auth.Sign(tc.csr, tc.signOpts, tc.extraOpts...)
279+
certChain, err := tc.auth.Sign(tc.csr, tc.signOpts, tc.extraOpts...)
280280
if err != nil {
281281
if assert.NotNil(t, tc.err) {
282282
switch v := err.(type) {
@@ -289,6 +289,8 @@ ttnEF4Rq8zqzr4fbv+AF451Mx36AkfgZr9XWGzxidrH+fBCNWXWNR+ymhrL6UFTG
289289
}
290290
}
291291
} else {
292+
leaf := certChain[0]
293+
intermediate := certChain[1]
292294
if assert.Nil(t, tc.err) {
293295
assert.Equals(t, leaf.NotBefore, signOpts.NotBefore.Time().Truncate(time.Second))
294296
assert.Equals(t, leaf.NotAfter, signOpts.NotAfter.Time().Truncate(time.Second))
@@ -453,11 +455,11 @@ func TestRenew(t *testing.T) {
453455
tc, err := genTestCase()
454456
assert.FatalError(t, err)
455457

456-
var leaf, intermediate *x509.Certificate
458+
var certChain []*x509.Certificate
457459
if tc.auth != nil {
458-
leaf, intermediate, err = tc.auth.Renew(tc.crt)
460+
certChain, err = tc.auth.Renew(tc.crt)
459461
} else {
460-
leaf, intermediate, err = a.Renew(tc.crt)
462+
certChain, err = a.Renew(tc.crt)
461463
}
462464
if err != nil {
463465
if assert.NotNil(t, tc.err) {
@@ -471,6 +473,8 @@ func TestRenew(t *testing.T) {
471473
}
472474
}
473475
} else {
476+
leaf := certChain[0]
477+
intermediate := certChain[1]
474478
if assert.Nil(t, tc.err) {
475479
assert.Equals(t, leaf.NotAfter.Sub(leaf.NotBefore), tc.crt.NotAfter.Sub(crt.NotBefore))
476480

0 commit comments

Comments
 (0)