10 ms·

Why did the OpenSSL punycode vulnerability happen?

197 points by stargrave 4y ago

throwaway67743 4y ago
- jiggawatts 4y ago
  Some systems steer people into the pit of failure, others steer people towards the pit of success.
- KyeRussell 4y ago
  The answer to your question is in most if not all arguments against the widespread use of C/C++ in something as high level as OpenSSL. C and C++ ask a lot of developers, mostly in terms of ensuring memory safety. It’s possible to write crappy code in Rust, sure, but the language design has taken the decades of experience of knowing where C falls short and provides a more safe natural path. The long-running rebuttal to this is “it’s as easy as using function x instead of function y!” with the name of function x being some inscrutable derivative of the name of function y. If it’s so easy, why do C codebases keep having the same issues? I’d seriously question the software development experience of someone that doesn’t see the value in reducing developer cognitive load.
- dang 4y ago
  "Eschew flamebait. Avoid generic tangents." https://news.ycombinator.com/newsguidelines.html https://news.ycombinator.com/newsguidelines.html
dang 4y ago
Ongoing related thread: The latest OpenSSL vulns were added fairly recently - https://news.ycombinator.com/item?id=33437158 https://news.ycombinator.com/item?id=33437158 - Nov 2022 (56 comments)
tester756 4y ago
It feels like issues like those are more common in parsers, this specific kind of software. But why? Why is parsing so hard? or is it just in low lvl languages? or maybe languages with poor string primitives? I've written parsers in high level languages and it didnt felt dangerous or insanely hard
- nightfly 4y ago
  Probably low level + safe + performant == hard
  - tester756 4y ago
    But why? where does the complexity come from
    - tux3 4y ago
      There are naturally lots of edge cases when you parse a format, because you have to constrain the combination of all the different fields. Some formats are simple and the fields don't interact with each other at all, some are complex and the format changes depending on other values. Parsing is hard because you have to handle all the possible inputs someone could throw at you, and depending on the format that can leave hundreds of very rare edge case no reasonable human would normally think of. This is also why fuzzing is so effective on parser, fuzzers are great at throwing many different combinations at the wall until they find a new interesting edge case, and jumping off from there to see if they can mutate it into more.
      - ahartmetz 4y ago
        You are of course correct. This also ties into my sibling comment - the cleverness in clever low-level parsing code is based on assumptions that may be wrong for some part of the huge input space.
    - ahartmetz 4y ago
      Parsing code without any kind of framework or high level helpers is, for lack of a better word, fiddly. C strings are an awful tool to do it because memory safety depends on getting a lot of buffer size / string length calculations right - there are plenty of opportunities to make mistakes. The fiddliness also induces developers to make changes in existing code in the form of local clever tricks instead of adapting the code to cleanly implement new requirements. It's only a small change and the tests (hopefully there are any) pass, job done! But maybe it violates a non-obvious assumption for another clever trick somewhere else, etc...
mikepavone 4y ago
> There is a function, ossl_punycode_decode that decodes Punycode. Punycode is a way to encode Unicode as ASCII, used to represent Unicode strings in the ASCII-only world of DNS. ossl_punycode_decode takes an output buffer, and if the buffer runs out it keeps parsing and verifying the Punycode but discards the rest of the output. > I did not have the heart to figure out why it works like this. Maybe there's a good reason to do progressive parsing. Maybe it's an artifact of how C makes you do memory management or of OpenSSL's C style. Anyway. Not involved in OpenSSL, but this is a fairly common pattern in a lot of C APIs. You want to decode some data, but you're not sure how big the output is going to be ahead of time. You could write a separate function to calculate the length, but that function has to do most of the work of actual decoding to figure out that length. A lot of times the output is small enough it can fit in some conservatively sized buffer so you can save a fair bit of work by having a (potentially stack-allocated) buffer of some fixed size and then allocating a precisely sized buffer on the heap if it turns out to not be big enough. Further, having a separate length function means you typically end up with two similar but separate decode implementations which has its own problems. In most other languages, you have some sort of growable container in the standard library so you just avoid the problem entirely at the expense of having less control over memory allocations.
- j-bos 4y ago
  I am by no means a rust developer and asking in ignorance, would this have been a problem had it been written in rust?
  - vore 4y ago
    Rust would take the growable container approach here, so it wouldn't be vulnerable in the same way. There's nothing in C that prevents you from doing this either, but as the parent post mentions it's common style to do this (see also things like snprintf).
    - Elinvynia 4y ago
      > There's nothing in C that prevents you from doing this either Yet these errors consistently happen in C projects of various sizes.
AceJohnny2 4y ago
> As curl author Daniel Stenberg said, "I've never even considered to decode punycode. Why does something like OpenSSL need to decode this?" > [...] > Internationalization is not the issue, internationalization is the job. I want to cheer at this. Internationalization is hard, really hard, especially in a computing world so defined by its english-language dominance. The little amount of effort demonstrated in defining and testing this feature demonstrates that. For all I admire Stenberg's work and focus on quality, that was a rather poor take from him.
- FiloSottile 4y ago
  Oh I don't think Daniel was asking why we're doing i18n. My remark was not at him, at all. I was just pre-empting a possible made up objection. He's in fact correct to wonder why punycode decoding ended up in OpenSSL, as the rest of that section explores. The point being that OpenSSL could do its job and still support i18n domains and emails without having to ever decode punycode if only the spec had made different tradeoffs.
  - AceJohnny2 4y ago
    ah. Thanks for the clarification!
  - tialaramex 4y ago
    Exactly this. I had the very image of this discussion with Python folks for DNSname SANs back when Python was learning not to just rely on CN. Python folks felt like the correct thing was: 1) Implement a Punycode decoder. 2) Decode hostnames and certificate information to get Unicode 3) Compare the Unicode Strings. They were fretting about how complicated it would be to arrange all this, the months of work needed and the need to bring in teams who understood about i18n issues... And I was like No, don't do any of that, take the bytes and compare the bytes. If the bytes aren't identical that's not a match, you are done. It seemed to take a while for it to sink in that this is correct and simpler and thus better.
    - cryptonector 4y ago
MBCook 4y ago
So the blog mentions that in certain cases you have to decode the punycode from one field to compare it to the value in another field. Would it have been safer to encode the data in the other field to punycode and compare the encoded values? That way a hacker can’t mess with your decoder (where bugs like to lie). But at the other end you risk that your encoder has an issue. I do t know how to judge if those are equal risks or not. Thoughts?
- mike_d 4y ago
  Encoding and decoding are both transforms on attacker supplied input. I don't think its helpful to armchair quarterbacking other peoples mistakes that ended up in a vulnerability when the overall code quality is good. There are a dozen ways they could have done it differently, which may or may not have resulted in different exotic bugs. Punycode was the footgun here. Not the language, or the implementation, or the code. They were forced to do something stupid and complex and dangerous deep within the bowels of a critical library.
- moring 4y ago
  If you want to avoid risks both in the encoder and decoder, you might have success with a function that takes an encoded strict and a decoded string and compares them directly, encoding/decoding (whatever works better) on the fly. This avoids the allocation altogether, at the cost of having another less-composable piece of code that must be maintained. I'm not sure if that is the way to go. An allocation per se isn't hard to get right, especially in this case where its scope is well-defined and very limited. What happened here is more the fact that while a single piece of code is easy to get right, a whole codebase isn't. So the only value added by my proposed solution above is that it works in an environment where you can't allocate (e.g. memory-constrained systems where you need to know the required amount of memory statically). Bottom line: I don't think you can really solve this problem on a technical level. edit: typo
deleted 4y ago
[deleted]
bumblebritches5 4y ago
nraynaud 4y ago
About the classification: I would add that the people choosing the severity are a bit alone because of secrecy, so they also can’t really ask too much for advice. That’s probably where the NSA could be useful because of their big number of competent and sworn to secrecy employees, but nobody can trust that the zero day sent for an opinion will not be used to fuck with a foreign country on day one.
ahoog42 4y ago
I decided to review SBOMs from about 3,800 popular mobile apps to see if any included vulnerable versions of OpenSSL v3.0.x. No mobile apps did (not surprised) but what did surprise me was 98% of the OpenSSL versions included in these apps were vulnerable to older CVEs. About 16% of the apps included OpenSSL, mostly as a transitive dependency. I posted additional details in this blog+video: https://www.andrewhoog.com/post/how-to-detect-openssl-v3-and-heartbleed-in-mobile-apps/ https://www.andrewhoog.com/post/how-to-detect-openssl-v3-and...
- cesarb 4y ago
  > No mobile apps did (not surprised) but what did surprise me was 98% of the OpenSSL versions included in these apps were vulnerable to older CVEs. Just the openssl version is not enough, since it could be patched to fix vulnerabilities without increasing the version (this is very common on Linux distributions, which often apply security patches instead of migrating to a new version; for instance, Fedora released a patched 3.0.5 instead of going to 3.0.7). And using an older openssl version does not necessarily mean it's using vulnerable code; according to your blog post, the most common use is SQLCipher, which from a quick look at its README.md seems to use openssl only for the encryption algorithms. Unless the vulnerability was on the basic algorithms used (AES, HMAC, etc), it won't affect this usage.
  - ahoog42 4y ago
    Great points. Static binary analysis looks for the version string but doesn’t currently do deeper analysis of reversed code to see if it’s patched. Could go either way. And determining if the code is triggered and exploitable is quite challenging. Dynamic analysis can help here, provided you have the coverage. More generally tho, istm that there will be instances when the version is unpatched and there is some exploitable vector (even if it’s just crashing the app). My hope is to raise awareness for developers (and security) about 1) transitive dependencies and 2) some really old OpenSSL versions in very popular mobile apps. I don’t believe most folks think about this and awareness can lead to shipping safer apps.
deleted 4y ago
[deleted]
jesprenj 4y ago
Why would a TLS library even parse punycode? The sole reason we still use this hack is so that core infrastructure does not have to change when we use internationalized domains. If TLS libraries, DNS servers, HTTP servers, ... needed to be patched anyways for the use of IDNs, then why did we not do it properly and just use UTF-8? No matter how many vulnerabilities we've introduced into software, "Š" in my name still cannot be represented in domain names and is instead written with american letters xn--pga. I'm sorry for poorly articulating my thoughts here.
- boxy_brown 4y ago
  > Why was this code even necessary > The answer is: an explicit IETF design choice, that made punycode decoding part of X.509 verification, without even a line of acknowledgement in the Security Considerations.
- cryptonector 4y ago
  The idea is that back when IDNA was created sending just-UTF-8 in places that had historically been ASCII-only would break software. The scale of the breakage was probably unknown. The real question is: would a flag day have been needed to just use UTF-8 where previously only ASCII was allowed. I think the answer to that is "no". The punycode thing certainly avoided the question altogether, which is why it was done, but there's no question that had we bitten the bullet then and just gone with just-use-UTF-8 life today would be easier. (There's also Unicode normalization issues here. One of the problems with just-use-UTF-8 is that dumb servers would only match on octet-wise label equality, but with Unicode you want normalization-insensitive matching. Now, at the time the notion of normalization-insensitive matching hadn't been invented... Requiring something like IDNA made it possible to slip in a normalization requirement.)
deleted 4y ago
[deleted]
cryptonector 4y ago
So, punycode I do think was silly. We should just have used UTF-8 in DNS and have left it at that. Using UTF-8 would not have required a flag day. It would have required upgrading some servers in order to be able to have non-ASCII domainnames, but it wouldn't have broken anyone not using non-ASCII domainnames.
- tinus_hn 4y ago
  So it would have broken the whole internet for anyone using a UTF-8 domain name and this system would be adopted by no one. It’s like IPv6, using IPv6 doesn’t break anything for anyone not using it. It’s just that if you use ipv6, you can’t talk to half of the internet including this site.
  - cryptonector 4y ago
    It wouldn't have broken the whole internet though. It's hyperbolic to suggest that it would have.
    - tinus_hn 4y ago
      Yes, it would, because at the start no one would be supporting it so no one would be able to connect. So it would have broken the entire internets ability to connect to you. And remember, as a user, you don’t just use one DNS server that you need to upgrade, you use multiple and they all need to support it and a typical user is not in a position to upgrade it. If you don’t believe that, believe this: if it were as simple as you claim, just add in UTF8, the world wouldn’t have settled on punycode. They didn’t, because it is not actually that simple. It’s so hard, it’s actually simpler to settle for punycode.

197 points by stargrave 4y ago

jiggawatts 4y ago
Some systems steer people into the pit of failure, others steer people towards the pit of success.
KyeRussell 4y ago
The answer to your question is in most if not all arguments against the widespread use of C/C++ in something as high level as OpenSSL. C and C++ ask a lot of developers, mostly in terms of ensuring memory safety. It’s possible to write crappy code in Rust, sure, but the language design has taken the decades of experience of knowing where C falls short and provides a more safe natural path. The long-running rebuttal to this is “it’s as easy as using function x instead of function y!” with the name of function x being some inscrutable derivative of the name of function y. If it’s so easy, why do C codebases keep having the same issues? I’d seriously question the software development experience of someone that doesn’t see the value in reducing developer cognitive load.
dang 4y ago
"Eschew flamebait. Avoid generic tangents." https://news.ycombinator.com/newsguidelines.html https://news.ycombinator.com/newsguidelines.html

dang 4y ago

Ongoing related thread: The latest OpenSSL vulns were added fairly recently - https://news.ycombinator.com/item?id=33437158 https://news.ycombinator.com/item?id=33437158 - Nov 2022 (56 comments)

tester756 4y ago

It feels like issues like those are more common in parsers, this specific kind of software. But why? Why is parsing so hard? or is it just in low lvl languages? or maybe languages with poor string primitives? I've written parsers in high level languages and it didnt felt dangerous or insanely hard

nightfly 4y ago
Probably low level + safe + performant == hard
- tester756 4y ago
  But why? where does the complexity come from
  - tux3 4y ago
    There are naturally lots of edge cases when you parse a format, because you have to constrain the combination of all the different fields. Some formats are simple and the fields don't interact with each other at all, some are complex and the format changes depending on other values. Parsing is hard because you have to handle all the possible inputs someone could throw at you, and depending on the format that can leave hundreds of very rare edge case no reasonable human would normally think of. This is also why fuzzing is so effective on parser, fuzzers are great at throwing many different combinations at the wall until they find a new interesting edge case, and jumping off from there to see if they can mutate it into more.
    - ahartmetz 4y ago
      You are of course correct. This also ties into my sibling comment - the cleverness in clever low-level parsing code is based on assumptions that may be wrong for some part of the huge input space.
  - ahartmetz 4y ago
    Parsing code without any kind of framework or high level helpers is, for lack of a better word, fiddly. C strings are an awful tool to do it because memory safety depends on getting a lot of buffer size / string length calculations right - there are plenty of opportunities to make mistakes. The fiddliness also induces developers to make changes in existing code in the form of local clever tricks instead of adapting the code to cleanly implement new requirements. It's only a small change and the tests (hopefully there are any) pass, job done! But maybe it violates a non-obvious assumption for another clever trick somewhere else, etc...

mikepavone 4y ago

> There is a function, ossl_punycode_decode that decodes Punycode. Punycode is a way to encode Unicode as ASCII, used to represent Unicode strings in the ASCII-only world of DNS. ossl_punycode_decode takes an output buffer, and if the buffer runs out it keeps parsing and verifying the Punycode but discards the rest of the output. > I did not have the heart to figure out why it works like this. Maybe there's a good reason to do progressive parsing. Maybe it's an artifact of how C makes you do memory management or of OpenSSL's C style. Anyway. Not involved in OpenSSL, but this is a fairly common pattern in a lot of C APIs. You want to decode some data, but you're not sure how big the output is going to be ahead of time. You could write a separate function to calculate the length, but that function has to do most of the work of actual decoding to figure out that length. A lot of times the output is small enough it can fit in some conservatively sized buffer so you can save a fair bit of work by having a (potentially stack-allocated) buffer of some fixed size and then allocating a precisely sized buffer on the heap if it turns out to not be big enough. Further, having a separate length function means you typically end up with two similar but separate decode implementations which has its own problems. In most other languages, you have some sort of growable container in the standard library so you just avoid the problem entirely at the expense of having less control over memory allocations.

j-bos 4y ago
I am by no means a rust developer and asking in ignorance, would this have been a problem had it been written in rust?
- vore 4y ago
  Rust would take the growable container approach here, so it wouldn't be vulnerable in the same way. There's nothing in C that prevents you from doing this either, but as the parent post mentions it's common style to do this (see also things like snprintf).
  - Elinvynia 4y ago
    > There's nothing in C that prevents you from doing this either Yet these errors consistently happen in C projects of various sizes.

AceJohnny2 4y ago

> As curl author Daniel Stenberg said, "I've never even considered to decode punycode. Why does something like OpenSSL need to decode this?" > [...] > Internationalization is not the issue, internationalization is the job. I want to cheer at this. Internationalization is hard, really hard, especially in a computing world so defined by its english-language dominance. The little amount of effort demonstrated in defining and testing this feature demonstrates that. For all I admire Stenberg's work and focus on quality, that was a rather poor take from him.

FiloSottile 4y ago
Oh I don't think Daniel was asking why we're doing i18n. My remark was not at him, at all. I was just pre-empting a possible made up objection. He's in fact correct to wonder why punycode decoding ended up in OpenSSL, as the rest of that section explores. The point being that OpenSSL could do its job and still support i18n domains and emails without having to ever decode punycode if only the spec had made different tradeoffs.
- AceJohnny2 4y ago
  ah. Thanks for the clarification!
- tialaramex 4y ago
  Exactly this. I had the very image of this discussion with Python folks for DNSname SANs back when Python was learning not to just rely on CN. Python folks felt like the correct thing was: 1) Implement a Punycode decoder. 2) Decode hostnames and certificate information to get Unicode 3) Compare the Unicode Strings. They were fretting about how complicated it would be to arrange all this, the months of work needed and the need to bring in teams who understood about i18n issues... And I was like No, don't do any of that, take the bytes and compare the bytes. If the bytes aren't identical that's not a match, you are done. It seemed to take a while for it to sink in that this is correct and simpler and thus better.
  - cryptonector 4y ago

MBCook 4y ago

So the blog mentions that in certain cases you have to decode the punycode from one field to compare it to the value in another field. Would it have been safer to encode the data in the other field to punycode and compare the encoded values? That way a hacker can’t mess with your decoder (where bugs like to lie). But at the other end you risk that your encoder has an issue. I do t know how to judge if those are equal risks or not. Thoughts?

mike_d 4y ago
Encoding and decoding are both transforms on attacker supplied input. I don't think its helpful to armchair quarterbacking other peoples mistakes that ended up in a vulnerability when the overall code quality is good. There are a dozen ways they could have done it differently, which may or may not have resulted in different exotic bugs. Punycode was the footgun here. Not the language, or the implementation, or the code. They were forced to do something stupid and complex and dangerous deep within the bowels of a critical library.
moring 4y ago
If you want to avoid risks both in the encoder and decoder, you might have success with a function that takes an encoded strict and a decoded string and compares them directly, encoding/decoding (whatever works better) on the fly. This avoids the allocation altogether, at the cost of having another less-composable piece of code that must be maintained. I'm not sure if that is the way to go. An allocation per se isn't hard to get right, especially in this case where its scope is well-defined and very limited. What happened here is more the fact that while a single piece of code is easy to get right, a whole codebase isn't. So the only value added by my proposed solution above is that it works in an environment where you can't allocate (e.g. memory-constrained systems where you need to know the required amount of memory statically). Bottom line: I don't think you can really solve this problem on a technical level. edit: typo

deleted 4y ago

[deleted]

bumblebritches5 4y ago

nraynaud 4y ago

About the classification: I would add that the people choosing the severity are a bit alone because of secrecy, so they also can’t really ask too much for advice. That’s probably where the NSA could be useful because of their big number of competent and sworn to secrecy employees, but nobody can trust that the zero day sent for an opinion will not be used to fuck with a foreign country on day one.

ahoog42 4y ago

I decided to review SBOMs from about 3,800 popular mobile apps to see if any included vulnerable versions of OpenSSL v3.0.x. No mobile apps did (not surprised) but what did surprise me was 98% of the OpenSSL versions included in these apps were vulnerable to older CVEs. About 16% of the apps included OpenSSL, mostly as a transitive dependency. I posted additional details in this blog+video: https://www.andrewhoog.com/post/how-to-detect-openssl-v3-and-heartbleed-in-mobile-apps/ https://www.andrewhoog.com/post/how-to-detect-openssl-v3-and...

cesarb 4y ago
> No mobile apps did (not surprised) but what did surprise me was 98% of the OpenSSL versions included in these apps were vulnerable to older CVEs. Just the openssl version is not enough, since it could be patched to fix vulnerabilities without increasing the version (this is very common on Linux distributions, which often apply security patches instead of migrating to a new version; for instance, Fedora released a patched 3.0.5 instead of going to 3.0.7). And using an older openssl version does not necessarily mean it's using vulnerable code; according to your blog post, the most common use is SQLCipher, which from a quick look at its README.md seems to use openssl only for the encryption algorithms. Unless the vulnerability was on the basic algorithms used (AES, HMAC, etc), it won't affect this usage.
- ahoog42 4y ago
  Great points. Static binary analysis looks for the version string but doesn’t currently do deeper analysis of reversed code to see if it’s patched. Could go either way. And determining if the code is triggered and exploitable is quite challenging. Dynamic analysis can help here, provided you have the coverage. More generally tho, istm that there will be instances when the version is unpatched and there is some exploitable vector (even if it’s just crashing the app). My hope is to raise awareness for developers (and security) about 1) transitive dependencies and 2) some really old OpenSSL versions in very popular mobile apps. I don’t believe most folks think about this and awareness can lead to shipping safer apps.

deleted 4y ago

[deleted]

jesprenj 4y ago

Why would a TLS library even parse punycode? The sole reason we still use this hack is so that core infrastructure does not have to change when we use internationalized domains. If TLS libraries, DNS servers, HTTP servers, ... needed to be patched anyways for the use of IDNs, then why did we not do it properly and just use UTF-8? No matter how many vulnerabilities we've introduced into software, "Š" in my name still cannot be represented in domain names and is instead written with american letters xn--pga. I'm sorry for poorly articulating my thoughts here.

boxy_brown 4y ago
> Why was this code even necessary > The answer is: an explicit IETF design choice, that made punycode decoding part of X.509 verification, without even a line of acknowledgement in the Security Considerations.
cryptonector 4y ago
The idea is that back when IDNA was created sending just-UTF-8 in places that had historically been ASCII-only would break software. The scale of the breakage was probably unknown. The real question is: would a flag day have been needed to just use UTF-8 where previously only ASCII was allowed. I think the answer to that is "no". The punycode thing certainly avoided the question altogether, which is why it was done, but there's no question that had we bitten the bullet then and just gone with just-use-UTF-8 life today would be easier. (There's also Unicode normalization issues here. One of the problems with just-use-UTF-8 is that dumb servers would only match on octet-wise label equality, but with Unicode you want normalization-insensitive matching. Now, at the time the notion of normalization-insensitive matching hadn't been invented... Requiring something like IDNA made it possible to slip in a normalization requirement.)

deleted 4y ago

[deleted]

cryptonector 4y ago

So, punycode I do think was silly. We should just have used UTF-8 in DNS and have left it at that. Using UTF-8 would not have required a flag day. It would have required upgrading some servers in order to be able to have non-ASCII domainnames, but it wouldn't have broken anyone not using non-ASCII domainnames.

tinus_hn 4y ago
So it would have broken the whole internet for anyone using a UTF-8 domain name and this system would be adopted by no one. It’s like IPv6, using IPv6 doesn’t break anything for anyone not using it. It’s just that if you use ipv6, you can’t talk to half of the internet including this site.
- cryptonector 4y ago
  It wouldn't have broken the whole internet though. It's hyperbolic to suggest that it would have.
  - tinus_hn 4y ago
    Yes, it would, because at the start no one would be supporting it so no one would be able to connect. So it would have broken the entire internets ability to connect to you. And remember, as a user, you don’t just use one DNS server that you need to upgrade, you use multiple and they all need to support it and a typical user is not in a position to upgrade it. If you don’t believe that, believe this: if it were as simple as you claim, just add in UTF8, the world wouldn’t have settled on punycode. They didn’t, because it is not actually that simple. It’s so hard, it’s actually simpler to settle for punycode.