Bug #8575
closedCrash in openssl verify_certificate_identity
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 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-----
Updated by MartinBosslet (Martin Bosslet) over 11 years ago
- 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!
Updated by nagachika (Tomoyuki Chikanaga) over 11 years 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?
Updated by bascule (Tony Arcieri) over 11 years 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:
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.
Updated by usa (Usaku NAKAMURA) over 11 years ago
- Backport changed from 1.9.3: UNKNOWN, 2.0.0: REQUIRED to 1.9.3: REQUIRED, 2.0.0: REQUIRED
Updated by nahi (Hiroshi Nakamura) over 11 years ago
emboss: Here's my patch. https://gist.github.com/nahi/5934959
Updated by jeremyevans0 (Jeremy Evans) over 11 years 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.
Updated by MartinBosslet (Martin Bosslet) over 11 years 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.
Updated by Anonymous over 11 years 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, 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!
Updated by nagachika (Tomoyuki Chikanaga) over 11 years 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 ruby_2_0_0 branch at r41812.
Updated by usa (Usaku NAKAMURA) about 11 years 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.