pushpad / web-push

Web Push library for Ruby (RFC8030) - A fork of zaru/webpush actively maintained by Pushpad with many improvements, bug fixes and updates.
MIT License
139 stars 13 forks source link

flakey test #4

Open firien opened 1 year ago

firien commented 1 year ago

https://github.com/pushpad/web-push/blob/20f40d9586b524b8c75e64887683931d6e11b078/spec/web_push/vapid_key_spec.rb#L19

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 commented 1 year ago

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 commented 1 year ago

@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 commented 8 months ago

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.