Skip to content

Commit

Permalink
Merge pull request #1163 from akgmartin/master
Browse files Browse the repository at this point in the history
  • Loading branch information
nickysemenza committed Feb 1, 2021
2 parents 57882e0 + 5628e97 commit 9f7129a
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 15 deletions.
15 changes: 15 additions & 0 deletions cli/gencert/gencert_test.go
Expand Up @@ -3,6 +3,7 @@ package gencert
import (
"io/ioutil"
"os"
"strings"
"testing"

"github.com/cloudflare/cfssl/cli"
Expand Down Expand Up @@ -215,3 +216,17 @@ func TestBadGencertMain(t *testing.T) {
}

}

func TestOidMain(t *testing.T) {
c := cli.Config{
CAFile: "../testdata/ca.pem",
CAKeyFile: "../testdata/ca-key.pem",
}
err := gencertMain([]string{"../testdata/bad_oid_csr.json"}, c)
if err == nil {
t.Fatal("Expected error")
}
if !strings.Contains(err.Error(), "invalid OID part abc") {
t.Fatalf("Unexpected error: %s", err.Error())
}
}
22 changes: 22 additions & 0 deletions cli/testdata/bad_oid_csr.json
@@ -0,0 +1,22 @@
{
"hosts": [
"cloudflare.com",
"www.cloudflare.com"
],
"key": {
"algo": "rsa",
"size": 2048
},
"names": [
{
"C": "US",
"L": "San Francisco",
"O": "CloudFlare",
"OU": "Systems Engineering",
"ST": "California",
"OID": {
"abc": "abc"
}
}
]
}
7 changes: 5 additions & 2 deletions cli/testdata/csr.json
Expand Up @@ -13,7 +13,10 @@
"L": "San Francisco",
"O": "CloudFlare",
"OU": "Systems Engineering",
"ST": "California"
"ST": "California",
"OID": {
"1.2.3.4.5": "abc"
}
}
]
}
}
50 changes: 41 additions & 9 deletions csr/csr.go
Expand Up @@ -12,9 +12,11 @@ import (
"encoding/asn1"
"encoding/pem"
"errors"
"fmt"
"net"
"net/mail"
"net/url"
"strconv"
"strings"

cferr "github.com/cloudflare/cfssl/errors"
Expand All @@ -30,12 +32,13 @@ const (

// A Name contains the SubjectInfo fields.
type Name struct {
C string `json:"C,omitempty" yaml:"C,omitempty"` // Country
ST string `json:"ST,omitempty" yaml:"ST,omitempty"` // State
L string `json:"L,omitempty" yaml:"L,omitempty"` // Locality
O string `json:"O,omitempty" yaml:"O,omitempty"` // OrganisationName
OU string `json:"OU,omitempty" yaml:"OU,omitempty"` // OrganisationalUnitName
SerialNumber string `json:"SerialNumber,omitempty" yaml:"SerialNumber,omitempty"`
C string `json:"C,omitempty" yaml:"C,omitempty"` // Country
ST string `json:"ST,omitempty" yaml:"ST,omitempty"` // State
L string `json:"L,omitempty" yaml:"L,omitempty"` // Locality
O string `json:"O,omitempty" yaml:"O,omitempty"` // OrganisationName
OU string `json:"OU,omitempty" yaml:"OU,omitempty"` // OrganisationalUnitName
SerialNumber string `json:"SerialNumber,omitempty" yaml:"SerialNumber,omitempty"`
OID map[string]string `json:"OID,omitempty", yaml:"OID,omitempty"`
}

// A KeyRequest contains the algorithm and key size for a new private key.
Expand Down Expand Up @@ -157,8 +160,25 @@ func appendIf(s string, a *[]string) {
}
}

// OIDFromString creates an ASN1 ObjectIdentifier from its string representation
func OIDFromString(s string) (asn1.ObjectIdentifier, error) {
var oid []int
parts := strings.Split(s, ".")
if len(parts) < 1 {
return oid, fmt.Errorf("invalid OID string: %s", s)
}
for _, p := range parts {
i, err := strconv.Atoi(p)
if err != nil {
return nil, fmt.Errorf("invalid OID part %s", p)
}
oid = append(oid, i)
}
return oid, nil
}

// Name returns the PKIX name for the request.
func (cr *CertificateRequest) Name() pkix.Name {
func (cr *CertificateRequest) Name() (pkix.Name, error) {
var name pkix.Name
name.CommonName = cr.CN

Expand All @@ -168,9 +188,16 @@ func (cr *CertificateRequest) Name() pkix.Name {
appendIf(n.L, &name.Locality)
appendIf(n.O, &name.Organization)
appendIf(n.OU, &name.OrganizationalUnit)
for k, v := range n.OID {
oid, err := OIDFromString(k)
if err != nil {
return name, err
}
name.ExtraNames = append(name.ExtraNames, pkix.AttributeTypeAndValue{Type: oid, Value: v})
}
}
name.SerialNumber = cr.SerialNumber
return name
return name, nil
}

// BasicConstraints CSR information RFC 5280, 4.2.1.9
Expand Down Expand Up @@ -367,8 +394,13 @@ func Generate(priv crypto.Signer, req *CertificateRequest) (csr []byte, err erro
return nil, cferr.New(cferr.PrivateKeyError, cferr.Unavailable)
}

subj, err := req.Name()
if err != nil {
return nil, err
}

var tpl = x509.CertificateRequest{
Subject: req.Name(),
Subject: subj,
SignatureAlgorithm: sigAlgo,
}

Expand Down
9 changes: 6 additions & 3 deletions csr/csr_test.go
Expand Up @@ -72,7 +72,10 @@ func TestPKIXName(t *testing.T) {
KeyRequest: NewKeyRequest(),
}

name := cr.Name()
name, err := cr.Name()
if err != nil {
t.Fatalf("Error getting name: %s", err.Error())
}
if len(name.Country) != 2 {
t.Fatal("Expected two countries in SubjInfo.")
} else if len(name.Province) != 2 {
Expand Down Expand Up @@ -113,7 +116,7 @@ func TestParseRequest(t *testing.T) {
KeyRequest: NewKeyRequest(),
Extensions: []pkix.Extension{
pkix.Extension{
Id: asn1.ObjectIdentifier{1, 2, 3, 4, 5},
Id: asn1.ObjectIdentifier{1, 2, 3, 4, 5},
Value: []byte("AgEB"),
},
},
Expand All @@ -123,7 +126,7 @@ func TestParseRequest(t *testing.T) {
if err != nil {
t.Fatalf("%v", err)
}

block, _ := pem.Decode(csrBytes)
if block == nil {
t.Fatalf("%v", err)
Expand Down
25 changes: 24 additions & 1 deletion signer/signer.go
Expand Up @@ -172,6 +172,10 @@ func DefaultSigAlgo(priv crypto.Signer) x509.SignatureAlgorithm {
}
}

func isCommonAttr(t []int) bool {
return (len(t) == 4 && t[0] == 2 && t[1] == 5 && t[2] == 4 && (t[3] == 3 || (t[3] >= 5 && t[3] <= 11) || t[3] == 17))
}

// ParseCertificateRequest takes an incoming certificate request and
// builds a certificate template from it.
func ParseCertificateRequest(s Signer, p *config.SigningProfile, csrBytes []byte) (template *x509.Certificate, err error) {
Expand All @@ -181,14 +185,33 @@ func ParseCertificateRequest(s Signer, p *config.SigningProfile, csrBytes []byte
return
}

var r pkix.RDNSequence
_, err = asn1.Unmarshal(csrv.RawSubject, &r)

if err != nil {
err = cferr.Wrap(cferr.CSRError, cferr.ParseFailed, err)
return
}

var subject pkix.Name
subject.FillFromRDNSequence(&r)

for _, v := range r {
for _, vv := range v {
if !isCommonAttr(vv.Type) {
subject.ExtraNames = append(subject.ExtraNames, vv)
}
}
}

err = csrv.CheckSignature()
if err != nil {
err = cferr.Wrap(cferr.CSRError, cferr.KeyMismatch, err)
return
}

template = &x509.Certificate{
Subject: csrv.Subject,
Subject: subject,
PublicKeyAlgorithm: csrv.PublicKeyAlgorithm,
PublicKey: csrv.PublicKey,
SignatureAlgorithm: s.SigAlgo(),
Expand Down

0 comments on commit 9f7129a

Please sign in to comment.