From 370b6f9bf9274011154d4f99c67cd1069fcdc4ff Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Sat, 6 Jun 2015 17:12:16 -0700 Subject: [PATCH] Return error from core.GoodKey --- ca/certificate-authority.go | 4 ++-- core/good_key.go | 38 +++++++++++++++++++++--------------- core/good_key_test.go | 24 +++++++++++------------ ra/registration-authority.go | 4 ++-- 4 files changed, 38 insertions(+), 32 deletions(-) diff --git a/ca/certificate-authority.go b/ca/certificate-authority.go index f46f6caa2..441919a05 100644 --- a/ca/certificate-authority.go +++ b/ca/certificate-authority.go @@ -257,8 +257,8 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(csr x509.CertificateRequest ca.log.AuditErr(err) return emptyCert, err } - if !core.GoodKey(key, ca.MaxKeySize) { - err = fmt.Errorf("Invalid public key in CSR.") + if err = core.GoodKey(key, ca.MaxKeySize); err != nil { + err = fmt.Errorf("Invalid public key in CSR: %s", err.Error()) // AUDIT[ Certificate Requests ] 11917fa4-10ef-4e0d-9105-bacbe7836a3c ca.log.AuditErr(err) return emptyCert, err diff --git a/core/good_key.go b/core/good_key.go index 507f1fcdc..61064bb7d 100644 --- a/core/good_key.go +++ b/core/good_key.go @@ -42,7 +42,7 @@ var ( // key use (our requirements are the same for either one), according to basic // strength and algorithm checking. // TODO: Support JsonWebKeys once go-jose migration is done. -func GoodKey(key crypto.PublicKey, maxKeySize int) bool { +func GoodKey(key crypto.PublicKey, maxKeySize int) error { log := blog.GetAuditLogger() switch t := key.(type) { case rsa.PublicKey: @@ -54,30 +54,34 @@ func GoodKey(key crypto.PublicKey, maxKeySize int) bool { case *ecdsa.PublicKey: return GoodKeyECDSA(*t, maxKeySize) default: - log.Debug(fmt.Sprintf("Unknown key type %s", reflect.TypeOf(key))) - return false + err := fmt.Errorf("Unknown key type %s", reflect.TypeOf(key)) + log.Debug(err.Error()) + return err } } -func GoodKeyECDSA(key ecdsa.PublicKey, maxKeySize int) bool { +func GoodKeyECDSA(key ecdsa.PublicKey, maxKeySize int) (err error) { log := blog.GetAuditLogger() - log.Debug(fmt.Sprintf("ECDSA keys not yet supported.")) - return false + err = fmt.Errorf("ECDSA keys not yet supported") + log.Debug(err.Error()) + return } -func GoodKeyRSA(key rsa.PublicKey, maxKeySize int) bool { +func GoodKeyRSA(key rsa.PublicKey, maxKeySize int) (err error) { log := blog.GetAuditLogger() // Baseline Requirements Appendix A // Modulus must be >= 2048 bits and < maxKeySize modulus := key.N modulusBitLen := modulus.BitLen() if modulusBitLen < 2048 { - log.Debug(fmt.Sprintf("Key too small: %d", modulusBitLen)) - return false + err = fmt.Errorf("Key too small: %d", modulusBitLen) + log.Debug(err.Error()) + return err } if modulusBitLen > maxKeySize { - log.Debug(fmt.Sprintf("Key too large: %d > %d", modulusBitLen, maxKeySize)) - return false + err = fmt.Errorf("Key too large: %d > %d", modulusBitLen, maxKeySize) + log.Debug(err.Error()) + return err } // The CA SHALL confirm that the value of the public exponent is an // odd number equal to 3 or more. Additionally, the public exponent @@ -86,8 +90,9 @@ func GoodKeyRSA(key rsa.PublicKey, maxKeySize int) bool { // 2^32 - 1 or 2^64 - 1, because it stores E as an integer. So we // don't need to check the upper bound. if (key.E%2) == 0 || key.E < ((1<<16)+1) { - log.Debug(fmt.Sprintf("Key exponent should be odd and >2^16: %d", key.E)) - return false + err = fmt.Errorf("Key exponent should be odd and >2^16: %d", key.E) + log.Debug(err.Error()) + return err } // The modulus SHOULD also have the following characteristics: an odd // number, not the power of a prime, and have no factors smaller than 752. @@ -101,9 +106,10 @@ func GoodKeyRSA(key rsa.PublicKey, maxKeySize int) bool { var result big.Int result.Mod(modulus, prime) if result.Sign() == 0 { - log.Debug(fmt.Sprintf("Key divisible by small prime: %d", prime)) - return false + err = fmt.Errorf("Key divisible by small prime: %d", prime) + log.Debug(err.Error()) + return err } } - return true + return nil } diff --git a/core/good_key_test.go b/core/good_key_test.go index b97d7972c..9c4b55351 100644 --- a/core/good_key_test.go +++ b/core/good_key_test.go @@ -19,27 +19,27 @@ var maxKeySize int = 2048 func TestUnknownKeyType(t *testing.T) { notAKey := struct{}{} - test.Assert(t, !GoodKey(notAKey, maxKeySize), "Should have rejected a key of unknown type") + test.AssertError(t, GoodKey(notAKey, maxKeySize), "Should have rejected a key of unknown type") } func TestWrongKeyType(t *testing.T) { ecdsaKey := ecdsa.PublicKey{} - test.Assert(t, !GoodKey(&ecdsaKey, maxKeySize), "Should have rejected ECDSA key.") - test.Assert(t, !GoodKey(ecdsaKey, maxKeySize), "Should have rejected ECDSA key.") + test.AssertError(t, GoodKey(&ecdsaKey, maxKeySize), "Should have rejected ECDSA key.") + test.AssertError(t, GoodKey(ecdsaKey, maxKeySize), "Should have rejected ECDSA key.") } func TestSmallModulus(t *testing.T) { private, err := rsa.GenerateKey(rand.Reader, 2040) test.AssertNotError(t, err, "Error generating key") - test.Assert(t, !GoodKey(&private.PublicKey, maxKeySize), "Should have rejected too-short key.") - test.Assert(t, !GoodKey(private.PublicKey, maxKeySize), "Should have rejected too-short key.") + test.AssertError(t, GoodKey(&private.PublicKey, maxKeySize), "Should have rejected too-short key.") + test.AssertError(t, GoodKey(private.PublicKey, maxKeySize), "Should have rejected too-short key.") } func TestLargeModulus(t *testing.T) { private, err := rsa.GenerateKey(rand.Reader, maxKeySize+1) test.AssertNotError(t, err, "Error generating key") - test.Assert(t, !GoodKey(&private.PublicKey, maxKeySize), "Should have rejected too-long key.") - test.Assert(t, !GoodKey(private.PublicKey, maxKeySize), "Should have rejected too-long key.") + test.AssertError(t, GoodKey(&private.PublicKey, maxKeySize), "Should have rejected too-long key.") + test.AssertError(t, GoodKey(private.PublicKey, maxKeySize), "Should have rejected too-long key.") } func TestSmallExponent(t *testing.T) { @@ -48,7 +48,7 @@ func TestSmallExponent(t *testing.T) { N: bigOne.Lsh(bigOne, 2048), E: 5, } - test.Assert(t, !GoodKey(&key, maxKeySize), "Should have rejected small exponent.") + test.AssertError(t, GoodKey(&key, maxKeySize), "Should have rejected small exponent.") } func TestEvenExponent(t *testing.T) { @@ -57,7 +57,7 @@ func TestEvenExponent(t *testing.T) { N: bigOne.Lsh(bigOne, 2048), E: 1 << 17, } - test.Assert(t, !GoodKey(&key, maxKeySize), "Should have rejected even exponent.") + test.AssertError(t, GoodKey(&key, maxKeySize), "Should have rejected even exponent.") } func TestEvenModulus(t *testing.T) { @@ -66,7 +66,7 @@ func TestEvenModulus(t *testing.T) { N: bigOne.Lsh(bigOne, 2048), E: (1 << 17) + 1, } - test.Assert(t, !GoodKey(&key, maxKeySize), "Should have rejected even modulus.") + test.AssertError(t, GoodKey(&key, maxKeySize), "Should have rejected even modulus.") } func TestModulusDivisibleBy752(t *testing.T) { @@ -78,11 +78,11 @@ func TestModulusDivisibleBy752(t *testing.T) { N: N, E: (1 << 17) + 1, } - test.Assert(t, !GoodKey(&key, maxKeySize), "Should have rejected modulus divisible by 751.") + test.AssertError(t, GoodKey(&key, maxKeySize), "Should have rejected modulus divisible by 751.") } func TestGoodKey(t *testing.T) { private, err := rsa.GenerateKey(rand.Reader, 2048) test.AssertNotError(t, err, "Error generating key") - test.Assert(t, GoodKey(&private.PublicKey, maxKeySize), "Should have accepted good key.") + test.AssertNotError(t, GoodKey(&private.PublicKey, maxKeySize), "Should have accepted good key.") } diff --git a/ra/registration-authority.go b/ra/registration-authority.go index afb5822b5..01f22d545 100644 --- a/ra/registration-authority.go +++ b/ra/registration-authority.go @@ -88,8 +88,8 @@ type certificateRequestEvent struct { } func (ra *RegistrationAuthorityImpl) NewRegistration(init core.Registration) (reg core.Registration, err error) { - if !core.GoodKey(init.Key.Key, ra.MaxKeySize) { - return core.Registration{}, core.UnauthorizedError("Invalid public key") + if err = core.GoodKey(init.Key.Key, ra.MaxKeySize); err != nil { + return core.Registration{}, core.MalformedRequestError(fmt.Sprintf("Invalid public key: %s", err.Error())) } reg = core.Registration{ RecoveryToken: core.NewToken(),