Handle duplicate extensions

Handle duplicate extensions

[ekasper, 2013-09-20 10:19:02]

And just for good measure, handle corrupt time, too.

The cert that choked the scan had a dupe EKU - I've checked that it no longer
chokes. I'll add a specific test when I add EKU parsing.

[Ben Laurie (Google), 2013-09-20 11:01:27]

https://codereview.appspot.com/13750045/diff/3001/src/python/ct/crypto/cert.py
File src/python/ct/crypto/cert.py (right):

https://codereview.appspot.com/13750045/diff/3001/src/python/ct/crypto/cert.p...
src/python/ct/crypto/cert.py:283: raise CertificateError("Corrupt notBefore")
Maybe try to reparse the not_before here, so the "real" error is raised? Or
cache the exception?

[ekasper, 2013-09-20 11:29:28]

https://codereview.appspot.com/13750045/diff/3001/src/python/ct/crypto/cert.py
File src/python/ct/crypto/cert.py (right):

https://codereview.appspot.com/13750045/diff/3001/src/python/ct/crypto/cert.p...
src/python/ct/crypto/cert.py:283: raise CertificateError("Corrupt notBefore")
On 2013/09/20 11:01:28, Ben Laurie (Google) wrote:
> Maybe try to reparse the not_before here, so the "real" error is raised? Or
> cache the exception?

I like! Done. Also noticed that I can simplify the rest by going through this
API.

[Ben Laurie (Google), 2013-09-20 13:09:12]

https://codereview.appspot.com/13750045/diff/12001/src/python/ct/crypto/cert.py
File src/python/ct/crypto/cert.py (right):

https://codereview.appspot.com/13750045/diff/12001/src/python/ct/crypto/cert....
src/python/ct/crypto/cert.py:97: decoding_error)
Make CachedTime() a class and move all the above into it to avoid duplication?

https://codereview.appspot.com/13750045/diff/12001/src/python/ct/crypto/cert....
src/python/ct/crypto/cert.py:307: if self.__not_before.decoded_time is None:
Now this duplicated code ought to go into CachedTime, too.

[ekasper, 2013-09-20 13:32:17]

https://codereview.appspot.com/13750045/diff/12001/src/python/ct/crypto/cert.py
File src/python/ct/crypto/cert.py (right):

https://codereview.appspot.com/13750045/diff/12001/src/python/ct/crypto/cert....
src/python/ct/crypto/cert.py:97: decoding_error)
On 2013/09/20 13:09:12, Ben Laurie (Google) wrote:
> Make CachedTime() a class and move all the above into it to avoid duplication?

Done.

I was a bit sloppy because I'm rewriting the decoder to do this automatically
for all components, so we can partially parse malformed certificates. But this
case is easy so I've done it.

https://codereview.appspot.com/13750045/diff/12001/src/python/ct/crypto/cert....
src/python/ct/crypto/cert.py:307: if self.__not_before.decoded_time is None:
On 2013/09/20 13:09:12, Ben Laurie (Google) wrote:
> Now this duplicated code ought to go into CachedTime, too.

Done.

[Ben Laurie (Google), 2013-09-20 13:43:30]

LGTM

https://codereview.appspot.com/13750045/diff/18001/src/python/ct/crypto/cert.py
File src/python/ct/crypto/cert.py (right):

https://codereview.appspot.com/13750045/diff/18001/src/python/ct/crypto/cert....
src/python/ct/crypto/cert.py:305: 
Extra blank line.

[ekasper, 2013-09-20 13:46:24]

https://codereview.appspot.com/13750045/diff/18001/src/python/ct/crypto/cert.py
File src/python/ct/crypto/cert.py (right):

https://codereview.appspot.com/13750045/diff/18001/src/python/ct/crypto/cert....
src/python/ct/crypto/cert.py:305: 
On 2013/09/20 13:43:30, Ben Laurie (Google) wrote:
> Extra blank line.

Will remove.

[Eran, 2013-09-23 13:50:28]

Drive-by review. Overall LGTM.

https://codereview.appspot.com/13750045/diff/18001/src/python/ct/crypto/cert.py
File src/python/ct/crypto/cert.py (right):

https://codereview.appspot.com/13750045/diff/18001/src/python/ct/crypto/cert....
src/python/ct/crypto/cert.py:48: # We can't raise here because we don't want to
abort __init__,
Why not do caching the standard way, when decode_time() is first called?
Will save the need for storing the error and just more standard pattern.