Skip to content

Commit 6b49bea

Browse files
DarkEdgesnirving
andauthored
add copy_extensions configuration to local signer to allow (#1082)
* Added ability to copy Extensions from CSR * Added Profile to determine if the Signer should copy the extensions provided in the CSR across. * Added config test Co-authored-by: Nicholas Irving <nirving@darkedges.com>
1 parent f30ae6a commit 6b49bea

7 files changed

Lines changed: 110 additions & 9 deletions

File tree

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,5 @@ profile.out
55
bin
66
*.deb
77
*.rpm
8+
test
9+

config/config.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ type SigningProfile struct {
8484
ExpiryString string `json:"expiry"`
8585
BackdateString string `json:"backdate"`
8686
AuthKeyName string `json:"auth_key"`
87+
CopyExtensions bool `json:"copy_extensions"`
8788
PrevAuthKeyName string `json:"prev_auth_key"` // to suppport key rotation
8889
RemoteName string `json:"remote"`
8990
NotBefore time.Time `json:"not_before"`

config/config_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,25 @@ var validLocalConfigsWithCAConstraint = []string{
257257
}`,
258258
}
259259

260+
var copyExtensionWantedlLocalConfig = `
261+
{
262+
"signing": {
263+
"default": {
264+
"expiry": "8000h",
265+
"copy_extensions": true
266+
}
267+
}
268+
}`
269+
270+
var copyExtensionNotWantedlLocalConfig = `
271+
{
272+
"signing": {
273+
"default": {
274+
"expiry": "8000h"
275+
}
276+
}
277+
}`
278+
260279
func TestInvalidProfile(t *testing.T) {
261280
if invalidProfileConfig.Signing.Profiles["invalid"].validProfile(false) {
262281
t.Fatal("invalid profile accepted as valid")
@@ -580,3 +599,25 @@ func TestValidCAConstraint(t *testing.T) {
580599
}
581600
}
582601
}
602+
603+
func TestWantCopyExtension(t *testing.T) {
604+
localConfig, err := LoadConfig([]byte(copyExtensionWantedlLocalConfig))
605+
if localConfig.Signing.Default.CopyExtensions != true {
606+
t.Fatal("incorrect TestWantCopyExtension().")
607+
}
608+
609+
if err != nil {
610+
t.Fatal(err)
611+
}
612+
}
613+
614+
func TestDontWantCopyExtension(t *testing.T) {
615+
localConfig, err := LoadConfig([]byte(copyExtensionNotWantedlLocalConfig))
616+
if localConfig.Signing.Default.CopyExtensions != false {
617+
t.Fatal("incorrect TestDontWantCopyExtension().")
618+
}
619+
620+
if err != nil {
621+
t.Fatal(err)
622+
}
623+
}

csr/csr.go

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ type CertificateRequest struct {
138138
KeyRequest *KeyRequest `json:"key,omitempty" yaml:"key,omitempty"`
139139
CA *CAConfig `json:"ca,omitempty" yaml:"ca,omitempty"`
140140
SerialNumber string `json:"serialnumber,omitempty" yaml:"serialnumber,omitempty"`
141+
Extensions []pkix.Extension `json:"extensions,omitempty" yaml:"extensions,omitempty"`
141142
}
142143

143144
// New returns a new, empty CertificateRequest with a
@@ -382,6 +383,8 @@ func Generate(priv crypto.Signer, req *CertificateRequest) (csr []byte, err erro
382383
}
383384
}
384385

386+
tpl.ExtraExtensions = []pkix.Extension{}
387+
385388
if req.CA != nil {
386389
err = appendCAInfoToCSR(req.CA, &tpl)
387390
if err != nil {
@@ -390,6 +393,14 @@ func Generate(priv crypto.Signer, req *CertificateRequest) (csr []byte, err erro
390393
}
391394
}
392395

396+
if req.Extensions != nil {
397+
err = appendExtensionsToCSR(req.Extensions, &tpl)
398+
if err != nil {
399+
err = cferr.Wrap(cferr.CSRError, cferr.GenerationFailed, err)
400+
return
401+
}
402+
}
403+
393404
csr, err = x509.CreateCertificateRequest(rand.Reader, &tpl, priv)
394405
if err != nil {
395406
log.Errorf("failed to generate a CSR: %v", err)
@@ -418,13 +429,19 @@ func appendCAInfoToCSR(reqConf *CAConfig, csr *x509.CertificateRequest) error {
418429
return err
419430
}
420431

421-
csr.ExtraExtensions = []pkix.Extension{
422-
{
423-
Id: asn1.ObjectIdentifier{2, 5, 29, 19},
432+
csr.ExtraExtensions = append(csr.ExtraExtensions, pkix.Extension{
433+
Id: asn1.ObjectIdentifier{2, 5, 29, 19},
424434
Value: val,
425-
Critical: true,
426-
},
427-
}
435+
Critical: true,
436+
})
428437

438+
return nil
439+
}
440+
441+
// appendCAInfoToCSR appends user-defined extension to a CSR
442+
func appendExtensionsToCSR(extensions []pkix.Extension, csr *x509.CertificateRequest) error {
443+
for _, extension := range extensions {
444+
csr.ExtraExtensions = append(csr.ExtraExtensions, extension)
445+
}
429446
return nil
430447
}

csr/csr_test.go

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"crypto/elliptic"
77
"crypto/rsa"
88
"crypto/x509"
9+
"crypto/x509/pkix"
910
"encoding/asn1"
1011
"encoding/pem"
1112
"io/ioutil"
@@ -110,12 +111,44 @@ func TestParseRequest(t *testing.T) {
110111
},
111112
Hosts: []string{"cloudflare.com", "www.cloudflare.com", "192.168.0.1", "jdoe@example.com", "https://www.cloudflare.com"},
112113
KeyRequest: NewKeyRequest(),
114+
Extensions: []pkix.Extension{
115+
pkix.Extension{
116+
Id: asn1.ObjectIdentifier{1, 2, 3, 4, 5},
117+
Value: []byte("AgEB"),
118+
},
119+
},
120+
}
121+
122+
csrBytes, _, err := ParseRequest(cr)
123+
if err != nil {
124+
t.Fatalf("%v", err)
125+
}
126+
127+
block, _ := pem.Decode(csrBytes)
128+
if block == nil {
129+
t.Fatalf("%v", err)
130+
}
131+
132+
if block.Type != "CERTIFICATE REQUEST" {
133+
t.Fatalf("Incorrect block type: %s", block.Type)
113134
}
114135

115-
_, _, err := ParseRequest(cr)
136+
csr, err := x509.ParseCertificateRequest(block.Bytes)
116137
if err != nil {
117138
t.Fatalf("%v", err)
118139
}
140+
141+
found := false
142+
for _, ext := range csr.Extensions {
143+
if ext.Id.Equal(asn1.ObjectIdentifier{1, 2, 3, 4, 5}) {
144+
found = true
145+
break
146+
}
147+
}
148+
149+
if !found {
150+
t.Fatalf("CSR did not include Custom Extension")
151+
}
119152
}
120153

121154
// TestParseRequestCA ensures that a valid CA certificate request does not

signer/local/local.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ func (s *Signer) Sign(req signer.SignRequest) (cert []byte, err error) {
298298
cferr.BadRequest, errors.New("not a csr"))
299299
}
300300

301-
csrTemplate, err := signer.ParseCertificateRequest(s, block.Bytes)
301+
csrTemplate, err := signer.ParseCertificateRequest(s, profile, block.Bytes)
302302
if err != nil {
303303
return nil, err
304304
}

signer/signer.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ func DefaultSigAlgo(priv crypto.Signer) x509.SignatureAlgorithm {
171171

172172
// ParseCertificateRequest takes an incoming certificate request and
173173
// builds a certificate template from it.
174-
func ParseCertificateRequest(s Signer, csrBytes []byte) (template *x509.Certificate, err error) {
174+
func ParseCertificateRequest(s Signer, p *config.SigningProfile, csrBytes []byte) (template *x509.Certificate, err error) {
175175
csrv, err := x509.ParseCertificateRequest(csrBytes)
176176
if err != nil {
177177
err = cferr.Wrap(cferr.CSRError, cferr.ParseFailed, err)
@@ -193,6 +193,8 @@ func ParseCertificateRequest(s Signer, csrBytes []byte) (template *x509.Certific
193193
IPAddresses: csrv.IPAddresses,
194194
EmailAddresses: csrv.EmailAddresses,
195195
URIs: csrv.URIs,
196+
Extensions: csrv.Extensions,
197+
ExtraExtensions: []pkix.Extension{},
196198
}
197199

198200
for _, val := range csrv.Extensions {
@@ -212,6 +214,11 @@ func ParseCertificateRequest(s Signer, csrBytes []byte) (template *x509.Certific
212214
template.IsCA = constraints.IsCA
213215
template.MaxPathLen = constraints.MaxPathLen
214216
template.MaxPathLenZero = template.MaxPathLen == 0
217+
} else {
218+
// If the profile has 'copy_extensions' to true then lets add it
219+
if (p.CopyExtensions) {
220+
template.ExtraExtensions = append(template.ExtraExtensions, val)
221+
}
215222
}
216223
}
217224

0 commit comments

Comments
 (0)