Most active commenters
  • timewizard(3)

←back to thread

314 points Bogdanp | 13 comments | | HN request time: 1.937s | source | bottom
1. timewizard ◴[] No.44380241[source]
I've personally never felt comfortable using regexes to solve production problems. The certificate code referenced here shows why:

https://github.com/mozilla-firefox/firefox/blob/d5979c2a5c2e...

Yikes.

replies(3): >>44380967 #>>44381082 #>>44381215 #
2. ameliaquining ◴[] No.44380967[source]
I think that's not doing anything security-critical, it's just formatting an IPv6 address for display in the certificate-viewer UI.
3. cpburns2009 ◴[] No.44381082[source]
All that regex does is split an IPv6 address into groups of 4 digits, joins them with ":", and collapses any sequence of ":0000:" to "::". I don't see anything problematic with it.
replies(1): >>44381405 #
4. baobun ◴[] No.44381215[source]
Unless you see a glaring issue I don't: I think you are getting the causality wrong there. You "Yikes" because of your discomfort and lack of practice with regexes.
replies(1): >>44381413 #
5. timewizard ◴[] No.44381405[source]
> and collapses any sequence of ":0000:" to "::"

Which is an error. Any ip like 2001:0000:0000::1 is going to be incorrect. It willingly produces errors. Whoever wrote this didn't even spend a few seconds thinking about the structure of IPv6 addresses.

> I don't see anything problematic with it.

Other than it being completely wrong and requiring a regex to be compiled for an amount of work that's certainly less than the compilation itself.

replies(4): >>44381780 #>>44381818 #>>44381852 #>>44384570 #
6. timewizard ◴[] No.44381413[source]
> You "Yikes" because of your discomfort and lack of practice with regexes.

That's exceptionally presumptions to the point of being snotty.

> I think you are getting the causality wrong there.

Where did I imply causality? This was simply an occasion to look at the code. This is bad code. I would not pass this. What's your _justification_ for using a regex here?

replies(1): >>44382487 #
7. cpburns2009 ◴[] No.44381780{3}[source]
It only operates on a 32 digit IPv6 address so it won't already be abbreviated. My phrasing was inexact. It replaces only the first sequence of any number of ":0000:" to "::".
8. remram ◴[] No.44381818{3}[source]
> Any ip like 2001:0000:0000::1 is going to be incorrect.

This is neither a possible input nor a possible output of that code.

replies(1): >>44383205 #
9. ephou7 ◴[] No.44381852{3}[source]
> Other than it being completely wrong and requiring a regex to be compiled for an amount of work that's certainly less than the compilation itself.

It's not. And the sequence you describe is not even parsed because colons are not part of the IPv6 extension of the SAN. PLease educate yourself before spilling such drivel.

10. baobun ◴[] No.44382487{3}[source]
> Where did I imply causality?

> > The certificate code referenced here shows why

So what's the implication here, then?

> This is bad code.

Without justifying further I think we're on equal footing on the snottiness here (:

What's bad? Why not use regex here? It's not like they're using it to parse user-controlled HTML. Simple string transormations like this is a great use-case where the manual character iteration easily becomes inefficient and messy. And you may introduce bugs in the process (unicode length bugs are common).

Do you also avoid grep and sed without the -F flag in shell?

11. dontdoxxme ◴[] No.44383205{4}[source]
That example doesn't work, but an IPv6 address like: 3fff:0020::

Would be in the IP SAN as 3fff0020000000000000000000000000, which this code expands:

   "3fff0020000000000000000000000000"
                .toLowerCase()
                .match(/.{1,4}/g)
                .join(":")
                .replace(/\b:?(?:0+:?){2,}/, "::")
   '3fff::20:0000:0000:0000:0000:0000:0000'
Which has one too many parts and doesn't parse as an IPv6 address. But like mentioned this is just presentation code. I don't want to waste time if this isn't actually a bug, but maybe someone on the LetsEncrypt trial could actually make a cert to see if IP addresses formatted like that are a problem in reality...
replies(1): >>44383387 #
12. remram ◴[] No.44383387{5}[source]
That one does look like a bug. I stand corrected.
13. ranger_danger ◴[] No.44384570{3}[source]
> Any ip like 2001:0000:0000::1 is going to be incorrect

How so?