clearhaus / pedicel

Handle Apple Pay payment tokens.
MIT License
6 stars 3 forks source link

Specs randomly fails #4

Closed ct-clearhaus closed 6 years ago

ct-clearhaus commented 6 years ago
# bundle exec rspec -I lib/ -I lib/pedicel -r json -r pedicel spec/some_spec.rb
..

Finished in 0.0173 seconds (files took 0.13308 seconds to load)
2 examples, 0 failures
# bundle exec rspec -I lib/ -I lib/pedicel -r json -r pedicel spec/some_spec.rb
.F

Failures:

  1) basic test works
     Failure/Error: raise SignatureError, 'signature does not match the message'

     Pedicel::SignatureError:
       signature does not match the message
     # ./lib/pedicel/ec.rb:164:in `validate_signature'
     # ./lib/pedicel/base.rb:134:in `verify_signature'
     # ./lib/pedicel/ec.rb:27:in `decrypt'
     # ./spec/some_spec.rb:51:in `block (2 levels) in <top (required)>'

Finished in 0.01971 seconds (files took 0.14109 seconds to load)
2 examples, 1 failure

Failed examples:

rspec ./spec/some_spec.rb:25 # basic test works
ct-clearhaus commented 6 years ago

Both when it succeeds and when it fails, $message == message and $s.to_pem == signature.to_pem (when pry'ing into https://github.com/clearhaus/pedicel/blob/master/lib/pedicel/ec.rb#L163).

😕

ct-clearhaus commented 6 years ago
require 'openssl'

key = OpenSSL::PKey::EC.new('prime256v1')
key.generate_key

public_key = OpenSSL::PKey::EC.new('prime256v1')
public_key.public_key = key.public_key

cert = OpenSSL::X509::Certificate.new
cert.serial = 1
cert.version = 2
cert.not_before = Time.now-365*24*60*60
cert.not_after = Time.now+365*24*60*60
cert.subject = OpenSSL::X509::Name.parse('/CN=foobar/')
cert.public_key = public_key

ef = OpenSSL::X509::ExtensionFactory.new
ef.subject_certificate = cert
cert.add_extension(ef.create_extension('keyUsage', 'digitalSignature', true))

flags = \
  OpenSSL::PKCS7::NOCHAIN  | # Ignore certs in the message.
  OpenSSL::PKCS7::NOINTERN | # Only look at the supplied certificate.
  OpenSSL::PKCS7::NOVERIFY   # Do not verify the chain; already done.

puts 1.upto(1000).map { |i|
  message = Random.new.bytes(100)

  signature = OpenSSL::PKCS7.sign(cert, key, message)

  !signature.verify([cert], OpenSSL::X509::Store.new, message, flags)
}.count{|x| x}
$ ruby spec/minex.rb
314

What the heck is wrong?

ct-clearhaus commented 6 years ago
h = 1.upto(1000).map { |i|
  message = Random.new.bytes(1)

  signature = OpenSSL::PKCS7.sign(cert, key, message)

  verified = signature.verify([cert], OpenSSL::X509::Store.new, message, flags)

  [message, verified]
}.group_by{|m,v| v}

puts 'Good'
puts h[true].map{|m,v| m}.sort.uniq.map(&:ord).join(',')

puts
puts 'Bad'
puts h[false].map{|m,v| m}.sort.uniq.map(&:ord).join(',')
$ ruby spec/minex.rb
Good
0,1,2,3,4,5,6,7,8,9,11,12,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100,101,102,103,104,105,106,107,108,109,110,111,112,113,114,115,116,117,118,119,120,121,122,123,124,125,126,127,128,129,130,131,132,133,134,135,136,137,138,139,140,141,142,143,144,145,146,147,148,149,150,151,152,153,154,155,156,157,158,159,160,161,162,163,164,165,166,167,168,169,170,171,172,173,174,175,176,177,178,179,181,182,183,184,185,186,187,188,189,190,191,192,193,194,195,196,197,198,199,200,201,203,204,205,206,207,208,209,210,211,212,213,214,215,216,217,218,219,220,221,222,223,224,225,226,227,228,229,230,231,232,233,234,235,236,237,238,239,240,241,242,243,244,245,246,247,248,249,250,251,252,253,254,255

Bad
10,13

Byte 10 is \n and byte 13 is \r.

mt-clearhaus commented 6 years ago

The smoking gun:

Normally the supplied content is translated into MIME canonical format (as required by the S/MIME specifications) if PKCS7_BINARY is set no translation occurs. This option should be used if the supplied data is in binary format otherwise the translation will corrupt it.

From https://wiki.openssl.org/index.php/Manual:PKCS7_sign(3)

ct-clearhaus commented 6 years ago

Thanks for all the help @mt-clearhaus @kse-clearhaus @ad-clearhaus.

The magic lies in OpenSSL::PKCS7.sign(cert, key, message, [], OpenSSL::PKCS7::BINARY) 🙂

mt-clearhaus commented 6 years ago

See also https://github.com/emboss/ruby-openssl/blob/5c25974827df21723b3b620809995b20376c1db0/test/openssl/test_pkcs7.rb#L56-L58

ct-clearhaus commented 6 years ago

Strangely enough, the signature of "\r" matches verifying "", whereas neither the empty string nor any byte matches the signature of "\n".

I'm quite ready if somebody can explain to me why 🙂

ct-clearhaus commented 6 years ago

Oh yes,

> message = "\n"
> signature.verify([cert], OpenSSL::X509::Store.new, "\r\n", flags)
# => true
ct-clearhaus commented 6 years ago

For completeness, the latest link @mt-clearhaus points to:

# Normaly OpenSSL tries to translate the supplied content into canonical
# MIME format (e.g. a newline character is converted into CR+LF).
# If the content is a binary, PKCS7::BINARY flag should be used.