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

flakey test #4

Open
firien opened this issue Feb 2, 2023 · 3 comments
Open

flakey test #4

firien opened this issue Feb 2, 2023 · 3 comments

Comments

@firien
Copy link
Contributor

firien commented Feb 2, 2023

expect(Base64.urlsafe_decode64(key.private_key).bytesize).to eq(32)

occasionally this is 31 bytes

key = nil
30_000.times do |x|
  key = Webpush::VapidKey.new
  if key.curve.private_key.to_s(2).bytesize < 32
    puts x
    break
  end
end

puts([format("%064x", key.curve.private_key.to_i)].pack('H*').bytes[0...3])
puts '---'
puts(key.curve.private_key.to_s(2).bytes[0...3])
@collimarco
Copy link
Contributor

collimarco commented Feb 2, 2023

Good catch.

You can also reproduce the issue in the tests with this code:

# vapid_key_spec.rb

it 'returns an encoded private key' do
  30_000.times do
    key = WebPush::VapidKey.new
    puts key.private_key
    expect(Base64.urlsafe_decode64(key.private_key).bytesize).to eq(32)
  end
end

However, after further investigation, it seems that the problem relies in Ruby OpenSSL and not in this gem.

For example this will sometimes fail:

30_000.times do
  key = OpenSSL::PKey::EC.generate('prime256v1')
  expect(key.private_key.to_s(2).bytesize).to eq(32)
end

Now the question is... is it a bug in OpenSSL::PKey::EC or is it normal that the private key is only 31 characters sometimes?

@collimarco
Copy link
Contributor

@rossta I see that you are the author of this test, do you have any idea why it fails sometimes (very rare)? What was the meaning of that test on the length?

@rossta
Copy link
Contributor

rossta commented Jan 21, 2024

I seem to recall this as my interpretation of the literature at the time, but I’m not confident this length has to be enforced. It might be worth manually testing the behavior with a shorter key generated from the method shown above.

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

No branches or pull requests

3 participants