diff --git a/CHANGELOG.md b/CHANGELOG.md index 388d6742..07c2fb66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ **Features:** - Add support for x5t header parameter for X.509 certificate thumbprint verification [#669](https://github.com/jwt/ruby-jwt/pull/669) ([@hieuk09](https://github.com/hieuk09)) +- Raise an error if the ECDSA signing or verification key is not an instance of `OpenSSL::PKey::EC` [#688](https://github.com/jwt/ruby-jwt/pull/688) ([@anakinj](https://github.com/anakinj)) - Your contribution here **Fixes and enhancements:** diff --git a/lib/jwt/jwa/ecdsa.rb b/lib/jwt/jwa/ecdsa.rb index e011ff44..44004bd5 100644 --- a/lib/jwt/jwa/ecdsa.rb +++ b/lib/jwt/jwa/ecdsa.rb @@ -12,14 +12,19 @@ def initialize(alg, digest) end def sign(data:, signing_key:) + raise_sign_error!("The given key is a #{signing_key.class}. It has to be an OpenSSL::PKey::EC instance.") unless signing_key.is_a?(::OpenSSL::PKey::EC) + curve_definition = curve_by_name(signing_key.group.curve_name) key_algorithm = curve_definition[:algorithm] + raise IncorrectAlgorithm, "payload algorithm is #{alg} but #{key_algorithm} signing key was provided" if alg != key_algorithm asn1_to_raw(signing_key.dsa_sign_asn1(digest.digest(data)), signing_key) end def verify(data:, signature:, verification_key:) + raise_verify_error!("The given key is a #{verification_key.class}. It has to be an OpenSSL::PKey::EC instance.") unless verification_key.is_a?(::OpenSSL::PKey::EC) + curve_definition = curve_by_name(verification_key.group.curve_name) key_algorithm = curve_definition[:algorithm] raise IncorrectAlgorithm, "payload algorithm is #{alg} but #{key_algorithm} verification key was provided" if alg != key_algorithm diff --git a/spec/jwt/jwa/ecdsa_spec.rb b/spec/jwt/jwa/ecdsa_spec.rb index fa899ac5..370770c5 100644 --- a/spec/jwt/jwa/ecdsa_spec.rb +++ b/spec/jwt/jwa/ecdsa_spec.rb @@ -55,5 +55,40 @@ end.to raise_error(JWT::VerificationError, 'Signature verification raised') end end + + context 'when the verification key is not an OpenSSL::PKey::EC instance' do + it 'raises a JWT::DecodeError' do + expect do + instance.verify(data: data, signature: '', verification_key: 'not_a_key') + end.to raise_error(JWT::DecodeError, 'The given key is a String. It has to be an OpenSSL::PKey::EC instance.') + end + end + end + + describe '#sign' do + context 'when the signing key is valid' do + it 'returns a valid signature' do + signature = instance.sign(data: data, signing_key: ecdsa_key) + expect(signature).to be_a(String) + expect(signature.length).to be > 0 + end + end + + context 'when the signing key is not an OpenSSL::PKey::EC instance' do + it 'raises a JWT::DecodeError' do + expect do + instance.sign(data: data, signing_key: 'not_a_key') + end.to raise_error(JWT::EncodeError, 'The given key is a String. It has to be an OpenSSL::PKey::EC instance.') + end + end + + context 'when the signing key is invalid' do + it 'raises a JWT::DecodeError' do + invalid_key = OpenSSL::PKey::EC.generate('sect571r1') + expect do + instance.sign(data: data, signing_key: invalid_key) + end.to raise_error(JWT::DecodeError, "The ECDSA curve 'sect571r1' is not supported") + end + end end end diff --git a/spec/jwt/jwk/decode_with_jwk_spec.rb b/spec/jwt/jwk/decode_with_jwk_spec.rb index 9e657d57..f7ee88f2 100644 --- a/spec/jwt/jwk/decode_with_jwk_spec.rb +++ b/spec/jwt/jwk/decode_with_jwk_spec.rb @@ -169,7 +169,7 @@ it 'fails in some way' do expect { described_class.decode(signed_token, nil, true, algorithms: ['ES384'], jwks: jwks) }.to( - raise_error(NoMethodError, /undefined method .*group/) + raise_error(JWT::DecodeError, 'The given key is a String. It has to be an OpenSSL::PKey::EC instance.') ) end end diff --git a/spec/jwt/jwk/ec_spec.rb b/spec/jwt/jwk/ec_spec.rb index a469dae9..147c9591 100644 --- a/spec/jwt/jwk/ec_spec.rb +++ b/spec/jwt/jwk/ec_spec.rb @@ -110,6 +110,22 @@ end end + describe '.to_openssl_curve' do + context 'when a valid curve name is given' do + it 'returns the corresponding OpenSSL curve name' do + expect(JWT::JWK::EC.to_openssl_curve('P-256')).to eq('prime256v1') + expect(JWT::JWK::EC.to_openssl_curve('P-384')).to eq('secp384r1') + expect(JWT::JWK::EC.to_openssl_curve('P-521')).to eq('secp521r1') + expect(JWT::JWK::EC.to_openssl_curve('P-256K')).to eq('secp256k1') + end + end + context 'when an invalid curve name is given' do + it 'raises an error' do + expect { JWT::JWK::EC.to_openssl_curve('invalid-curve') }.to raise_error(JWT::JWKError, 'Invalid curve provided') + end + end + end + describe '.import' do subject { described_class.import(params) } let(:include_private) { false } diff --git a/spec/jwt/jwt_spec.rb b/spec/jwt/jwt_spec.rb index ce258cd0..1bc14e0e 100644 --- a/spec/jwt/jwt_spec.rb +++ b/spec/jwt/jwt_spec.rb @@ -39,10 +39,6 @@ } end - after(:each) do - expect(OpenSSL.errors).to be_empty - end - context 'alg: NONE' do let(:alg) { 'none' } let(:encoded_token) { data['NONE'] }