Skip to content
Permalink
Browse files

Address review comments

  • Loading branch information
bifurcation committed Dec 16, 2015
1 parent 54e336e commit a439df40cacc599dafcda8511f7a1350e6cc8dc6
Showing with 19 additions and 36 deletions.
  1. +19 −11 ca/certificate-authority.go
  2. +0 −25 core/util.go
@@ -48,6 +48,9 @@ var badSignatureAlgorithms = map[x509.SignatureAlgorithm]bool{
x509.ECDSAWithSHA1: true,
}

// PKCS#9 attribute for requesting certificate extensions
var oidExtensionRequest = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 9, 14}

// OID and fixed value for the "must staple" variant of the TLS Feature
// extension:
//
@@ -318,18 +321,23 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(csr x509.CertificateRequest
// that only requires stapling.
//
// Other requested extensions are silently ignored.
//
// XXX(rlb): It might be good to add a generic mechanism for extensions, but
// on the other hand, we probably *don't* want to copy arbitrary client-
// provided data into extensions.
extensions := core.ExtensionsFromCSR(&csr)
requestedExtensions := []signer.Extension{}
if _, present := extensions[oidTLSFeature.String()]; present {
requestedExtensions = append(requestedExtensions, signer.Extension{
ID: cfsslConfig.OID(oidTLSFeature),
Critical: false,
Value: mustStapleFeatureValue,
})
for _, attr := range csr.Attributes {
if !attr.Type.Equal(oidExtensionRequest) {
continue
}

for _, extList := range attr.Value {
for _, ext := range extList {
if ext.Type.Equal(oidTLSFeature) {
requestedExtensions = append(requestedExtensions, signer.Extension{
ID: cfsslConfig.OID(oidTLSFeature),
Critical: false,
Value: mustStapleFeatureValue,
})
}
}
}
}

notAfter := ca.clk.Now().Add(ca.validityPeriod)
@@ -483,31 +483,6 @@ var (
oidSubjectAltName = asn1.ObjectIdentifier{2, 5, 29, 17}
)

This comment has been minimized.

Copy link
@osirisinferi

osirisinferi Dec 16, 2015

Forgive me if I'm saying something really stupid, as I'm not a Go programmer, but should these two variables be left here in core/util, as oidExtensionRequest is again declared in ca/certificate-authority in the latest commit?


// ExtensionsFromCSR extracts the set of requested extensions from a CSR
func ExtensionsFromCSR(csr *x509.CertificateRequest) map[string][]byte {
extensionMap := map[string][]byte{}
for _, attr := range csr.Attributes {
if !attr.Type.Equal(oidExtensionRequest) || len(attr.Value) != 1 {
continue
}

for _, ext := range attr.Value[0] {
// SubjectAltName is already handled by ParseCertificateRequest
if ext.Type.Equal(oidSubjectAltName) {
continue
}

extValue, ok := ext.Value.([]byte)
if !ok {
continue
}

extensionMap[ext.Type.String()] = extValue
}
}
return extensionMap
}

// SerialToString converts a certificate serial number (big.Int) to a String
// consistently.
func SerialToString(serial *big.Int) string {

0 comments on commit a439df4

Please sign in to comment.
You can’t perform that action at this time.