[Freeipa-devel] [PATCH] 339 fix some certificate issues

[John Dennis]

On 12/18/2009 07:45 AM, Jason Gerard DeRose wrote:
> On Thu, 2009-12-17 at 11:32 -0500, Rob Crittenden wrote:
>> Found a few problems with certificate handling with certmonger. Add a
>> try/except to handle base64-encoded certificates more gracefully. I had
>> also missed a function import causing things to blow up in some cases.
>>
>> rob
>
> ack.  pushed to master.

Hmm... maybe this should have been NAK'ed. The issues were under active 
discussion. I don't think the patch is doing any harm but I'm not sure 
it's the right solution. Maybe the patch shouldn't have been applied.

We have to be careful with our data types.

The patch effectively was trying to determine if a certificate was 
encoded in binary DER format as opposed to base64 encoded PEM format by 
trying to base64 decode the certificate, if it successfully decoded it 
was assumed to be PEM. That's not the right way to handle this IMHO.

We either need to:

* adopt the convention that all certificates are in pem format when 
exchanged at an interface boundary

* Have a method to unambiguously identify the certificate encoding, this 
could be done in one of two ways.

1. Always associate an encoding format attribute with the certificate

2. We do have the ability to unambiguously distinguish between binary 
objects and text objects. We could adopt the convention that if the data 
type of the certificate object is binary it is in DER format and if the 
data type of the certificate is TEXT then it's in PEM format.

The distinction between binary and text is based on whether the object 
is a str class or a unicode class. The downside of this approach is 
we've haven't been rigorous with enforcing the correct data types, a 
problem compounded by the fact Python happily converts between str and 
unicode silently. Provided we're careful with using the right data type 
then the following would work:

if type(cert) is unicode:
     cert_der = base64.b64decode(cert)
else:
     cert_der = cert

-or-

if type(cert) is str:
     cert_pem = cert
else:
     cert_pem = der_cert_to_pem(cert)

What we don't want to do is start employing heuristics to guess the 
encoding, format, or data type of objects, it's not robust defensive 
coding practice.

-- 
John Dennis <jdennis at redhat.com>

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/