Bug #8575

Crash in openssl verify_certificate_identity

Added by Maximilian Szengel 10 months ago. Updated 9 months ago.

[ruby-core:55685]
Status:Closed
Priority:High
Assignee:Martin Bosslet
Category:ext/openssl
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

Description

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 verifycertificateidentity'
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
verifycertificateidentity'
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-----

Associated revisions

Revision 41805
Added by emboss 10 months ago

  • 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, octetstring] if critical,
    [id, octet
    string] if not.

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

    Thank you @nahi for providing the patch!

History

#1 Updated by Martin Bosslet 10 months ago

  • Category set to ext/openssl
  • Status changed from Open to Assigned
  • Assignee set to 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!

#2 Updated by Tomoyuki Chikanaga 10 months ago

  • 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?

#3 Updated by Tony Arcieri 10 months ago

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.

#4 Updated by Usaku NAKAMURA 10 months ago

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

#5 Updated by Hiroshi Nakamura 10 months ago

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

#6 Updated by Jeremy Evans 10 months ago

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.

#7 Updated by Martin Bosslet 10 months ago

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.

#8 Updated by Anonymous 10 months ago

  • 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, octetstring] if critical,
    [id, octet
    string] if not.

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

    Thank you @nahi for providing the patch!

#9 Updated by Tomoyuki Chikanaga 10 months ago

  • 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 ruby20_0 branch at r41812.

#10 Updated by Usaku NAKAMURA 9 months ago

  • 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.

Also available in: Atom PDF