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.

Sign up for GitHub

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

Jump to bottom

TypeError: nil is not a string when trying to write smime. #132

Closed
mungler opened this issue May 3, 2017 · 11 comments
Closed

TypeError: nil is not a string when trying to write smime. #132

mungler opened this issue May 3, 2017 · 11 comments
Milestone
0.9.21

Comments

@mungler
Copy link

mungler commented May 3, 2017 •
edited by headius
Loading

Environment

Re: https://twitter.com/headius/status/859532672611356672

macOS Sierra 10.12.3 on iMac 27'' 5K

Darwin Eddie.mungler.home 16.4.0 Darwin Kernel Version 16.4.0: Thu Dec 22 22:53:21 PST 2016; root:xnu-3789.41.3~3/RELEASE_X86_64 x86_64

jruby 9.1.8.0 (2.3.1) 2017-03-06 90fc7ab Java HotSpot(TM) 64-Bit Server VM 25.40-b23 on 1.8.0_40-ea-b20 +jit [darwin-x86_64]
JRUBY_OPTS=-J-Xmn512m -J-Xms4096m -J-Xmx4096m -J-server

Gems included by the bundle:

  • activesupport (3.0.0)
  • asw-cache (0.2.0)
  • asw-services (0.0.55 399297c)
  • asw_database (1.0.1)
  • asw_models (0.2.10 5f042c2)
  • bond (0.5.1)
  • bundler (1.14.6)
  • chronic (0.10.2)
  • commander (4.4.3)
  • concurrent-ruby (1.0.5)
  • diff-lcs (1.3)
  • dry-configurable (0.7.0)
  • dry-container (0.6.0)
  • dry-core (0.2.4)
  • dry-equalizer (0.2.0)
  • dry-logic (0.4.1)
  • dry-struct (0.2.1)
  • dry-types (0.10.2)
  • faraday (0.12.1)
  • fotc (0.2.1)
  • gcm (0.1.1)
  • grocer (0.7.1)
  • highline (1.7.8)
  • httparty (0.14.0)
  • ice_nine (0.11.2)
  • inflecto (0.0.2)
  • jdbc-mysql (5.1.40)
  • jedis_rb (2.7.2)
  • jruby-openssl (0.9.20)
  • json (2.1.0)
  • msgpack (1.1.0)
  • multi_xml (0.6.0)
  • multipart-post (2.0.0)
  • nelson-rpc (0.0.24)
  • passbook (0.4.4)
  • puma (3.8.2)
  • rack (1.6.5)
  • rack-test (0.6.3)
  • rake (10.5.0)
  • require_all (1.4.0)
  • ripl (0.7.1)
  • ripl-color_error (0.1.1)
  • ripl-color_streams (0.1.2)
  • ripl-ripper (0.1.0)
  • rspec (3.5.0)
  • rspec-core (3.5.4)
  • rspec-expectations (3.5.0)
  • rspec-mocks (3.5.0)
  • rspec-support (3.5.0)
  • rubyzip (1.2.1)
  • sequel (4.46.0)
  • sequel_polymorphic_asw (0.2.3)
  • terminal-table (1.7.3)
  • unicode-display_width (1.1.3)
  • whenever (0.9.7)
>> p12 = OpenSSL::PKCS12.new File.read(certificate), password
=> #<OpenSSL::PKCS12:0x2e62e227 @key=#<OpenSSL::PKey::RSA:0x5ebe903a>, @ca_certs=[], @der="0\x82\x19\x18\x02\x01\x030\x82\x18\xDF\x06\t*\x86H\x86\xF7\r\x01\a\x01\xA0\x82\x18\xD0\x04\x82\x18\xCC0\x82\x18\xC80\x82\r\xFF\x06\t*\x86H\x86\xF7\r\x01\a\x06\xA0\x82\r\xF00\x82\r\xEC\x02\x01\x000\x82\r\xE5\x06\t..... etc etc 

>> p12.key.to_s
=> "-----BEGIN RSA PRIVATE KEY-----\nMIIEdqpn1xRuIJyx\n6oklTnT4y8pDPhlnqX8BBJmURik8VUSF\ntFeKzfLRIh8GxoO6Yfdki2ESffh6ynxmv+c8PuMrhHd3zKow6eeSgXmFqjCIQu3okiVUkB8owBvJr9r/QZkKa3ZGlQiZz/XVTdzpaUSh7YzeZuzlI9ipj9lElrtsb\n0n7RFED50uW0SxTFtxTFGv4Y/uSxvvd6/uPw5TfxINfr5JDDsqQ9zqg8KxBZxHJsaqKN5RtmzhmYu6wXKk/TT4B8sMr\npGss6A...... snip ......9kZahyvHVKd/5bahPiwGuBXEcbfaGNL8sjPr3eRP3KDJf\n-----END RSA PRIVATE KEY-----\n"

>> p12.certificate.to_s
=> "-----BEGIN CERTIFICATE-----\nMIYxCzAJBgNV\nBAYTAlVTMRMwEQYDVQQKDApBcHBsZSBJbmMuMSwwKgYDVQQLDCNBcHBsZSBXb3Js\nZHdpZGUgRGV2ZWxvcGVyIFJlbGF0aW9uczFEMEIGA1UEAww7QXBwbGUgV29ybGR3\na..... snip .....EmZD1K5TM=\n-----END CERTIFICATE-----\n"
>> 

>> wwdc  = OpenSSL::X509::Certificate.new File.read(wwdc)
=> #<OpenSSL::X509::Certificate:0x5d2e65bd subject=/C=US/O=Apple Inc./OU=Apple Worldwide Developer Relations/CN=Apple Worldwide Developer Relations Certification Authority, issuer=/C=US/O=Apple Inc./OU=Apple Certification Authority/CN=Apple Root CA, serial=134752589830791184, not_before=2013-02-07 21:48:47 UTC, not_after=2023-02-07 21:48:47 UTC>

>> data = 'foobarbaz'
=> "foobarbaz"
>> key_hash = {}
=> {}
>> key_hash[:cert] = p12.certificate
=> #<OpenSSL::X509::Certificate:0x75663443 subject=/userId=pass.com.asmallworld.events.ticket/CN=Pass Type ID: pass.com.asmallworld.events.ticket/OU=QY8279AAHT/O=aSmallworld.net/C=US, issuer=/C=US/O=Apple Inc./OU=Apple Worldwide Developer Relations/CN=Apple Worldwide Developer Relations Certification Authority, serial=534939252856400662, not_before=2017-04-26 12:37:42 UTC, not_after=2018-04-26 12:37:42 UTC>
>> key_hash[:key] = p12.key
=> #<OpenSSL::PKey::RSA:0x7cbe3a05>
>>
>> pk7   = OpenSSL::PKCS7.sign key_hash[:cert], key_hash[:key], data.to_s, [wwdc], OpenSSL::PKCS7::BINARY | OpenSSL::PKCS7::DETACHED
=> #<OpenSSL::PKCS7:0x7aa63f50 @data="foobarbaz">

>> data  = OpenSSL::PKCS7.write_smime pk7
TypeError: nil is not a string
    org/jruby/ext/openssl/PKCS7.java:197:in `write_smime'
    (ripl):28:in `<eval>'
    org/jruby/RubyKernel.java:1000:in `eval'
>>

Sorry, had to throw this together quickly as its late, but wanted to get it logged asap.
@mungler
Copy link
Author

mungler commented May 3, 2017

@kares FYI

@mungler
Copy link
Author

mungler commented May 3, 2017

@headius FYI

@mungler
Copy link
Author

mungler commented May 3, 2017

@kares @headius here's a little example app to show the issue. It works fine under MRI:

https://github.com/asmallworldsite/jruby_ssl_issue

@headius
Copy link
Member

headius commented May 3, 2017

Ok thanks. I'll try to have a look today. We are hoping to do a 9.1.9.0 release next week or the week after and having an updated jruby-openssl would be good.

@headius
Copy link
Member

headius commented May 3, 2017

Confirmed on master (9.1.9.0).

@headius
Copy link
Member

headius commented May 3, 2017

Ok, so here's the basic problem:

https://github.com/jruby/jruby-openssl/blob/master/src/main/java/org/jruby/ext/openssl/PKCS7.java#L197

The data in this case is not passed in, so it's nil. When it attempts to write it, we get the nil error.

A workaround for you would be to pass pk7.data as a second parameter to write_smime. That produces the following for your script:

~/projects/jruby_ssl_issue $ git diff
diff --git a/test.rb b/test.rb
index 05ec55d..6f99713 100644
--- a/test.rb
+++ b/test.rb
@@ -8,7 +8,7 @@ wwdc = OpenSSL::X509::Certificate.new File.read('./wwdc_cert.pem')
 
 pk7 = OpenSSL::PKCS7.sign p12.certificate, p12.key, data.to_s, [wwdc], OpenSSL::PKCS7::BINARY | OpenSSL::PKCS7::DETACHED
 
-smime = OpenSSL::PKCS7.write_smime pk7
+smime = OpenSSL::PKCS7.write_smime pk7, pk7.data
 
-puts smime
+p smime
 

~/projects/jruby_ssl_issue $ jruby test.rb 
"MIME-Version: 1.0\nContent-Type: multipart/signed; protocol=\"application/pkcs7-signature\"; micalg=\"sha1\"; boundary=\"----Q0YNRGHBODHTCRQEI5PRQ4NVWCNYZPW2\"\n\nThis is an S/MIME signed message.\n\n------Q0YNRGHBODHTCRQEI5PRQ4NVWCNYZPW2\nfoo bar baz\n------Q0YNRGHBODHTCRQEI5PRQ4NVWCNYZPW2\nContent-Type: application/x-pkcs7-signature; name=smime.p7s\nContent-Transfer-Encoding: base64\nContent-Description: S/MIME Signature\nContent-Disposition: attachment; filename=smime.p7s\n\nMIIIvAYJKoZIhvcNAQcCoIIIrTCCCKkCAQExCTAHBgUrDgMCGjAPBgkqhkiG9w0BBwGgAgQAoIIG\nnzCCAnUwggHeAgkAgK8jBSrQtqowDQYJKoZIhvcNAQEFBQAwfzELMAkGA1UEBhMCR0IxDTALBgNV\nBAgTBEZpZmUxETAPBgNVBAcTCE1hcmtpbmNoMREwDwYDVQQKEwhUZXN0Q29ycDEWMBQGA1UEAxMN\nUm9yeSBTaW5jbGFpcjEjMCEGCSqGSIb3DQEJARYUcm9yeUBhc21hbGx3b3JsZC5uZXQwHhcNMTcw\nNTAzMTE0NzA0WhcNMjcwNTAxMTE0NzA0WjB/MQswCQYDVQQGEwJHQjENMAsGA1UECBMERmlmZTER\nMA8GA1UEBxMITWFya2luY2gxETAPBgNVBAoTCFRlc3RDb3JwMRYwFAYDVQQDEw1Sb3J5IFNpbmNs\nYWlyMSMwIQYJKoZIhvcNAQkBFhRyb3J5QGFzbWFsbHdvcmxkLm5ldDCBnzANBgkqhkiG9w0BAQEF\nAAOBjQAwgYkCgYEAuQpkbyDgNWsUKNgAozWhel0NqOYv3eZpxmuCxfhrkwb/Uhd8Z5uYmNULZOnb\npTMoDoDw8BtijyfibGirlb2rC7vXNM1nNnMA/I6F5UzQPXg5Eu0XiUY840iFPyN6iydVWMbWkMUl\nIeJpzTXaWZnoD8FG3m1fsuOiS/75+CI6IJ0CAwEAATANBgkqhkiG9w0BAQUFAAOBgQBmyn8hqoAw\nUHXMThQegGSv141TGwnGqgBSEW6dzMdIkHbCUP353MKJbUpwIkw04wGn3u0JdB/fjuqOnjM4l84U\nJjkmSbqtGZ1WpZwZwgLFIVQEoE9E8BRLx6a1UWFhDPPXz+vvE1SSNFRtkMSaEaxpZnipvOzZ60xD\nWX6dM3lx8TCCBCIwggMKoAMCAQICCAHevMQ5baAQMA0GCSqGSIb3DQEBBQUAMGIxCzAJBgNVBAYT\nAlVTMRMwEQYDVQQKEwpBcHBsZSBJbmMuMSYwJAYDVQQLEx1BcHBsZSBDZXJ0aWZpY2F0aW9uIEF1\ndGhvcml0eTEWMBQGA1UEAxMNQXBwbGUgUm9vdCBDQTAeFw0xMzAyMDcyMTQ4NDdaFw0yMzAyMDcy\nMTQ4NDdaMIGWMQswCQYDVQQGEwJVUzETMBEGA1UECgwKQXBwbGUgSW5jLjEsMCoGA1UECwwjQXBw\nbGUgV29ybGR3aWRlIERldmVsb3BlciBSZWxhdGlvbnMxRDBCBgNVBAMMO0FwcGxlIFdvcmxkd2lk\nZSBEZXZlbG9wZXIgUmVsYXRpb25zIENlcnRpZmljYXRpb24gQXV0aG9yaXR5MIIBIjANBgkqhkiG\n9w0BAQEFAAOCAQ8AMIIBCgKCAQEAyjhUpstWqsgkOUjpjO7sX7h/JpG8NFN6znxjgGF3ZF6lByO2\nOf5QLRVWWHAtfsRuwUqFPi/w3oQaoVfJr3sY/2r6FRJJFQgZrKrbKjLtlmNoUhU9jIrsv2sYleAD\nrAF9lwVnzg6FlTdq7Qm2rmfNUWSfxlzRvFduZzWAdjakh4FuOI/YKxVOeyXYWr9Og8GN0pPVGnG1\nYJydM05V+RJYDIa4Fg3B5XdFjVBIuist5JSF4ejEncZopbCj/Gd+cLoCWUt3QpE5ufXN4UzvwDtI\njKblIV39amq7pxY1YNLmrfNGKcnow4vpecBqYWcVsvD95Wi8Yl9uz5nd7xtj/pJlqwIDAQABo4Gm\nMIGjMB0GA1UdDgQWBBSIJxcJqbYYYIvs67r2R1nFUlSjtzAPBgNVHRMBAf8EBTADAQH/MB8GA1Ud\nIwQYMBaAFCvQaUeUdgn+9GuNLkCm90dNfwheMC4GA1UdHwQnMCUwI6AhoB+GHWh0dHA6Ly9jcmwu\nYXBwbGUuY29tL3Jvb3QuY3JsMA4GA1UdDwEB/wQEAwIBhjAQBgoqhkiG92NkBgIBBAIFADANBgkq\nhkiG9w0BAQUFAAOCAQEAT8/vWb4s9bJsL4/uE4cy6AU1qG6LfclpDLnZF7x3LNRn4v2abTpZXN+D\nAb2yriphcrGvzcNFMI+jgw3OHUe08ZOKo3SbpMOYcoc7Pq9FC5JUuTK7kBhTawpOELbZHVBsIYAK\niU5XjGtbPD2m/d73DSMdC0omhz+6kZJMpBkSGW1X9XpYh3toiuSGjErr4kkUqqXdVQCprrtLMK7h\noLG8KYDmCXflvjSiAcp/3OIK5ju4u+y6YpXzBWNBgs0POx1MlaTbq/nJlelP5E3nJpmB6bz5tCnS\nAXpm4S6M9iGKxfh44YGuv9OQnamt86/9OBqWZzAcUaVc7HGKgrRsDwwVHzGCAeMwggHfAgEBMIGM\nMH8xCzAJBgNVBAYTAkdCMQ0wCwYDVQQIEwRGaWZlMREwDwYDVQQHEwhNYXJraW5jaDERMA8GA1UE\nChMIVGVzdENvcnAxFjAUBgNVBAMTDVJvcnkgU2luY2xhaXIxIzAhBgkqhkiG9w0BCQEWFHJvcnlA\nYXNtYWxsd29ybGQubmV0AgkAgK8jBSrQtqowBwYFKw4DAhqggbEwGAYJKoZIhvcNAQkDMQsGCSqG\nSIb3DQEHATAcBgkqhkiG9w0BCQUxDxcNMTcwNTAzMjAxMjAxWjAjBgkqhkiG9w0BCQQxFgQUx1Z+\nizniQo44v5ySJqxo3kxn3DkwUgYJKoZIhvcNAQkPMUUwQzAKBggqhkiG9w0DBzAOBggqhkiG9w0D\nAgICAIAwDQYIKoZIhvcNAwICAUAwDQYIKoZIhvcNAwICASgwBwYFKw4DAgcwCwYJKoZIhvcNAQEB\nBIGAqTOcIwhEV/Pi25bqYkriyT69FF3VJdt9FiieZi2Al1Aye/FTGCiuWSa69iqE3i5Pnz1XQhLR\nOdu6fTQeB9vjd+3Qn3dOk2shrJbGaxVBC/+3HS9tM7i+MKX4MUYQDSIlS190mO6Y4wBIJK7LlsBB\nJjptngE3EmfpDjxuSv6/9YE=\n------Q0YNRGHBODHTCRQEI5PRQ4NVWCNYZPW2--\n"

@kares I guess the code should check if data was passed in, and if not extract it from the pkcs7 instance?

@kares
Copy link
Member

kares commented May 3, 2017

@headius sounds about right. great that there's a work-around - was hoping to get more done into next.

@mungler
Copy link
Author

mungler commented May 3, 2017

Excellent, glad there's a work around. Will try to monkey patch the Passbook gem tomorrow. Thanks and good luck with the fix.

headius added a commit to headius/jruby-openssl that referenced this issue May 3, 2017
@headius
If data is not provided, extract it from the PKCS7 instance.
3c92fb8 
Fixes jruby#132.
@headius headius mentioned this issue May 3, 2017
Merged
@headius
Copy link
Member

headius commented May 3, 2017

I pushed #133 with a fix, but no test yet (not sure where it should go).

@mungler
Copy link
Author

mungler commented May 4, 2017

Hi @headius not sure if this is relevant to you, or if it should be raised separately, but there are some differences in the output between JRuby and MRI (tested with 2.1.9):

< This is an S/MIME signed message.
---
> This is an S/MIME signed message

< Content-Type: application/x-pkcs7-signature; name=smime.p7s
---
> Content-Type: application/x-pkcs7-signature; name="smime.p7s"

< Content-Disposition: attachment; filename=smime.p7s
---
> Content-Disposition: attachment; filename="smime.p7s"

This caused another issue with the Passbook gem, which expects the strings in a certain format - note the str_debut and str_end values here: https://github.com/frozon/passbook/blob/master/lib/passbook/signer.rb#L21-L24

I'm not suggesting that the Passbook gem has a particularly robust approach to reading this data, but I wonder if there is scope for getting the output closer to MRI? I've worked around this in my monkey-patch to fix the pk7 data issue, so it's not particularly affecting me, but I thought i'd put it out there :)

@headius
Copy link
Member

headius commented May 4, 2017

Well we definitely try to match text from MRI and/or OpenSSL. If you have a good list of these, it would be useful to open a separate issue and we can try to clean up formatting. You could also try your hand at a PR :-) Use JRuby, gem install ruby-maven, and rake build to get an updated jopenssl.jar and gem.

@headius headius added this to the 0.9.21 milestone May 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.9.21
Development

No branches or pull requests
3 participants
@headius @kares @mungler