Bug #8575: Crash in openssl verify_certificate_identity

• Status: Closed
• Assignee: MartinBosslet (Martin Bosslet)
• Target version: 2.1.0
• ruby -v: ruby 2.0.0p247 (2013-06-27 revision 41674) [x86_64-darwin12.4.0]
• Backport: 1.9.3: DONE, 2.0.0: DONE

[maxsz (Maximilian Szengel)]

When creating an openssl connection to a server with the certificate below, ruby crashes with the following error:

TypeError: no implicit conversion of true into String
from /Users/szengel/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/openssl/ssl.rb:102:in decode' from /Users/szengel/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/openssl/ssl.rb:102:in block in verify_certificate_identity'
from /Users/szengel/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/openssl/ssl.rb:99:in each' from /Users/szengel/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/openssl/ssl.rb:99:in verify_certificate_identity'
from /Users/szengel/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/openssl/ssl.rb:138:in post_connection_check' from /Users/szengel/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/net/http.rb:920:in connect'
from /Users/szengel/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/net/http.rb:862:in do_start' from /Users/szengel/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/net/http.rb:851:in start'
from /Users/szengel/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/net/http.rb:1367:in request' from /Users/szengel/.rvm/gems/ruby-2.0.0-p247/gems/httparty-0.11.0/lib/httparty/request.rb:92:in perform'
from /Users/szengel/.rvm/gems/ruby-2.0.0-p247/gems/httparty-0.11.0/lib/httparty.rb:461:in perform_request' from /Users/szengel/.rvm/gems/ruby-2.0.0-p247/gems/httparty-0.11.0/lib/httparty.rb:398:in get'

This worked fine with ruby 2.0.0-p195

-----BEGIN CERTIFICATE-----
MIID/jCCAuagAwIBAgIEdNlogTALBgkqhkiG9w0BAQswgaUxKDAmBgNVBAMMH2Vx
dWludXggQ2VydGlmaWNhdGlvbiBBdXRob3JpdHkxEzARBgNVBAoMCmVxdWludXgg
QUcxCzAJBgNVBAsMAkNBMRAwDgYDVQQIDAdCYXZhcmlhMQswCQYDVQQGEwJERTEP
MA0GA1UEBwwGTXVuaWNoMScwJQYJKoZIhvcNAQkBFhhjZXJ0aWZpY2F0ZXNAZXF1
aW51eC5uZXQwHhcNMTMwNjE5MTU1NTMyWhcNMTUwNjE5MTU1NTMyWjCBjDEeMBwG
A1UEAwwVZXF1aW51eGlkLmVxdWludXgubmV0MRAwDgYDVQQKDAdlcXVpbnV4MRAw
DgYDVQQIDAdCYXZhcmlhMQswCQYDVQQGEwJERTEPMA0GA1UEBwwGTXVuaWNoMSgw
JgYJKoZIhvcNAQkBFhl0ZWNobmlrLWludGVybkBlcXVpbnV4LmRlMIIBIjANBgkq
hkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAv2R9k6m5eaN5dAPTosO3u0jEwonaO3HB
rKZdpwPYC0hsuUA3dbPAt9oDkn28K5mcfQlajU4V4ypruUHD2M90CeOqQW/fQdck
eBijvfktWv8dHVndzEsPLljWrmV4M8XhMermUpRo/G5Tpn2DQ5w9gCdK4mFz50FX
9DqBKGj2IlMiQFcU9OGeMeqk2AiZ5QegLv8ZympMr7o5Jn+Mp8nQIhemJHpD9PdR
IBBYYjODAUs74yBNMPRpTYvvB4/XRZww6mm+Mvv782KAfNkjymnPaJk1cxwT5Y3b
KFZLfToOxi1uqwuiycCl8ZmrkY02oyX+o+YLvFNj3a+JBKw/I1vktQIDAQABo08w
TTAOBgNVHQ8BAf8EBAMCBaAwFgYDVR0lAQH/BAwwCgYIKwYBBQUHAwEwIwYDVR0R
AQH/BBkwF4IVZXF1aW51eGlkLmVxdWludXgubmV0MA0GCSqGSIb3DQEBCwUAA4IB
AQCo23JidcwKo4Zss65Hv+FlQIWkmVZSR8EhC/NpXmO6w6/H7ZreGWHSEh9e61Wf
TLe+dy1a0vmvrygMsM/M/2fAywOFl1A5DTRHrvpPJKnFbp70c3gQ16a6gYfCnVcf
Lkq7Eh2Lz8FVJeIsmOb7MrgwUs/xn/xFe1jt2iIhBYtsmuMhywsyshYvDrmWVbTX
/kf1fBk0bcZSjEVsgIHJi9pAABD3TPc6sp+YHQEMdRktOcZZM0qreX+wfVTS+3is
lphlnYfPWnvmbYIJGz/HspWqBrf3AThHj7uehVk9/RETU0yisT8mUL8BD9JHTWoz
lasKZP36VZ3YKcUF4MChyVFs
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
MIID5TCCAs2gAwIBAgIBATALBgkqhkiG9w0BAQUwgaUxKDAmBgNVBAMMH2VxdWlu
dXggQ2VydGlmaWNhdGlvbiBBdXRob3JpdHkxEzARBgNVBAoMCmVxdWludXggQUcx
CzAJBgNVBAsMAkNBMRAwDgYDVQQIDAdCYXZhcmlhMQswCQYDVQQGEwJERTEPMA0G
A1UEBwwGTXVuaWNoMScwJQYJKoZIhvcNAQkBFhhjZXJ0aWZpY2F0ZXNAZXF1aW51
eC5uZXQwHhcNMDkwMTE5MTA1MjQxWhcNMTQwMTE4MTA1MjQxWjCBpTEoMCYGA1UE
AwwfZXF1aW51eCBDZXJ0aWZpY2F0aW9uIEF1dGhvcml0eTETMBEGA1UECgwKZXF1
aW51eCBBRzELMAkGA1UECwwCQ0ExEDAOBgNVBAgMB0JhdmFyaWExCzAJBgNVBAYT
AkRFMQ8wDQYDVQQHDAZNdW5pY2gxJzAlBgkqhkiG9w0BCQEWGGNlcnRpZmljYXRl
c0BlcXVpbnV4Lm5ldDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAL2p
j8EPz24gvX8zZsVeb//UHaC+CEKWf0up/QX8gdhkl+3qUMhY2On6rWtBTDVLB2Yl
iaE6vagW19LYh3DWiRfhFNRgqfL+3pJl0gY1zIOPQPoDaiQLV8z26DkTSMFc0eCZ
qJpEOEQRGet4kodRwbGip13/TDjOT0DwZ5sqhKm0T/oat6MrYAZtM51HVccEGKYa
3FvenyRcmaIecllbr9gW6RPooaFaz8HY7m5JfcTJeTtioSxYQMcaWaJUIEpHAqL6
MhvWZ6gFdsZ0Xo3LZTj8Op/sFMkh3RHao1CkQGWBeOLAUTCn4lqrK6E2dA3QLsFu
wCeEqOUzH4hf9YFMkeMCAwEAAaMgMB4wDwYDVR0TAQH/BAUwAwEB/zALBgNVHQ8E
BAMCAoQwDQYJKoZIhvcNAQEFBQADggEBADKMjJ8+LdLqew0dYNiCKXDZOWOATCC1
Qq02OhLFYL2AeqXxjJ1DfgFNyqyhsd9z5shvcZ85It8lG24bqUlszVFGxFcsM6fC
TSV7PDfKuFlR8D0IExjTq2xD6v0txqqk/1hRiQ7GOXsScW1XCNSdnPmPZw4xQ/Mf
MzGzZFkVamE+HWfDVy8TPzJ3MP5u/x3xszExEZ5MSotTECqcd+2u/FhugYmo396X
9L7a4FdgWb/KCVNaDqeSW5dwbEAYi1ChrDKg7Nqzi0gAzAqNgEVIZXbmNZbVETyr
gE4iJ0t4Q5lNpWofp8Lxbky9eUMZHw3lM3IRgCcXr/gYpQ4aNfwhhak=
-----END CERTIFICATE-----

[MartinBosslet (Martin Bosslet)]

• Category set to ext/openssl
• Status changed from Open to Assigned
• Assignee set to MartinBosslet (Martin Bosslet)
• Target version set to 2.1.0

I'll have a look at the certificate tonight. As soon as I know what's causing the problem I'll prepare a commit with tests for the problem. Thank you for notifying us about this and thanks for providing the certificates!

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN to 1.9.3: UNKNOWN, 2.0.0: REQUIRED

Thank you for reporting this problem.

Is this error reproduced with 1.9.3-p448?

[bascule (Tony Arcieri)]

We can confirm this problem exists on all versions of Ruby, including 1.9 and 1.8.

We've also done some more digging into it. The problematic line of code is here:

https://github.com/ruby/ruby/blob/bc47f294ee88630bad65a603225b9486ec1752eb/ext/openssl/lib/openssl/ssl.rb#L101

The problem is that this ASN1 sequence may contain a boolean called "critical" which affects the processing of extensions. So this line also needs to handle the case:

id, critical, ostr = OpenSSL::ASN1.decode(ext.to_der).value

Where critical is an OpenSSL::ASN1::Boolean. Right now this case isn't handled, so the code explodes trying to parse "true" as an OctetString.

[usa (Usaku NAKAMURA)]

• Backport changed from 1.9.3: UNKNOWN, 2.0.0: REQUIRED to 1.9.3: REQUIRED, 2.0.0: REQUIRED

[nahi (Hiroshi Nakamura)]

emboss: Here's my patch. https://gist.github.com/nahi/5934959

[jeremyevans0 (Jeremy Evans)]

I humbly request that the fix be backported to 1.8.7 and a new 1.8.7 patch release be issued. Considering that this regression was reported before 1.8.7 support was dropped, I don't think it's fair to 1.8.7 users not to backport it, especially considering the apparent simplicity of the backport.

[MartinBosslet (Martin Bosslet)]

jeremyevans0 (Jeremy Evans) wrote:

I humbly request that the fix be backported to 1.8.7 and a new 1.8.7 patch release be issued. Considering that this regression was reported before 1.8.7 support was dropped, I don't think it's fair to 1.8.7 users not to backport it, especially considering the apparent simplicity of the backport.

Seems fair to me, but it's ultimately not my call, of course. But I am about to commit nahi's patch to trunk tonight and I have also asked the maintainers how they feel about backporting, to 1.8.7 in particular.

• Status changed from Assigned to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r41805.
Maximilian, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

• lib/openssl/ssl.rb: Fix SSL client connection crash for SAN marked
  critical.
  The patch for CVE-2013-4073 caused SSL crash when a SSL server returns
  the certificate that has critical SAN value. X509 extension could
  include 2 or 3 elements in it:

  [id, criticality, octet_string] if critical,
  [id, octet_string] if not.

  Making sure to pick the last element of X509 extension and use it as
  SAN value.
  [ruby-core:55685] [Bug #8575]

  Thank you @nahi (Hiroshi Nakamura) for providing the patch!

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 1.9.3: REQUIRED, 2.0.0: REQUIRED to 1.9.3: REQUIRED, 2.0.0: DONE

I've backported r41805 into ruby_2_0_0 branch at r41812.

[usa (Usaku NAKAMURA)]

• Backport changed from 1.9.3: REQUIRED, 2.0.0: DONE to 1.9.3: DONE, 2.0.0: DONE

backported to 1.9.3 at r42016.