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

OpenSSL::PKey::EC::Point::Error: EC_POINT_bn2point: invalid encoding when importing a JWK #412

Closed
fabn opened this issue Apr 13, 2021 · 22 comments · Fixed by #585
Closed

Comments

@fabn
Copy link

fabn commented Apr 13, 2021

Minimal reproducible case:

key_hash = {
  crv: "P-521",
  kid: "6f1cb481-c032-4e46-8f62-fe68c634a125",
  kty: "EC",
  use: "sig",
  x: "AMNQr/q+YGv4GfkEjrXH2N0+hnGes4cCqahJlV39m3aJpqSK+uiAvkRE5SDm2bZBc3YHGzhDzfMTUpnvXwjugUQP",
  y: "fIwouWsnp44Fjh2gBmO8ZafnpXZwLOCoaT5itu/Q4Z6j3duRfqmDsqyxZueDA3Gaac2LkbWGplT7mg4j7vCuGsw="
}
JWT::JWK.import(key_hash)

Getting this error (gem is current master, 2cea14f)

OpenSSL::PKey::EC::Point::Error: EC_POINT_bn2point: invalid encoding
from /Users/fabio/.rvm/gems/ruby-2.5.8/bundler/gems/ruby-jwt-2cea14fdae43/lib/jwt/jwk/ec.rb:129:in `initialize'

My OpenSSL Version

[3] pry(main)> OpenSSL::VERSION
"2.1.2"

I have no idea on how to fix this...

@fabn
Copy link
Author

fabn commented Apr 18, 2021

Any feedback about this? I was able to import that key and verify a token signed with it using Java so I assume this is an actual issue in this library. Am I right?

@excpt
Copy link
Member

excpt commented Apr 20, 2021

Hi @fabn,

I could reproduce the issue.

It looks like x and y cannot be read or be processed by OpenSSL.

I need to take a deeper dive into that one.

Current hot fix work around for you may be:

Change line 131 in lib/jwt/jwk/ec.rb to

            OpenSSL::BN.new([0x04, x_octets, y_octets].pack(''), 2)

@fabn
Copy link
Author

fabn commented Apr 20, 2021

Hi @excpt

I tried your hotfix and, while it solves the issue I reported (for all 3 failing keys in our keyset), it introduces another issue with the export funcionality. Here I have a failing test case (working before code change).

require 'rails_helper'

RSpec.describe 'PEM keys' do
  it 'is working' do
    pem_key = <<~EOT
    -----BEGIN PUBLIC KEY-----
    MIGbMBAGByqGSM49AgEGBSuBBAAjA4GGAAQBww52ETI/+NE6bi7m71mqBPp6/T+0
    dAd+CvhrZXYrXyKmxh88eE/j69+S8orvqeaeAYsYADwyPSjv4HdvWHcMcAgA5mKS
    ovZiRyEUegD1bopm/iSKx6Uh1sNl8Wvn6ZSyvVBIzkfAX0rSkHpunGUrxp02RRay
    Yfp5zkePrdTqsyqeqOQ=
    -----END PUBLIC KEY-----
    EOT

    # Given a PEM encoded key
    original_key = OpenSSL::PKey.read(pem_key)
    # Is an EC key
    expect(original_key).to be_a(OpenSSL::PKey::EC)
    # PEM is equal to source
    expect(original_key.to_pem).to eq(pem_key)
    # When I convert it to JWK
    jwk = JWT::JWK.new(original_key)
    # PEM format is still the same
    expect(jwk.keypair.to_pem).to eq(original_key.to_pem)
    # And reimport it
    imported_key = JWT::JWK.import(jwk.export)
    # After export to JWK => import from JWK public key is still the same
    expect(imported_key.keypair.to_pem).to eq(original_key.to_pem) # <--- This line will raise an exception with suggested changes
  end
end
  1) PEM keys is working
     Failure/Error: expect(imported_key.keypair.to_pem).to eq(original_key.to_pem)

     OpenSSL::PKey::ECError:
       can't export - EC_KEY_check_key failed: point at infinity

The same happens with other public keys I have in PEM format.

@excpt
Copy link
Member

excpt commented Apr 20, 2021

Thanks for detailed feedback and the code samples. This will help finding the exact reason for that behavior on ec key import and export.

@fabn
Copy link
Author

fabn commented May 17, 2021

Sorry to bother. Do you had any chance to find a workaround for this?

@a0960909060
Copy link

Minimal reproducible case:

key_hash = {

  crv: "P-521",

  kid: "6f1cb481-c032-4e46-8f62-fe68c634a125",

  kty: "EC",

  use: "sig",

  x: "AMNQr/q+YGv4GfkEjrXH2N0+hnGes4cCqahJlV39m3aJpqSK+uiAvkRE5SDm2bZBc3YHGzhDzfMTUpnvXwjugUQP",

  y: "fIwouWsnp44Fjh2gBmO8ZafnpXZwLOCoaT5itu/Q4Z6j3duRfqmDsqyxZueDA3Gaac2LkbWGplT7mg4j7vCuGsw="

}

JWT::JWK.import(key_hash)

Getting this error (gem is current master, 2cea14f)


OpenSSL::PKey::EC::Point::Error: EC_POINT_bn2point: invalid encoding

from /Users/fabio/.rvm/gems/ruby-2.5.8/bundler/gems/ruby-jwt-2cea14fdae43/lib/jwt/jwk/ec.rb:129:in `initialize'

My OpenSSL Version


[3] pry(main)> OpenSSL::VERSION

"2.1.2"

I have no idea on how to fix this...

@a0960909060
Copy link

helpers/build-app-list.js

@a0960909060
Copy link

Minimal reproducible case:

key_hash = {

  crv: "P-521",

  kid: "6f1cb481-c032-4e46-8f62-fe68c634a125",

  kty: "EC",

  use: "sig",

  x: "AMNQr/q+YGv4GfkEjrXH2N0+hnGes4cCqahJlV39m3aJpqSK+uiAvkRE5SDm2bZBc3YHGzhDzfMTUpnvXwjugUQP",

  y: "fIwouWsnp44Fjh2gBmO8ZafnpXZwLOCoaT5itu/Q4Z6j3duRfqmDsqyxZueDA3Gaac2LkbWGplT7mg4j7vCuGsw="

}

JWT::JWK.import(key_hash)

Getting this error (gem is current master, 2cea14f)


OpenSSL::PKey::EC::Point::Error: EC_POINT_bn2point: invalid encoding

from /Users/fabio/.rvm/gems/ruby-2.5.8/bundler/gems/ruby-jwt-2cea14fdae43/lib/jwt/jwk/ec.rb:129:in `initialize'

My OpenSSL Version


[3] pry(main)> OpenSSL::VERSION

"2.1.2"

I have no idea on how to fix this...

@a0960909060
Copy link

Minimal reproducible case:

key_hash = {

  crv: "P-521",

  kid: "6f1cb481-c032-4e46-8f62-fe68c634a125",

  kty: "EC",

  use: "sig",

  x: "AMNQr/q+YGv4GfkEjrXH2N0+hnGes4cCqahJlV39m3aJpqSK+uiAvkRE5SDm2bZBc3YHGzhDzfMTUpnvXwjugUQP",

  y: "fIwouWsnp44Fjh2gBmO8ZafnpXZwLOCoaT5itu/Q4Z6j3duRfqmDsqyxZueDA3Gaac2LkbWGplT7mg4j7vCuGsw="

}

JWT::JWK.import(key_hash)

Getting this error (gem is current master, 2cea14f)


OpenSSL::PKey::EC::Point::Error: EC_POINT_bn2point: invalid encoding

from /Users/fabio/.rvm/gems/ruby-2.5.8/bundler/gems/ruby-jwt-2cea14fdae43/lib/jwt/jwk/ec.rb:129:in `initialize'

My OpenSSL Version


[3] pry(main)> OpenSSL::VERSION

"2.1.2"

I have no idea on how to fix this...

@a0960909060
Copy link

helpers/build-app-list.js

@fabn
Copy link
Author

fabn commented Jul 24, 2023

For the record with OpenSSL 3.1.0 seems to not be an issue anymore

Loading development environment (Rails 7.0.4.3)
[1] pry(main)> OpenSSL::VERSION
"3.1.0"
[2] pry(main)> key_hash = {
  crv: "P-521",
  kid: "6f1cb481-c032-4e46-8f62-fe68c634a125",
  kty: "EC",
  use: "sig",
  x: "AMNQr/q+YGv4GfkEjrXH2N0+hnGes4cCqahJlV39m3aJpqSK+uiAvkRE5SDm2bZBc3YHGzhDzfMTUpnvXwjugUQP",
  y: "fIwouWsnp44Fjh2gBmO8ZafnpXZwLOCoaT5itu/Q4Z6j3duRfqmDsqyxZueDA3Gaac2LkbWGplT7mg4j7vCuGsw="
}
[2] pry(main)> key_hash = {
  crv: "P-521",
  kid: "6f1cb481-c032-4e46-8f62-fe68c634a125",
  kty: "EC",
  use: "sig",
  x: "AMNQr/q+YGv4GfkEjrXH2N0+hnGes4cCqahJlV39m3aJpqSK+uiAvkRE5SDm2bZBc3YHGzhDzfMTUpnvXwjugUQP",
  y: "fIwouWsnp44Fjh2gBmO8ZafnpXZwLOCoaT5itu/Q4Z6j3duRfqmDsqyxZueDA3Gaac2LkbWGplT7mg4j7vCuGsw="
}
#<JWT::JWK::EC:0x000000011298cac0 @parameters={:crv=>"P-521", :kid=>"6f1cb481-c032-4e46-8f62-fe68c634a125", :kty=>"EC", :use=>"sig", :x=>"AMNQr/q+YGv4GfkEjrXH2N0+hnGes4cCqahJlV39m3aJpqSK+uiAvkRE5SDm2bZBc3YHGzhDzfMTUpnvXwjugUQP", :y=>"fIwouWsnp44Fjh2gBmO8ZafnpXZwLOCoaT5itu/Q4Z6j3duRfqmDsqyxZueDA3Gaac2LkbWGplT7mg4j7vCuGsw="}>

I don't know if this can be closed.

@anakinj
Copy link
Member

anakinj commented Jul 25, 2023

Great to hear that it works. I'll close this one.

@anakinj anakinj closed this as completed Jul 25, 2023
@skatkov
Copy link

skatkov commented Sep 6, 2023

According to original steps to reproduce, this is already resolved. But issue still appears, steps to reproduce are slightly different though.

key_hash = {
    crv: "P-521",
    kid: "6f1cb481-c032-4e46-8f62-fe68c634a125",
    kty: "EC",
    use: "sig",
    x: "AMNQr/q+YGv4GfkEjrXH2N0+hnGes4cCqahJlV39m3aJpqSK+uiAvkRE5SDm2bZBc3YHGzhDzfMTUpnvXwjugUQP",
    y: "fIwouWsnp44Fjh2gBmO8ZafnpXZwLOCoaT5itu/Q4Z6j3duRfqmDsqyxZueDA3Gaac2LkbWGplT7mg4j7vCuGsw="
  }
JWT::JWK.import(key_hash).public_key

And error is the following:

/Users/skatkov/.rvm/gems/ruby-3.2.2/gems/jwt-2.7.1/lib/jwt/jwk/ec.rb:196:in `initialize': EC_POINT_bn2point: invalid encoding (OpenSSL::PKey::EC::Point::Error)
	from /Users/skatkov/.rvm/gems/ruby-3.2.2/gems/jwt-2.7.1/lib/jwt/jwk/ec.rb:196:in `new'
	from /Users/skatkov/.rvm/gems/ruby-3.2.2/gems/jwt-2.7.1/lib/jwt/jwk/ec.rb:196:in `create_ec_key'
	from /Users/skatkov/.rvm/gems/ruby-3.2.2/gems/jwt-2.7.1/lib/jwt/jwk/ec.rb:77:in `ec_key'
	from /Users/skatkov/.rvm/gems/ruby-3.2.2/gems/jwt-2.7.1/lib/jwt/jwk/ec.rb:46:in `public_key'

I suggest to re-open this ticket to finally address these.

@fabn
Copy link
Author

fabn commented Sep 6, 2023

I'm not able to reopen it, but I confirm that it happens also in my setup, so @anakinj I think it should be reopened unfortunately

@excpt excpt reopened this Sep 6, 2023
@julik
Copy link
Contributor

julik commented Jan 27, 2024

@anakinj I suspect it has become clearer what is going on here, I've reproduced it on two of our keys which fail to recreate an elliptic curve point correctly. This was just a hunch - I figured base64 encode/decode can play up due to the various forms and pecularities. The hunch turned out to be right, this is just luck 😆

This is likely related to the work you are doing in #582 in the sensed that this might be a relatively common issue. Note that I am not a crypto person and I have no idea what I am doing. Yet, observe:

key_hash = {
  kty: "EC",
  crv: "P-256",
  x: "0Nv5IKAlkvXuAKmOmFgmrwXKR7qGePOzu_7RXg5msw",
  y: "FqnPSNutcjfvXNlufwb7nLJuUEnBkbMdZ3P79nY9c3k"
}

def prepend_byte(base64_encoded_string_from_jwk, byte)
  bytes = Base64.urlsafe_decode64(base64_encoded_string_from_jwk).unpack("C*")
  bytes.unshift(byte)
  Base64.urlsafe_encode64(bytes.pack("C*"))
end

(0..255).each do |byte|
  imported = JWT::JWK.import(key_hash.merge(x: prepend_byte(key_hash.fetch(:x), byte)))
  warn "By prepending byte #{byte} to 'x' the curve the curve could be decoded! - #{imported.inspect}"
rescue OpenSSL::PKey::EC::Point::Error => e
end

key_hash = {
    crv: "P-521",
    kid: "6f1cb481-c032-4e46-8f62-fe68c634a125",
    kty: "EC",
    use: "sig",
    x: "AMNQr/q+YGv4GfkEjrXH2N0+hnGes4cCqahJlV39m3aJpqSK+uiAvkRE5SDm2bZBc3YHGzhDzfMTUpnvXwjugUQP",
    y: "fIwouWsnp44Fjh2gBmO8ZafnpXZwLOCoaT5itu/Q4Z6j3duRfqmDsqyxZueDA3Gaac2LkbWGplT7mg4j7vCuGsw="
  }

(0..255).each do |byte|
  imported = JWT::JWK.import(key_hash.merge(y: prepend_byte(key_hash.fetch(:y), byte))).public_key
  warn "By prepending byte #{byte} to 'y' the curve the curve could be decoded! - #{imported.inspect}"
rescue OpenSSL::PKey::EC::Point::Error => e
end

So: it seems that it is possible that byte 0x00 can get omitted from either the x or the y point coordinate. If that happens, the point will fail to decode - it passes through the base64 decoder alright, and it does get concatenated, but OpenSSL will get two coordinates which are fundamentally of different cardinality. If you check the bytes for the decoded x and y values you might see that they have different dimensions (for example - in the first sample it is 31 bytes for x and 32 bytes for y). I know for sure that this is byte 0 because all the others cause the "point not on curve" error to be raised.

So, it would appear that either:

  • Some base64 encoders (or the encoder we are using in our app, I work together with @skatkov ) may drop byte 0 before doing the base64 encoding, or
  • Base64 decoding done by the gem should account for the possibility that the string may have been 0-prefixed.

@fabn do you happen to have any JSON payloads which lead to this error on your end? and do you know what has encoded those payloads?

@fabn
Copy link
Author

fabn commented Jan 27, 2024

I don't work with that project anymore, but this was happening with keys I posted in the initial post. They've been generated by a spring boot application.

@julik
Copy link
Contributor

julik commented Jan 27, 2024

Ah I see that my second example is actually yours 😆 well, even better then - we know what is happening

@julik
Copy link
Contributor

julik commented Jan 27, 2024

Or this might be a bug in OpenSSL::BN.new 🤣 which would be ever more entertaining tbh

@julik
Copy link
Contributor

julik commented Jan 27, 2024

Here a whole block of known JWKSes from our APM which were causing the issue. This is confirmed fixed with the 0-byte hack above.

 jwk_payloads = [
      "{\"keys\":[{\"kty\":\"EC\",\"crv\":\"P-256\",\"x\":\"0Nv5IKAlkvXuAKmOmFgmrwXKR7qGePOzu_7RXg5msw\",\"y\":\"FqnPSNutcjfvXNlufwb7nLJuUEnBkbMdZ3P79nY9c3k\"}]}",
      "{\"keys\":[{\"kty\":\"EC\",\"crv\":\"P-256\",\"x\":\"xGjPg-7meZamM_yfkGeBUB2eJ5c82Y8vQdXwi5cVGw\",\"y\":\"9FwKAuJacVyEy71yoVn1u1ETsQoiwF7QfkfXURGxg14\"}]}",
      "{\"keys\":[{\"kty\":\"EC\",\"crv\":\"P-256\",\"x\":\"yTvy0bwt5s29mIg1DMq-IjZH4pDgZIN9keEEaSuWZhk\",\"y\":\"a0nrmd8qz8jpZDgpY82Rgv3vZ5xiJuiAoMIuRlGnaw\"}]}",
      "{\"keys\":[{\"kty\":\"EC\",\"crv\":\"P-256\",\"x\":\"yJen7AW4lLUTMH4luDj0wlMNSGCuOBB5R-ZoxlAU_g\",\"y\":\"aMbA-M6ORHePSatiPVz_Pzu7z2XRnKMzK-HIscpfud8\"}]}",
      "{\"keys\":[{\"kty\":\"EC\",\"crv\":\"P-256\",\"x\":\"p_D00Z1ydC7mBIpSKPUUrzVzY9Fr5NMhhGfnf4P9guw\",\"y\":\"lCqM3B_s04uhm7_91oycBvoWzuQWJCbMoZc46uqHXA\"}]}",
      "{\"keys\":[{\"kty\":\"EC\",\"crv\":\"P-256\",\"x\":\"hKS-vxV1bvfZ2xOuHv6Qt3lmHIiArTnhWac31kXw3w\",\"y\":\"f_UWjrTpmq_oTdfss7YJ-9dEiYw_JC90kwAE-y0Yu-w\"}]}",
      "{\"keys\":[{\"kty\":\"EC\",\"crv\":\"P-256\",\"x\":\"3W22hN16OJN1XPpUQuCxtwoBRlf-wGyBNIihQiTmSdI\",\"y\":\"eUaveaPQ4CpyfY7sfCqEF1DCOoxHdMpPHW15BmUF0w\"}]}",
      "{\"keys\":[{\"kty\":\"EC\",\"crv\":\"P-256\",\"x\":\"oq_00cGL3SxUZTA-JvcXALhfQya7elFuC7jcJScN7Bs\",\"y\":\"1nNPIinv_gQiwStfx7vqs7Vt_MSyzoQDy9sCnZlFfg\"}]}"
    ]

@julik
Copy link
Contributor

julik commented Jan 28, 2024

@excpt I've put up a PR if you want to take a look

@anakinj
Copy link
Member

anakinj commented Jan 28, 2024

Nice findings. Got a bit curious if this is intended behaviour on the openssl level. Wonder if this is also a problem when building keys using the openssl 3.0 pkey features (EVP_PKEY_fromdata_settable)

@julik
Copy link
Contributor

julik commented Jan 28, 2024

You could try reproducing by roundtripping the example keys I've added to the test. If they come out truncated - this might as well be a bug in OpenSSL. Now that would be fun!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants