Skip to content

Commit

Permalink
Remove weakKeyFile and blockedKeyFile support (letsencrypt#7783)
Browse files Browse the repository at this point in the history
Goodkey has two ways to detect a key as weak: it runs a variety of
algorithmic checks (such as Fermat factorization and rocacheck), or the
key can be listed in a "weak key file". Similarly, it has two ways to
detect a key as blocked: it can call a generic function (which we use to
query our database), or the key can be listed in a "blocked key file".

This is two methods too many. Reliance on files of weak or blocked keys
introduces unnecessary complexity to both the implementation and
configuration of the goodkey package. Remove both "key file" options and
delete all code which supported them.

Also remove //test/block-a-key, as it was only used to generate these
test files.

IN-10762 tracked the removal of these files in prod.

Fixes letsencrypt#7748
  • Loading branch information
aarongable authored Nov 6, 2024
1 parent 6a2819a commit 2603aa4
Show file tree
Hide file tree
Showing 24 changed files with 7 additions and 709 deletions.
6 changes: 0 additions & 6 deletions cmd/cert-checker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -547,12 +547,6 @@ func main() {
// Validate PA config and set defaults if needed.
cmd.FailOnError(config.PA.CheckChallenges(), "Invalid PA configuration")

if config.CertChecker.GoodKey.WeakKeyFile != "" {
cmd.Fail("cert-checker does not support checking against weak key files")
}
if config.CertChecker.GoodKey.BlockedKeyFile != "" {
cmd.Fail("cert-checker does not support checking against blocked key files")
}
kp, err := sagoodkey.NewPolicy(&config.CertChecker.GoodKey, nil)
cmd.FailOnError(err, "Unable to create key policy")

Expand Down
95 changes: 0 additions & 95 deletions goodkey/blocked.go

This file was deleted.

100 changes: 0 additions & 100 deletions goodkey/blocked_test.go

This file was deleted.

45 changes: 2 additions & 43 deletions goodkey/good_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,6 @@ type Config struct {
// AllowedKeys enables or disables specific key algorithms and sizes. If
// nil, defaults to just those keys allowed by the Let's Encrypt CPS.
AllowedKeys *AllowedKeys
// WeakKeyFile is the path to a JSON file containing truncated modulus hashes
// of known weak RSA keys. If this config value is empty, then RSA modulus
// hash checking will be disabled.
WeakKeyFile string
// BlockedKeyFile is the path to a YAML file containing base64-encoded SHA256
// hashes of PKIX Subject Public Keys that should be blocked. If this config
// value is empty, then blocked key checking will be disabled.
//
// Deprecated: This functionality is better performed by the blockedKeys database
// table, and will be removed in a future release.
BlockedKeyFile string
// FermatRounds is an integer number of rounds of Fermat's factorization
// method that should be performed to attempt to detect keys whose modulus can
// be trivially factored because the two factors are very close to each other.
Expand Down Expand Up @@ -115,17 +104,14 @@ type BlockedKeyCheckFunc func(ctx context.Context, keyHash []byte) (bool, error)
// operations.
type KeyPolicy struct {
allowedKeys AllowedKeys
weakRSAList *WeakRSAKeys
blockedList *blockedKeys
fermatRounds int
blockedCheck BlockedKeyCheckFunc
}

// NewPolicy returns a key policy based on the given configuration, with sane
// defaults. If the config's AllowedKeys is nil, the LetsEncryptCPS AllowedKeys
// is used. If the config's WeakKeyFile or BlockedKeyFile paths are empty, those
// checks are disabled. If the config's FermatRounds is 0, Fermat Factorization
// defaults to attempting 110 rounds.
// is used. If the configured FermatRounds is 0, Fermat Factorization defaults to
// attempting 110 rounds.
func NewPolicy(config *Config, bkc BlockedKeyCheckFunc) (KeyPolicy, error) {
if config == nil {
config = &Config{}
Expand All @@ -138,20 +124,6 @@ func NewPolicy(config *Config, bkc BlockedKeyCheckFunc) (KeyPolicy, error) {
} else {
kp.allowedKeys = *config.AllowedKeys
}
if config.WeakKeyFile != "" {
keyList, err := LoadWeakRSASuffixes(config.WeakKeyFile)
if err != nil {
return KeyPolicy{}, err
}
kp.weakRSAList = keyList
}
if config.BlockedKeyFile != "" {
blocked, err := loadBlockedKeysList(config.BlockedKeyFile)
if err != nil {
return KeyPolicy{}, err
}
kp.blockedList = blocked
}
if config.FermatRounds == 0 {
// The BRs require 100 rounds, so give ourselves a margin above that.
kp.fermatRounds = 110
Expand All @@ -176,15 +148,6 @@ func (policy *KeyPolicy) GoodKey(ctx context.Context, key crypto.PublicKey) erro
default:
return badKey("unsupported key type %T", t)
}
// If there is a blocked list configured then check if the public key is one
// that has been administratively blocked.
if policy.blockedList != nil {
if blocked, err := policy.blockedList.blocked(key); err != nil {
return fmt.Errorf("error checking blocklist for key: %v", key)
} else if blocked {
return badKey("public key is forbidden")
}
}
if policy.blockedCheck != nil {
digest, err := core.KeyDigest(key)
if err != nil {
Expand Down Expand Up @@ -329,10 +292,6 @@ func (policy *KeyPolicy) goodKeyRSA(key *rsa.PublicKey) error {
return err
}

if policy.weakRSAList != nil && policy.weakRSAList.Known(key) {
return badKey("key is on a known weak RSA key list")
}

// Rather than support arbitrary exponents, which significantly increases
// the size of the key space we allow, we restrict E to the defacto standard
// RSA exponent 65537. There is no specific standards document that specifies
Expand Down
7 changes: 0 additions & 7 deletions goodkey/good_key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,6 @@ func TestUnknownKeyType(t *testing.T) {
err := testingPolicy.GoodKey(context.Background(), notAKey)
test.AssertError(t, err, "Should have rejected a key of unknown type")
test.AssertEquals(t, err.Error(), "unsupported key type struct {}")

// Check for early rejection and that no error is seen from blockedKeys.blocked.
testingPolicyWithBlockedKeys := *testingPolicy
testingPolicyWithBlockedKeys.blockedList = &blockedKeys{}
err = testingPolicyWithBlockedKeys.GoodKey(context.Background(), notAKey)
test.AssertError(t, err, "Should have rejected a key of unknown type")
test.AssertEquals(t, err.Error(), "unsupported key type struct {}")
}

func TestNilKey(t *testing.T) {
Expand Down
66 changes: 0 additions & 66 deletions goodkey/weak.go

This file was deleted.

Loading

0 comments on commit 2603aa4

Please sign in to comment.