Skip to content

Commit

Permalink
goodkey: default to 110 rounds of Fermat factorization (#7579)
Browse files Browse the repository at this point in the history
This change guarantees compliance with CA/BF Ballot SC-073 "Compromised
and Weak Keys", which requires that at least 100 rounds of Fermat
Factorization be attempted:

> Section 6.1.1.3 Subscriber Key Pair Generation
> The CA SHALL reject a certificate request if... The Public Key
corresponds to an industry-demonstrated weak Private Key. For requests
submitted on or after November 15, 2024,... In the case of Close Primes
vulnerability (https://fermatattack.secvuln.info/), the CA SHALL reject
weak keys which can be factored within 100 rounds using Fermat’s
factorization method.

We choose 110 rounds to ensure a margin above and beyond the requirements.

Fixes #7558
  • Loading branch information
aarongable authored Jul 17, 2024
1 parent 2c15725 commit a3e9943
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 20 deletions.
2 changes: 1 addition & 1 deletion cmd/ceremony/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var kp goodkey.KeyPolicy

func init() {
var err error
kp, err = goodkey.NewPolicy(&goodkey.Config{FermatRounds: 100}, nil)
kp, err = goodkey.NewPolicy(nil, nil)
if err != nil {
log.Fatal("Could not create goodkey.KeyPolicy")
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/cert-checker/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func init() {
if err != nil {
log.Fatal(err)
}
kp, err = sagoodkey.NewPolicy(&goodkey.Config{FermatRounds: 100}, nil)
kp, err = sagoodkey.NewPolicy(nil, nil)
if err != nil {
log.Fatal(err)
}
Expand Down
23 changes: 13 additions & 10 deletions goodkey/good_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ type Config struct {
// 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.
// If this config value is empty (0), no factorization will be attempted.
// If this config value is empty or 0, it will default to 110 rounds.
FermatRounds int
}

Expand Down Expand Up @@ -122,7 +122,7 @@ type KeyPolicy struct {
// 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
// is disabled.
// defaults to attempting 110 rounds.
func NewPolicy(config *Config, bkc BlockedKeyCheckFunc) (KeyPolicy, error) {
if config == nil {
config = &Config{}
Expand All @@ -149,10 +149,14 @@ func NewPolicy(config *Config, bkc BlockedKeyCheckFunc) (KeyPolicy, error) {
}
kp.blockedList = blocked
}
if config.FermatRounds < 0 {
return KeyPolicy{}, fmt.Errorf("Fermat factorization rounds cannot be negative: %d", config.FermatRounds)
if config.FermatRounds == 0 {
// The BRs require 100 rounds, so give ourselves a margin above that.
kp.fermatRounds = 110
} else if config.FermatRounds < 100 {
return KeyPolicy{}, fmt.Errorf("Fermat factorization rounds must be at least 100: %d", config.FermatRounds)
} else {
kp.fermatRounds = config.FermatRounds
}
kp.fermatRounds = config.FermatRounds
return kp, nil
}

Expand Down Expand Up @@ -354,12 +358,11 @@ func (policy *KeyPolicy) goodKeyRSA(key *rsa.PublicKey) error {
if rocacheck.IsWeak(key) {
return badKey("key generated by vulnerable Infineon-based hardware")
}

// Check if the key can be easily factored via Fermat's factorization method.
if policy.fermatRounds > 0 {
err := checkPrimeFactorsTooClose(modulus, policy.fermatRounds)
if err != nil {
return badKey("key generated with factors too close together: %w", err)
}
err = checkPrimeFactorsTooClose(modulus, policy.fermatRounds)
if err != nil {
return badKey("key generated with factors too close together: %w", err)
}

return nil
Expand Down
2 changes: 1 addition & 1 deletion goodkey/good_key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ func TestDefaultAllowedKeys(t *testing.T) {
test.Assert(t, policy.allowedKeys.ECDSAP384, "NIST P384 should be allowed")
test.Assert(t, !policy.allowedKeys.ECDSAP521, "NIST P521 should not be allowed")

policy, err = NewPolicy(&Config{FermatRounds: 100}, nil)
policy, err = NewPolicy(&Config{}, nil)
test.AssertNotError(t, err, "NewPolicy with nil config.AllowedKeys failed")
test.Assert(t, policy.allowedKeys.RSA2048, "RSA 2048 should be allowed")
test.Assert(t, policy.allowedKeys.RSA3072, "RSA 3072 should be allowed")
Expand Down
3 changes: 1 addition & 2 deletions test/config-next/ca.json
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,7 @@
"lifespanOCSP": "96h",
"goodkey": {
"weakKeyFile": "test/example-weak-keys.json",
"blockedKeyFile": "test/example-blocked-keys.yaml",
"fermatRounds": 100
"blockedKeyFile": "test/example-blocked-keys.yaml"
},
"ocspLogMaxLength": 4000,
"ocspLogPeriod": "500ms",
Expand Down
3 changes: 0 additions & 3 deletions test/config-next/cert-checker.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@
"maxOpenConns": 10
},
"hostnamePolicyFile": "test/hostname-policy.yaml",
"goodkey": {
"fermatRounds": 100
},
"workers": 16,
"unexpiredOnly": true,
"badResultsOnly": true,
Expand Down
3 changes: 1 addition & 2 deletions test/config-next/ra.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
"pendingAuthorizationLifetimeDays": 7,
"goodkey": {
"weakKeyFile": "test/example-weak-keys.json",
"blockedKeyFile": "test/example-blocked-keys.yaml",
"fermatRounds": 100
"blockedKeyFile": "test/example-blocked-keys.yaml"
},
"orderLifetime": "168h",
"finalizeTimeout": "30s",
Expand Down

0 comments on commit a3e9943

Please sign in to comment.