glenpowell / certificate-transparency

Automatically exported from code.google.com/p/certificate-transparency
0 stars 0 forks source link

Format IP addresses in names instead returning tuples of ints, patch included #12

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
http://codereview.appspot.com/14660043

in name.py, __parse_name_value has

         elif self.__type == x509_name.IP_ADDRESS_NAME:
                 self.__value = name_value.asNumbers()

This returns a tuple of ints.   However, other name-types return a string (most 
of the time). 

The following patch formats the IP NAME as an IPv4 or IPv6 depending on size.

Why I care:  there is a test cert for a CVE that contains names with embedded 
null characters and IP addresses.  I'd like to add it to the CT test suite.  
It's much easier if all values from subject alternative name are strings.

If accepted I'll submit the test case as well.

thanks,

nickg

Original issue reported on code.google.com by nickgsup...@gmail.com on 14 Oct 2013 at 2:27

GoogleCodeExporter commented 9 years ago
Ahh, you already have a test cert with an IPv4 address.  Patch updated to fix 
unit test.

Original comment by nickgsup...@gmail.com on 14 Oct 2013 at 3:12

GoogleCodeExporter commented 9 years ago
ahh, I understand the API a bit more now... I thought the type was dropped. 
It's not, so the caller could do this IP int-to-text transformation.  However I 
think this it's a good patch regardless.

Original comment by nickgsup...@gmail.com on 14 Oct 2013 at 3:27

GoogleCodeExporter commented 9 years ago
I agree that there's an issue with the API.

But I'm reluctant to accept the patch because certificates can (and do) contain 
bogus IP addresses, so if you return a formatted string, the caller has to 
reverse-engineer from the format whether they got an IPv4 address, an IPv6 
address, or "unknown".

I suspect the right answer is to properly wrap the returned type in an 
IPAddress class. I have an ASN.1 rewrite in the works, so I'll try to address 
it as part of it.

Meanwhile, as you noted, you can reconstruct the IP from the tuple at the 
caller. Not pretty but works.

Original comment by ekasper@google.com on 14 Oct 2013 at 6:07

GoogleCodeExporter commented 9 years ago
re: Meanwhile, as you noted, you can reconstruct the IP from the tuple at the 
caller. Not pretty but works.

Actually it's pretty enough.  I wouldn't expect the library to pretty--print 
things in exactly the same format as I would like.  Having the type, and the 
value in a not-crazy-format is fine (and a tuple of ints is not crazy).  Also, 
IPv6 string formatting is not trivial.

thanks for your time.

nickg

Original comment by nickgsup...@gmail.com on 14 Oct 2013 at 11:41

GoogleCodeExporter commented 9 years ago
Fixed in 
https://code.google.com/p/certificate-transparency/source/detail?r=8c76f1c8965df
68efe9b90da763aed4dbf07ce63

The API now allows extracting a list of IPAddress entries which support a 
semantic __str__.

Original comment by ekasper@google.com on 9 Dec 2013 at 3:23