Skip to content

Commit

Permalink
Add a blocked keys table, and use it (letsencrypt#4773)
Browse files Browse the repository at this point in the history
  • Loading branch information
rolandshoemaker authored Apr 15, 2020
1 parent 5254844 commit 9df97cb
Show file tree
Hide file tree
Showing 34 changed files with 742 additions and 165 deletions.
1 change: 1 addition & 0 deletions ca/ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,7 @@ func (ca *CertificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
}

if err := csrlib.VerifyCSR(
ctx,
csr,
ca.maxNames,
&ca.keyPolicy,
Expand Down
12 changes: 8 additions & 4 deletions cmd/boulder-ca/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
pkcs11key "github.com/letsencrypt/pkcs11key/v4"

"github.com/letsencrypt/boulder/ca"
"github.com/letsencrypt/boulder/ca/config"
ca_config "github.com/letsencrypt/boulder/ca/config"
caPB "github.com/letsencrypt/boulder/ca/proto"
"github.com/letsencrypt/boulder/cmd"
"github.com/letsencrypt/boulder/core"
Expand Down Expand Up @@ -155,9 +155,6 @@ func main() {
issuers, err := loadIssuers(c)
cmd.FailOnError(err, "Couldn't load issuers")

kp, err := goodkey.NewKeyPolicy(c.CA.WeakKeyFile, c.CA.BlockedKeyFile)
cmd.FailOnError(err, "Unable to create key policy")

tlsConfig, err := c.CA.TLS.Load()
cmd.FailOnError(err, "TLS config")

Expand All @@ -168,6 +165,13 @@ func main() {
cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to SA")
sa := bgrpc.NewStorageAuthorityClient(sapb.NewStorageAuthorityClient(conn))

var blockedKeyFunc goodkey.BlockedKeyCheckFunc
if features.Enabled(features.BlockedKeyTable) {
blockedKeyFunc = sa.KeyBlocked
}
kp, err := goodkey.NewKeyPolicy(c.CA.WeakKeyFile, c.CA.BlockedKeyFile, blockedKeyFunc)
cmd.FailOnError(err, "Unable to create key policy")

var orphanQueue *goque.Queue
if c.CA.OrphanQueueDir != "" {
orphanQueue, err = goque.OpenQueue(c.CA.OrphanQueueDir)
Expand Down
6 changes: 5 additions & 1 deletion cmd/boulder-ra/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,11 @@ func main() {
pendingAuthorizationLifetime = time.Duration(c.RA.PendingAuthorizationLifetimeDays) * 24 * time.Hour
}

kp, err := goodkey.NewKeyPolicy(c.RA.WeakKeyFile, c.RA.BlockedKeyFile)
var blockedKeyFunc goodkey.BlockedKeyCheckFunc
if features.Enabled(features.BlockedKeyTable) {
blockedKeyFunc = sac.KeyBlocked
}
kp, err := goodkey.NewKeyPolicy(c.RA.WeakKeyFile, c.RA.BlockedKeyFile, blockedKeyFunc)
cmd.FailOnError(err, "Unable to create key policy")

if c.RA.MaxNames == 0 {
Expand Down
8 changes: 6 additions & 2 deletions cmd/boulder-wfe/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,14 @@ func main() {

clk := cmd.Clock()

rac, sac, rns, npm := setupWFE(c, logger, stats, clk)
var blockedKeyFunc goodkey.BlockedKeyCheckFunc
if features.Enabled(features.BlockedKeyTable) {
blockedKeyFunc = sac.KeyBlocked
}
// don't load any weak keys, but do load blocked keys
kp, err := goodkey.NewKeyPolicy("", c.WFE.BlockedKeyFile)
kp, err := goodkey.NewKeyPolicy("", c.WFE.BlockedKeyFile, blockedKeyFunc)
cmd.FailOnError(err, "Unable to create key policy")
rac, sac, rns, npm := setupWFE(c, logger, stats, clk)
wfe, err := wfe.NewWebFrontEndImpl(stats, clk, kp, rns, npm, logger)
cmd.FailOnError(err, "Unable to create WFE")
wfe.RA = rac
Expand Down
8 changes: 6 additions & 2 deletions cmd/boulder-wfe2/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,10 +298,14 @@ func main() {

clk := cmd.Clock()

rac, sac, rns, npm := setupWFE(c, logger, stats, clk)
var blockedKeyFunc goodkey.BlockedKeyCheckFunc
if features.Enabled(features.BlockedKeyTable) {
blockedKeyFunc = sac.KeyBlocked
}
// don't load any weak keys, but do load blocked keys
kp, err := goodkey.NewKeyPolicy("", c.WFE.BlockedKeyFile)
kp, err := goodkey.NewKeyPolicy("", c.WFE.BlockedKeyFile, blockedKeyFunc)
cmd.FailOnError(err, "Unable to create key policy")
rac, sac, rns, npm := setupWFE(c, logger, stats, clk)

if c.WFE.StaleTimeout.Duration == 0 {
c.WFE.StaleTimeout.Duration = time.Minute * 10
Expand Down
2 changes: 2 additions & 0 deletions core/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ type StorageGetter interface {
CountInvalidAuthorizations2(ctx context.Context, req *sapb.CountInvalidAuthorizationsRequest) (*sapb.Count, error)
GetValidAuthorizations2(ctx context.Context, req *sapb.GetValidAuthorizationsRequest) (*sapb.Authorizations, error)
SerialExists(ctx context.Context, req *sapb.Serial) (*sapb.Exists, error)
KeyBlocked(ctx context.Context, req *sapb.KeyBlockedRequest) (*sapb.Exists, error)
}

// StorageAdder are the Boulder SA's write/update methods
Expand All @@ -155,6 +156,7 @@ type StorageAdder interface {
NewAuthorizations2(ctx context.Context, req *sapb.AddPendingAuthorizationsRequest) (*sapb.Authorization2IDs, error)
FinalizeAuthorization2(ctx context.Context, req *sapb.FinalizeAuthorizationRequest) error
DeactivateAuthorization2(ctx context.Context, req *sapb.AuthorizationID2) (*corepb.Empty, error)
AddBlockedKey(ctx context.Context, req *sapb.AddBlockedKeyRequest) (*corepb.Empty, error)
}

// StorageAuthority interface represents a simple key/value
Expand Down
5 changes: 3 additions & 2 deletions csr/csr.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package csr

import (
"context"
"crypto"
"crypto/x509"
"strings"
Expand Down Expand Up @@ -44,13 +45,13 @@ var (
// VerifyCSR checks the validity of a x509.CertificateRequest. Before doing checks it normalizes
// the CSR which lowers the case of DNS names and subject CN, and if forceCNFromSAN is true it
// will hoist a DNS name into the CN if it is empty.
func VerifyCSR(csr *x509.CertificateRequest, maxNames int, keyPolicy *goodkey.KeyPolicy, pa core.PolicyAuthority, forceCNFromSAN bool, regID int64) error {
func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, maxNames int, keyPolicy *goodkey.KeyPolicy, pa core.PolicyAuthority, forceCNFromSAN bool, regID int64) error {
normalizeCSR(csr, forceCNFromSAN)
key, ok := csr.PublicKey.(crypto.PublicKey)
if !ok {
return invalidPubKey
}
if err := keyPolicy.GoodKey(key); err != nil {
if err := keyPolicy.GoodKey(ctx, key); err != nil {
return berrors.BadPublicKeyError("invalid public key in CSR: %s", err)
}
if !goodSignatureAlgorithms[csr.SignatureAlgorithm] {
Expand Down
3 changes: 2 additions & 1 deletion csr/csr_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package csr

import (
"context"
"crypto/rand"
"crypto/rsa"
"crypto/x509"
Expand Down Expand Up @@ -170,7 +171,7 @@ func TestVerifyCSR(t *testing.T) {
}

for _, c := range cases {
err := VerifyCSR(c.csr, c.maxNames, c.keyPolicy, c.pa, true, c.regID)
err := VerifyCSR(context.Background(), c.csr, c.maxNames, c.keyPolicy, c.pa, true, c.regID)
test.AssertDeepEquals(t, c.expectedError, err)
}
}
Expand Down
5 changes: 3 additions & 2 deletions features/featureflag_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions features/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ const (
StoreIssuerInfo
// StoreKeyHashes enables storage of SPKI hashes associated with certificates.
StoreKeyHashes
// BlockedKeyTable enables storage, and checking, of the blockedKeys table in addition
// to the blocked key list
BlockedKeyTable
)

// List of features and their default value, protected by fMu
Expand All @@ -71,6 +74,7 @@ var features = map[FeatureFlag]bool{
StoreIssuerInfo: false,
WriteIssuedNamesPrecert: false,
StoreKeyHashes: false,
BlockedKeyTable: false,
}

var fMu = new(sync.RWMutex)
Expand Down
5 changes: 3 additions & 2 deletions goodkey/blocked_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package goodkey

import (
"context"
"crypto"
"io/ioutil"
"os"
Expand Down Expand Up @@ -85,7 +86,7 @@ func TestBlockedKeys(t *testing.T) {

// All of the test keys should not be considered blocked
for _, k := range blockedKeys {
err := testingPolicy.GoodKey(k)
err := testingPolicy.GoodKey(context.Background(), k)
test.AssertNotError(t, err, "test key was blocked by key policy without block list")
}

Expand All @@ -95,7 +96,7 @@ func TestBlockedKeys(t *testing.T) {
// Now all of the test keys should be considered blocked, and with the correct
// type of error.
for _, k := range blockedKeys {
err := testingPolicy.GoodKey(k)
err := testingPolicy.GoodKey(context.Background(), k)
test.AssertError(t, err, "test key was not blocked by key policy with block list")
test.Assert(t, berrors.Is(err, berrors.BadPublicKey), "err was not BadPublicKey error")
}
Expand Down
28 changes: 26 additions & 2 deletions goodkey/good_key.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
package goodkey

import (
"context"
"crypto"
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rsa"
"math/big"
"sync"

"github.com/letsencrypt/boulder/core"
berrors "github.com/letsencrypt/boulder/errors"
sapb "github.com/letsencrypt/boulder/sa/proto"

"github.com/titanous/rocacheck"
)

Expand All @@ -34,6 +38,11 @@ var (
smallPrimes []*big.Int
)

// BlockedKeyCheckFunc is used to pass in the sa.BlockedKey method to KeyPolicy,
// rather than storing a full sa.SQLStorageAuthority. This makes testing
// significantly simpler.
type BlockedKeyCheckFunc func(context.Context, *sapb.KeyBlockedRequest) (*sapb.Exists, error)

// KeyPolicy determines which types of key may be used with various boulder
// operations.
type KeyPolicy struct {
Expand All @@ -42,6 +51,7 @@ type KeyPolicy struct {
AllowECDSANISTP384 bool // Whether ECDSA NISTP384 keys should be allowed.
weakRSAList *WeakRSAKeys
blockedList *blockedKeys
dbCheck BlockedKeyCheckFunc
}

// NewKeyPolicy returns a KeyPolicy that allows RSA, ECDSA256 and ECDSA384.
Expand All @@ -51,11 +61,12 @@ type KeyPolicy struct {
// containing Base64 encoded SHA256 hashes of pkix subject public keys that
// should be blocked. If this argument is empty then no blocked key checking is
// performed.
func NewKeyPolicy(weakKeyFile, blockedKeyFile string) (KeyPolicy, error) {
func NewKeyPolicy(weakKeyFile, blockedKeyFile string, bkc BlockedKeyCheckFunc) (KeyPolicy, error) {
kp := KeyPolicy{
AllowRSA: true,
AllowECDSANISTP256: true,
AllowECDSANISTP384: true,
dbCheck: bkc,
}
if weakKeyFile != "" {
keyList, err := LoadWeakRSASuffixes(weakKeyFile)
Expand All @@ -79,7 +90,7 @@ func NewKeyPolicy(weakKeyFile, blockedKeyFile string) (KeyPolicy, error) {
// strength and algorithm checking. GoodKey only supports pointers: *rsa.PublicKey
// and *ecdsa.PublicKey. It will reject non-pointer types.
// TODO: Support JSONWebKeys once go-jose migration is done.
func (policy *KeyPolicy) GoodKey(key crypto.PublicKey) error {
func (policy *KeyPolicy) GoodKey(ctx context.Context, key crypto.PublicKey) error {
// If there is a blocked list configured then check if the public key is one
// that has been administratively blocked.
if policy.blockedList != nil {
Expand All @@ -89,6 +100,19 @@ func (policy *KeyPolicy) GoodKey(key crypto.PublicKey) error {
return berrors.BadPublicKeyError("public key is forbidden")
}
}
if policy.dbCheck != nil {
digest, err := core.KeyDigest(key)
if err != nil {
return err
}
exists, err := policy.dbCheck(ctx, &sapb.KeyBlockedRequest{KeyHash: digest[:]})
if err != nil {
return err
}
if *exists.Exists {
return berrors.BadPublicKeyError("public key is forbidden")
}
}
switch t := key.(type) {
case *rsa.PublicKey:
return policy.goodKeyRSA(t)
Expand Down
Loading

0 comments on commit 9df97cb

Please sign in to comment.