Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support exporting RSA private keys #373

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions README.md
Expand Up @@ -486,6 +486,17 @@ jwks = { keys: [{ ... }] } # keys needs to be Symbol
JWT.decode(token, nil, true, { algorithms: ['RS512'], jwks: jwks})
```

### Importing and exporting JSON Web Keys

The ::JWT::JWK class can be used to import and export both the public key (default behaviour) and the private key. To include the private key in the export pass the `include_private` parameter to the export method.

```ruby
jwk = JWT::JWK.new(OpenSSL::PKey::RSA.new(2048))

jwk_hash = jwk.export
jwk_hash_with_private_key = jwk.export(include_private: true)
```

# Development and Tests

We depend on [Bundler](http://rubygems.org/gems/bundler) for defining gemspec and performing releases to rubygems.org, which can be done with
Expand Down
71 changes: 47 additions & 24 deletions lib/jwt/jwk/rsa.rb
Expand Up @@ -28,45 +28,68 @@ def kid
OpenSSL::Digest::SHA256.hexdigest(sequence.to_der)
end

def export
{
def export(options = {})

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assignment Branch Condition size for export is too high. [28.04/20]

ret = {
kty: KTY,
n: encode_open_ssl_bn(public_key.n),
e: encode_open_ssl_bn(public_key.e),
kid: kid
}

return ret if options[:include_private] != true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also check if the keypair has a private key.

return ret unless private? && options[:include_private] == true

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Will get back to this when I have the time.


ret.merge(
d: encode_open_ssl_bn(keypair.d),
p: encode_open_ssl_bn(keypair.p),
q: encode_open_ssl_bn(keypair.q),
dp: encode_open_ssl_bn(keypair.dmp1),
dq: encode_open_ssl_bn(keypair.dmq1),
qi: encode_open_ssl_bn(keypair.iqmp)
)
end

def encode_open_ssl_bn(key_part)
Copy link
Contributor

@phlegx phlegx Sep 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this method private. Also method kid. So, we can expose the same set of methods in all other JWKs.

::Base64.urlsafe_encode64(key_part.to_s(BINARY), padding: false)
end

def self.import(jwk_data)
jwk_n = jwk_data[:n] || jwk_data['n']
jwk_e = jwk_data[:e] || jwk_data['e']

raise JWT::JWKError, 'Key format is invalid for RSA' unless jwk_n && jwk_e

self.new(rsa_pkey(jwk_n, jwk_e))
end

def self.rsa_pkey(jwk_n, jwk_e)
key = OpenSSL::PKey::RSA.new
key_n = decode_open_ssl_bn(jwk_n)
key_e = decode_open_ssl_bn(jwk_e)
class << self
def import(jwk_data)
self.new(rsa_pkey(*jwk_attrs(jwk_data, :n, :e, :d, :p, :q, :dp, :dq, :qi)))
end

if key.respond_to?(:set_key)
key.set_key(key_n, key_e, nil)
else
key.n = key_n
key.e = key_e
def jwk_attrs(jwk_data, *attrs)
attrs.map do |attr|
decode_open_ssl_bn(jwk_data[attr] || jwk_data[attr.to_s])
end
end

key
end
def rsa_pkey(jwk_n, jwk_e, jwk_d, jwk_p, jwk_q, jwk_dp, jwk_dq, jwk_qi)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perceived complexity for rsa_pkey is too high. [16/7]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid parameter lists longer than 5 parameters. [8/5]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method has too many lines. [17/15]

You might want to extract some of this method lines into new methods inside its current class or isolate it on a class of its own and make it a collaborator instead.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cyclomatic complexity for rsa_pkey is too high. [15/6]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JWT::JWK::RSA#rsa_pkey has approx 14 statements

Read more about it here.

raise JWT::JWKError, 'Key format is invalid for RSA' unless jwk_n && jwk_e

key = OpenSSL::PKey::RSA.new

if key.respond_to?(:set_key)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JWT::JWK::RSA#rsa_pkey manually dispatches method call

key.set_key(jwk_n, jwk_e, jwk_d)
key.set_factors(jwk_p, jwk_q) if jwk_p && jwk_q
key.set_crt_params(jwk_dp, jwk_dq, jwk_qi) if jwk_dp && jwk_dq && jwk_qi
else
key.n = jwk_n
key.e = jwk_e
key.d = jwk_d if jwk_d
key.p = jwk_p if jwk_p
key.q = jwk_q if jwk_q
key.dmp1 = jwk_dp if jwk_dp
key.dmq1 = jwk_dq if jwk_dq
key.iqmp = jwk_qi if jwk_qi
end

key
end

def self.decode_open_ssl_bn(jwk_data)
OpenSSL::BN.new(::Base64.urlsafe_decode64(jwk_data), BINARY)
def decode_open_ssl_bn(jwk_data)
return nil if jwk_data.nil?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JWT::JWK::RSA#decode_open_ssl_bn performs a nil-check

Read more about it here.

OpenSSL::BN.new(::Base64.urlsafe_decode64(jwk_data), BINARY)
end
end
end
end
Expand Down
22 changes: 20 additions & 2 deletions spec/jwk/rsa_spec.rb
Expand Up @@ -34,7 +34,7 @@
it 'returns a hash with the public parts of the key' do
expect(subject).to be_a Hash
expect(subject).to include(:kty, :n, :e, :kid)
expect(subject).not_to include(:d)
expect(subject).not_to include(:d, :p, :dp, :dq,:qi)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: whitespace between :dq and :qi.

end
end

Expand All @@ -43,7 +43,7 @@
it 'returns a hash with the public parts of the key' do
expect(subject).to be_a Hash
expect(subject).to include(:kty, :n, :e, :kid)
expect(subject).not_to include(:d)
expect(subject).not_to include(:d, :p, :dp, :dq,:qi)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: whitespace again.

end
end

Expand All @@ -53,6 +53,15 @@
expect { subject }.to raise_error(ArgumentError, 'keypair must be of type OpenSSL::PKey::RSA')
end
end

context 'when private key is requested' do
subject { described_class.new(keypair).export(include_private: true) }
let(:keypair) { rsa_key }
it 'returns a hash with the public AND private parts of the key' do
expect(subject).to be_a Hash
expect(subject).to include(:kty, :n, :e, :kid, :d, :p, :q, :dp, :dq,:qi)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: also whitespace.

end
end
end

describe '.import' do
Expand All @@ -77,6 +86,15 @@
end
end

context 'when private key is included in the data' do
let(:exported_key) { described_class.new(rsa_key).export(include_private: true) }
let(:params) { exported_key }
it 'creates a complete keypair' do
expect(subject).to be_a described_class
expect(subject.private?).to eq true
end
end

context 'when jwk_data is given without e and/or n' do
let(:params) { { kty: "RSA" } }
it 'raises an error' do
Expand Down