Skip to content

Commit

Permalink
local: lint tbs certificates with zlint.
Browse files Browse the repository at this point in the history
  • Loading branch information
Daniel committed Jul 1, 2019
1 parent 66f7d5d commit 0434ea0
Show file tree
Hide file tree
Showing 2 changed files with 233 additions and 9 deletions.
96 changes: 88 additions & 8 deletions signer/local/local.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"encoding/hex"
"encoding/pem"
"errors"
"fmt"
"io"
"math/big"
"net"
Expand All @@ -31,6 +32,10 @@ import (
"github.com/google/certificate-transparency-go"
"github.com/google/certificate-transparency-go/client"
"github.com/google/certificate-transparency-go/jsonclient"

zx509 "github.com/zmap/zcrypto/x509"
"github.com/zmap/zlint"
"github.com/zmap/zlint/lints"
"golang.org/x/net/context"
)

Expand Down Expand Up @@ -61,8 +66,8 @@ func NewSigner(priv crypto.Signer, cert *x509.Certificate, sigAlgo x509.Signatur
}

var lintPriv crypto.Signer
// If there is at least one profile that configures pre-issuance linting then
// generate the one-off lintPriv key.
// If there is at least one profile (including the default) that configures
// pre-issuance linting then generate the one-off lintPriv key.
for _, profile := range policy.Profiles {
if profile.LintErrLevel > 0 || policy.Default.LintErrLevel > 0 {
// In the future there may be demand for specifying the type of signer used
Expand Down Expand Up @@ -121,7 +126,73 @@ func NewSignerFromFile(caFile, caKeyFile string, policy *config.Signing) (*Signe
return NewSigner(priv, parsedCa, signer.DefaultSigAlgo(priv), policy)
}

func (s *Signer) sign(template *x509.Certificate) (cert []byte, err error) {
// LintError is an error type returned when pre-issuance linting is configured
// in a signing profile and a TBS Certificate fails linting. It wraps the
// concrete zlint LintResults so that callers can further inspect the cause of
// the failing lints.
type LintError struct {
ErrorResults map[string]lints.LintResult
}

func (e *LintError) Error() string {
return fmt.Sprintf("pre-issuance linting found %d error results",
len(e.ErrorResults))
}

// lint performs pre-issuance linting of a given TBS certificate template when
// the provided errLevel is > 0. Any lint results with a status higher than the
// errLevel that isn't created by a lint in the ignoreMap will result in
// a LintError being returned to the caller. Note that the template is provided
// by-value and not by-reference. This is important as the lint function needs
// to mutate the template's signature algorithm to match the lintPriv.
func (s *Signer) lint(template x509.Certificate, errLevel int, ignoreMap map[string]bool) error {
// Always return nil when linting is disabled
if errLevel == 0 {
return nil
}
// without a lintPriv key to use to sign the tbsCertificate we can't lint it.
if s.lintPriv == nil {
return cferr.New(cferr.PrivateKeyError, cferr.Unavailable)
}

// The template's SignatureAlgorithm must be mutated to match the lintPriv or
// x509.CreateCertificate will error because of the mismatch. At the time of
// writing s.lintPriv is always an ECDSA private key. This switch will need to
// be expanded if the lint key type is made configurable.
switch s.lintPriv.(type) {
case *ecdsa.PrivateKey:
template.SignatureAlgorithm = x509.ECDSAWithSHA256
default:
return cferr.New(cferr.PrivateKeyError, cferr.KeyMismatch)
}

prelintBytes, err := x509.CreateCertificate(rand.Reader, &template, s.ca, template.PublicKey, s.lintPriv)
if err != nil {
return cferr.Wrap(cferr.CertificateError, cferr.Unknown, err)
}
prelintCert, err := zx509.ParseCertificate(prelintBytes)
if err != nil {
return cferr.Wrap(cferr.CertificateError, cferr.ParseFailed, err)
}
errorResults := map[string]lints.LintResult{}
results := zlint.LintCertificate(prelintCert)
for name, res := range results.Results {
if ignoreMap[name] {
continue
}
if int(res.Status) > errLevel {
errorResults[name] = *res
}
}
if len(errorResults) > 0 {
return &LintError{
ErrorResults: errorResults,
}
}
return nil
}

func (s *Signer) sign(template *x509.Certificate, lintErrLevel int, lintIgnore map[string]bool) (cert []byte, err error) {
var initRoot bool
if s.ca == nil {
if !template.IsCA {
Expand All @@ -135,6 +206,10 @@ func (s *Signer) sign(template *x509.Certificate) (cert []byte, err error) {
initRoot = true
}

if err := s.lint(*template, lintErrLevel, lintIgnore); err != nil {
return nil, err
}

derBytes, err := x509.CreateCertificate(rand.Reader, template, s.ca, template.PublicKey, s.priv)
if err != nil {
return nil, cferr.Wrap(cferr.CertificateError, cferr.Unknown, err)
Expand Down Expand Up @@ -379,7 +454,7 @@ func (s *Signer) Sign(req signer.SignRequest) (cert []byte, err error) {
var poisonExtension = pkix.Extension{Id: signer.CTPoisonOID, Critical: true, Value: []byte{0x05, 0x00}}
var poisonedPreCert = certTBS
poisonedPreCert.ExtraExtensions = append(safeTemplate.ExtraExtensions, poisonExtension)
cert, err = s.sign(&poisonedPreCert)
cert, err = s.sign(&poisonedPreCert, profile.LintErrLevel, profile.IgnoredLintsMap)
if err != nil {
return
}
Expand Down Expand Up @@ -422,8 +497,9 @@ func (s *Signer) Sign(req signer.SignRequest) (cert []byte, err error) {
var SCTListExtension = pkix.Extension{Id: signer.SCTListOID, Critical: false, Value: serializedSCTList}
certTBS.ExtraExtensions = append(certTBS.ExtraExtensions, SCTListExtension)
}

var signedCert []byte
signedCert, err = s.sign(&certTBS)
signedCert, err = s.sign(&certTBS, profile.LintErrLevel, profile.IgnoredLintsMap)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -461,7 +537,9 @@ func (s *Signer) Sign(req signer.SignRequest) (cert []byte, err error) {
// except for the removal of the poison extension and the addition of the SCT list
// extension. SignFromPrecert does not verify that the contents of the certificate
// still match the signing profile of the signer, it only requires that the precert
// was previously signed by the Signers CA.
// was previously signed by the Signers CA. Similarly, any linting configured
// by the profile used to sign the precert will not be re-applied to the final
// cert and must be done separately by the caller.
func (s *Signer) SignFromPrecert(precert *x509.Certificate, scts []ct.SignedCertificateTimestamp) ([]byte, error) {
// Verify certificate was signed by s.ca
if err := precert.CheckSignatureFrom(s.ca); err != nil {
Expand Down Expand Up @@ -530,8 +608,10 @@ func (s *Signer) SignFromPrecert(precert *x509.Certificate, scts []ct.SignedCert
// Insert the SCT list extension
tbsCert.ExtraExtensions = append(tbsCert.ExtraExtensions, sctExt)

// Sign the tbsCert
return s.sign(&tbsCert)
// Sign the tbsCert. Linting is always disabled because there is no way for
// this API to know the correct lint settings to use because there is no
// reference to the signing profile of the precert available.
return s.sign(&tbsCert, 0, nil)
}

// Info return a populated info.Resp struct or an error.
Expand Down
146 changes: 145 additions & 1 deletion signer/local/local_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ package local

import (
"bytes"
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rand"
"crypto/rsa"
"crypto/x509"
"crypto/x509/pkix"
"encoding/asn1"
"encoding/hex"
"encoding/pem"
"errors"
"io/ioutil"
"math/big"
"net"
Expand All @@ -29,6 +32,7 @@ import (
"github.com/cloudflare/cfssl/log"
"github.com/cloudflare/cfssl/signer"
"github.com/google/certificate-transparency-go"
"github.com/zmap/zlint/lints"
)

const (
Expand Down Expand Up @@ -1407,7 +1411,7 @@ func TestSignFromPrecert(t *testing.T) {
IPAddresses: []net.IP{net.ParseIP("1.1.1.1")},
CRLDistributionPoints: []string{"crl"},
PolicyIdentifiers: []asn1.ObjectIdentifier{{1, 2, 3}},
})
}, 0, nil)
if err != nil {
t.Fatalf("Failed to sign request: %s", err)
}
Expand Down Expand Up @@ -1478,3 +1482,143 @@ func TestSignFromPrecert(t *testing.T) {
t.Fatal("SignFromPrecert didn't fail with signature not from CA")
}
}

func TestLint(t *testing.T) {
k, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
serial := big.NewInt(1337)

// jankyTemplate is an x509 cert template that mostly passes through zlint
// without errors/warnings. It is used as the basis of both the signer's issuing
// certificate and the end entity certificate that is linted.
jankyTemplate := &x509.Certificate{
Subject: pkix.Name{
CommonName: "janky.cert",
},
SerialNumber: serial,
NotBefore: time.Now(),
NotAfter: time.Now().AddDate(0, 0, 90),
KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},
PolicyIdentifiers: []asn1.ObjectIdentifier{
{1, 2, 3},
},
BasicConstraintsValid: true,
IsCA: true,
IssuingCertificateURL: []string{"http://ca.cpu"},
SubjectKeyId: []byte("⚿"),
PublicKey: k.Public(),
}

// Create a self-signed issuer certificate to use as the CA
issuerDer, _ := x509.CreateCertificate(rand.Reader, jankyTemplate, jankyTemplate, k.Public(), k)
issuerCert, _ := x509.ParseCertificate(issuerDer)

lintSigner := &Signer{
lintPriv: k,
ca: issuerCert,
}

// Reconfigure the template for an end-entity certificate.
// On purpose this template will trip the following lints:
// 1. e_sub_cert_aia_does_not_contain_ocsp_url because there is no OCSP URL.
// 2. e_dnsname_not_valid_tld because `.cert` is not a real TLD
// 3. w_serial_number_low_entropy because '1338' is a static constant <8
// bytes in length.
serial = big.NewInt(1338)
jankyTemplate.SerialNumber = serial
jankyTemplate.Subject.CommonName = "www.janky.cert"
jankyTemplate.DNSNames = []string{"janky.cert", "www.janky.cert"}
jankyTemplate.KeyUsage = x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment
jankyTemplate.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}
jankyTemplate.IsCA = false

testCases := []struct {
name string
signer *Signer
lintErrLevel int
ignoredLintMap map[string]bool
expectedErr error
expectedErrResults map[string]lints.LintResult
}{
{
name: "linting disabled",
signer: lintSigner,
},
{
name: "signer without lint key",
signer: &Signer{},
lintErrLevel: 1,
expectedErr: errors.New(`{"code":2500,"message":"Private key is unavailable"}`),
},
{
name: "lint results above err level",
signer: lintSigner,
lintErrLevel: 4,
expectedErr: errors.New("pre-issuance linting found 3 error results"),
expectedErrResults: map[string]lints.LintResult{
"e_sub_cert_aia_does_not_contain_ocsp_url": lints.LintResult{Status: 6},
"e_dnsname_not_valid_tld": lints.LintResult{Status: 6},
"w_serial_number_low_entropy": lints.LintResult{Status: 5},
},
},
{
name: "lint results below err level",
signer: lintSigner,
lintErrLevel: 5,
expectedErr: errors.New("pre-issuance linting found 2 error results"),
expectedErrResults: map[string]lints.LintResult{
"e_sub_cert_aia_does_not_contain_ocsp_url": lints.LintResult{Status: 6},
"e_dnsname_not_valid_tld": lints.LintResult{Status: 6},
},
},
{
name: "ignored lints, lint results above err level",
signer: lintSigner,
lintErrLevel: 4,
ignoredLintMap: map[string]bool{
"w_serial_number_low_entropy": true,
},
expectedErr: errors.New("pre-issuance linting found 2 error results"),
expectedErrResults: map[string]lints.LintResult{
"e_sub_cert_aia_does_not_contain_ocsp_url": lints.LintResult{Status: 6},
"e_dnsname_not_valid_tld": lints.LintResult{Status: 6},
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
err := tc.signer.lint(*jankyTemplate, tc.lintErrLevel, tc.ignoredLintMap)
if err != nil && tc.expectedErr == nil {
t.Errorf("Expected no err, got %#v", err)
} else if err == nil && tc.expectedErr != nil {
t.Errorf("Expected err %v, got nil", tc.expectedErr)
} else if err != nil && tc.expectedErr != nil {
actual := err.Error()
expected := tc.expectedErr.Error()
if actual != expected {
t.Errorf("Expected err %q got %q", expected, actual)
}
if len(tc.expectedErrResults) > 0 {
le, ok := err.(*LintError)
if !ok {
t.Fatalf("expected LintError type err, got %v", err)
}
if count := len(le.ErrorResults); count != len(tc.expectedErrResults) {
t.Fatalf("expected %d LintError results, got %d", len(tc.expectedErrResults), len(le.ErrorResults))
}
for name, result := range le.ErrorResults {
if result.Status != tc.expectedErrResults[name].Status {
t.Errorf("expected error from lint %q to have status %d not %d",
name, tc.expectedErrResults[name].Status, result.Status)
}
if result.Details != tc.expectedErrResults[name].Details {
t.Errorf("expected error from lint %q to have details %q not %q",
name, tc.expectedErrResults[name].Details, result.Details)
}
}
}
}
})
}
}

0 comments on commit 0434ea0

Please sign in to comment.