Skip to content

Commit

Permalink
Merge 3adf4a9 into 6b3a6d1
Browse files Browse the repository at this point in the history
  • Loading branch information
andygabby committed Mar 9, 2021
2 parents 6b3a6d1 + 3adf4a9 commit ad914f1
Show file tree
Hide file tree
Showing 11 changed files with 162 additions and 124 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/boulder-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ jobs:
matrix:
# Add additional docker image tags here and all tests will be run with the additional image.
BOULDER_TOOLS_TAG:
- go1.15.7_2021-02-25
- go1.15.7_2021-03-08
- go1.16_2021-03-08
# Tests command definitions. Use the entire docker-compose command you want to run.
tests:
# Run ./test.sh --help for a description of each of the flags
Expand Down
13 changes: 1 addition & 12 deletions cmd/ceremony/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ Note: Intermediate certificates always include the extended key usages id-kp-ser
| Field | Description |
| --- | --- |
| `csr-path` | Path to store PEM CSR for cross-signing, optional. |
- `certificate-profile`: object containing profile for certificate to generate. Fields are documented [below](#Certificate-profile-format). Cannot include the `signature-algorithm`, `not-before`, and `not-after` fields.
- `certificate-profile`: object containing profile for certificate to generate. Fields are documented [below](#Certificate-profile-format). Should only include Subject related fields `common-name`, `organization`, `country`.

Example:

Expand All @@ -170,17 +170,6 @@ certificate-profile:
common-name: CA root
organization: good guys
country: US
ocsp-url: http://good-guys.com/ocsp
crl-url: http://good-guys.com/crl
issuer-url: http://good-guys.com/root
policies:
- oid: 1.2.3
- oid: 4.5.6
cps-uri: "http://example.com/cps"
key-usages:
- Digital Signature
- Cert Sign
- CRL Sign
```

This config generates a CSR signed by a key in the HSM, identified by the object label `intermediate signing key`, and writes it to `/home/user/csr.pem`.
Expand Down
59 changes: 30 additions & 29 deletions cmd/ceremony/cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package main

import (
"crypto"
"crypto/rand"
"crypto/sha1"
"crypto/x509"
"crypto/x509/pkix"
Expand Down Expand Up @@ -87,6 +86,15 @@ const (
requestCert
)

// Subject returns a pkix.Name from the appropriate certProfile fields
func (profile *certProfile) Subject() pkix.Name {
return pkix.Name{
CommonName: profile.CommonName,
Organization: []string{profile.Organization},
Country: []string{profile.Country},
}
}

func (profile *certProfile) verifyProfile(ct certType) error {
if ct != requestCert {
if profile.NotBefore == "" {
Expand All @@ -108,6 +116,21 @@ func (profile *certProfile) verifyProfile(ct certType) error {
if profile.SignatureAlgorithm != "" {
return errors.New("signature-algorithm cannot be set for a CSR")
}
if profile.OCSPURL != "" {
return errors.New("ocsp-url cannot be set for a CSR")
}
if profile.CRLURL != "" {
return errors.New("crl-url cannot be set for a CSR")
}
if profile.IssuerURL != "" {
return errors.New("issuer-url cannot be set for a CSR")
}
if profile.Policies != nil {
return errors.New("policies cannot be set for a CSR")
}
if profile.KeyUsages != nil {
return errors.New("key-usages cannot be set for a CSR")
}
}
if profile.CommonName == "" {
return errors.New("common-name is required")
Expand All @@ -119,7 +142,7 @@ func (profile *certProfile) verifyProfile(ct certType) error {
return errors.New("country is required")
}

if ct == intermediateCert || ct == requestCert {
if ct == intermediateCert {
if profile.CRLURL == "" {
return errors.New("crl-url is required for intermediates")
}
Expand Down Expand Up @@ -243,15 +266,12 @@ func makeTemplate(randReader io.Reader, profile *certProfile, pubKey []byte, ct
return nil, errors.New("at least one key usage must be set")
}

templateSubject := profile.Subject()
cert := &x509.Certificate{
SerialNumber: big.NewInt(0).SetBytes(serial),
BasicConstraintsValid: true,
IsCA: true,
Subject: pkix.Name{
CommonName: profile.CommonName,
Organization: []string{profile.Organization},
Country: []string{profile.Country},
},
Subject: templateSubject,
OCSPServer: ocspServer,
CRLDistributionPoints: crlDistributionPoints,
IssuingCertificateURL: issuingCertificateURL,
Expand Down Expand Up @@ -320,32 +340,13 @@ func (fr *failReader) Read([]byte) (int, error) {
return 0, errors.New("Empty reader used by x509.CreateCertificate")
}

func generateCSR(profile *certProfile, randReader *hsmRandReader, pubBytes []byte, pub crypto.PublicKey, signer crypto.Signer) ([]byte, error) {
// currently Go doesn't support all of the convenience fields for x509.CertificateRequest
// that x509.Certificate has. Instead of doing all of the manual extension construction
// ourselves here we just create a throwaway self-signed certificate and then dump all
// of the generated extensions into a x509.CertificateRequest. In the future Go should
// support doing this properly itself, but for now this is the easiest approach.
template, err := makeTemplate(randReader, profile, pubBytes, requestCert)
if err != nil {
return nil, fmt.Errorf("failed to create certificate template: %s", err)
}
selfSignedDER, err := x509.CreateCertificate(rand.Reader, template, template, pub, &csrSelfSigner{pub})
if err != nil {
return nil, fmt.Errorf("failed to create certificate for CSR: %s", err)
}
selfSigned, err := x509.ParseCertificate(selfSignedDER)
if err != nil {
return nil, fmt.Errorf("failed to parse certificate template for CSR: %s", err)
}

func generateCSR(profile *certProfile, signer crypto.Signer) ([]byte, error) {
csrSubject := profile.Subject()
csrDER, err := x509.CreateCertificateRequest(&failReader{}, &x509.CertificateRequest{
Subject: selfSigned.Subject,
ExtraExtensions: selfSigned.Extensions,
Subject: csrSubject,
}, signer)
if err != nil {
return nil, fmt.Errorf("failed to create and sign CSR: %s", err)
}

return csrDER, nil
}
91 changes: 55 additions & 36 deletions cmd/ceremony/cert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"encoding/asn1"
"encoding/hex"
"errors"
"fmt"
"testing"

"github.com/letsencrypt/boulder/pkcs11helpers"
Expand Down Expand Up @@ -42,6 +43,20 @@ func TestParseOID(t *testing.T) {
test.Assert(t, oid.Equal(asn1.ObjectIdentifier{1, 2, 3}), "parseOID returned incorrect OID")
}

func TestMakeSubject(t *testing.T) {
profile := &certProfile{
CommonName: "common name",
Organization: "organization",
Country: "country",
}
expectedSubject := pkix.Name{
CommonName: "common name",
Organization: []string{"organization"},
Country: []string{"country"},
}
test.AssertDeepEquals(t, profile.Subject(), expectedSubject)
}

func TestMakeTemplate(t *testing.T) {
s, ctx := pkcs11helpers.NewSessionWithMock()
profile := &certProfile{}
Expand Down Expand Up @@ -431,6 +446,42 @@ func TestVerifyProfile(t *testing.T) {
certType: requestCert,
expectedErr: "signature-algorithm cannot be set for a CSR",
},
{
profile: certProfile{
OCSPURL: "a",
},
certType: requestCert,
expectedErr: "ocsp-url cannot be set for a CSR",
},
{
profile: certProfile{
CRLURL: "a",
},
certType: requestCert,
expectedErr: "crl-url cannot be set for a CSR",
},
{
profile: certProfile{
IssuerURL: "a",
},
certType: requestCert,
expectedErr: "issuer-url cannot be set for a CSR",
},
{
profile: certProfile{
Policies: []policyInfoConfig{
{OID: "1.2.3"}, {OID: "1.2.3.4", CPSURI: "hello"}},
},
certType: requestCert,
expectedErr: "policies cannot be set for a CSR",
},
{
profile: certProfile{
KeyUsages: []string{"a"},
},
certType: requestCert,
expectedErr: "key-usages cannot be set for a CSR",
},
} {
err := tc.profile.verifyProfile(tc.certType)
if err != nil {
Expand All @@ -444,55 +495,23 @@ func TestVerifyProfile(t *testing.T) {
}

func TestGenerateCSR(t *testing.T) {
s, ctx := pkcs11helpers.NewSessionWithMock()
randReader := newRandReader(s)
pubKeyBytes := samplePubkey()
pubKey, err := x509.ParsePKIXPublicKey(pubKeyBytes)
test.AssertNotError(t, err, "failed to parse test key")
profile := &certProfile{
CommonName: "common name",
Organization: "organization",
Country: "country",
KeyUsages: []string{"Digital Signature", "CRL Sign"},
OCSPURL: "ocsp",
CRLURL: "crl",
IssuerURL: "issuer",
Policies: []policyInfoConfig{
{
OID: "1.2.3",
CPSURI: "hello",
},
{
OID: "1.2.3.4",
},
},
}
ctx.GenerateRandomFunc = realRand

signer, err := rsa.GenerateKey(rand.Reader, 1024)
test.AssertNotError(t, err, "failed to generate test key")

csrBytes, err := generateCSR(profile, randReader, pubKeyBytes, pubKey, &wrappedSigner{signer})
csrBytes, err := generateCSR(profile, &wrappedSigner{signer})
test.AssertNotError(t, err, "failed to generate CSR")

csr, err := x509.ParseCertificateRequest(csrBytes)
test.AssertNotError(t, err, "failed to parse CSR")
test.AssertNotError(t, csr.CheckSignature(), "CSR signature check failed")
test.AssertEquals(t, len(csr.Extensions), 7)
test.AssertEquals(t, len(csr.Extensions), 0)

containsExt := func(oid asn1.ObjectIdentifier, extensions []pkix.Extension) bool {
for _, ext := range extensions {
if ext.Id.Equal(oid) {
return true
}
}
return false
}
test.Assert(t, containsExt(asn1.ObjectIdentifier{2, 5, 29, 15}, csr.Extensions), "CSR doesn't contain keyUsage extension")
test.Assert(t, containsExt(asn1.ObjectIdentifier{2, 5, 29, 37}, csr.Extensions), "CSR doesn't contain extKeyUsage extension")
test.Assert(t, containsExt(asn1.ObjectIdentifier{2, 5, 29, 19}, csr.Extensions), "CSR doesn't contain basicConstraints extension")
test.Assert(t, containsExt(asn1.ObjectIdentifier{2, 5, 29, 14}, csr.Extensions), "CSR doesn't contain subjectKeyIdentifier extension")
test.Assert(t, containsExt(asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 1}, csr.Extensions), "CSR doesn't contain authorityInfoAccess extension")
test.Assert(t, containsExt(asn1.ObjectIdentifier{2, 5, 29, 31}, csr.Extensions), "CSR doesn't contain cRLDistributionPoints extension")
test.Assert(t, containsExt(asn1.ObjectIdentifier{2, 5, 29, 32}, csr.Extensions), "CSR doesn't contain certificatePolicies extension")
test.AssertEquals(t, csr.Subject.String(), fmt.Sprintf("CN=%s,O=%s,C=%s",
profile.CommonName, profile.Organization, profile.Country))
}
22 changes: 2 additions & 20 deletions cmd/ceremony/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"errors"
"flag"
"fmt"
"io"
"io/ioutil"
"log"
"os"
Expand Down Expand Up @@ -525,23 +524,6 @@ func intermediateCeremony(configBytes []byte, ct certType) error {
return nil
}

// csrSelfSigner is a crypto.Signer that returns an empty signature. When generating a CSR we first
// generate a self-signed certificate so that we can get extension generation for free. Instead of
// creating a throwaway key to sign that certificate we just use a signer that returns an empty
// signature, since x509.CreateCertificate doesn't really care about the actual contents of the
// signature itself.
type csrSelfSigner struct {
pub crypto.PublicKey
}

func (css *csrSelfSigner) Sign(_ io.Reader, _ []byte, _ crypto.SignerOpts) ([]byte, error) {
return []byte{}, nil
}

func (css *csrSelfSigner) Public() crypto.PublicKey {
return css.pub
}

func csrCeremony(configBytes []byte) error {
var config csrConfig
err := yaml.UnmarshalStrict(configBytes, &config)
Expand All @@ -565,12 +547,12 @@ func csrCeremony(configBytes []byte) error {
return fmt.Errorf("failed to parse public key: %s", err)
}

signer, randReader, err := openSigner(config.PKCS11, pub)
signer, _, err := openSigner(config.PKCS11, pub)
if err != nil {
return err
}

csrDER, err := generateCSR(&config.CertProfile, randReader, pubPEM.Bytes, pub, signer)
csrDER, err := generateCSR(&config.CertProfile, signer)
if err != nil {
return fmt.Errorf("failed to generate CSR: %s", err)
}
Expand Down
3 changes: 0 additions & 3 deletions cmd/ceremony/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -479,9 +479,6 @@ func TestCSRConfigValidate(t *testing.T) {
CommonName: "d",
Organization: "e",
Country: "f",
OCSPURL: "g",
CRLURL: "h",
IssuerURL: "i",
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ version: '3'
services:
boulder:
# To minimize fetching this should be the same version used below
image: letsencrypt/boulder-tools:${BOULDER_TOOLS_TAG:-go1.15.7_2021-02-25}
image: letsencrypt/boulder-tools:${BOULDER_TOOLS_TAG:-go1.15.7_2021-03-08}
environment:
- FAKE_DNS=10.77.77.77
- BOULDER_CONFIG_DIR=test/config
Expand Down Expand Up @@ -76,7 +76,7 @@ services:
logging:
driver: none
netaccess:
image: letsencrypt/boulder-tools:${BOULDER_TOOLS_TAG:-go1.15.7_2021-02-25}
image: letsencrypt/boulder-tools:${BOULDER_TOOLS_TAG:-go1.15.7_2021-03-08}
environment:
GO111MODULE: "on"
GOFLAGS: "-mod=vendor"
Expand Down
2 changes: 1 addition & 1 deletion test/boulder-tools/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ GO111MODULE=on go get \

# Pebble's latest version is v2+, but it's not properly go mod compatible, so we
# fetch it in GOPATH mode.
go get github.com/letsencrypt/pebble/cmd/pebble-challtestsrv
GO111MODULE=off go get github.com/letsencrypt/pebble/cmd/pebble-challtestsrv

go clean -cache
go clean -modcache
Expand Down
2 changes: 1 addition & 1 deletion test/boulder-tools/tag_and_upload.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ cd $(dirname $0)
DATESTAMP=$(date +%Y-%m-%d)
DOCKER_REPO="letsencrypt/boulder-tools"

GO_VERSIONS=( "1.15.7" )
GO_VERSIONS=( "1.15.7" "1.16" )

# Build a tagged image for each GO_VERSION
for GO_VERSION in "${GO_VERSIONS[@]}"
Expand Down
4 changes: 4 additions & 0 deletions va/tlsalpn.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ func certNames(cert *x509.Certificate) []string {
}
names = append(names, cert.DNSNames...)
names = core.UniqueLowerNames(names)
// TODO(#5321): This for loop can be deleted after new builds of boulder use
// golang 1.16. In 1.16, code was added to crypto/x509 to not allow
// invalid unicode into a DNSName in a SAN. An error will be caught in
// the standard library before it gets to this point.
for i, n := range names {
names[i] = replaceInvalidUTF8([]byte(n))
}
Expand Down

0 comments on commit ad914f1

Please sign in to comment.