[Freeipa-devel] [PATCH] 360 be smarter about decoding certs

[Rob Crittenden]

John Dennis wrote:
> On 01/28/2010 10:30 PM, Rob Crittenden wrote:
>> John Dennis wrote:
>>> On 01/28/2010 04:15 PM, Rob Crittenden wrote:
>>>> Gah, got the description mixed up with the last patch :-(
>>>>
>>>> Be a bit smarter about decoding certificates that might be base64
>>>> encoded. First see if it only contains those characters allowed before
>>>> trying to decode it. This reduces the number of false positives.
>>>
>>> I'm not sure the test is doing what you want or even if it's the right
>>> test.
>>>
>>> The test is saying "If there is one or more characters in the bas64
>>> alphabet then try and decode. That means just about anything will
>>> match, which doesn't seem like a very strong test.
>>>
>>> Why not just try and decode it and let the decoder decide if it's
>>> really base64, the decoder has much strong rules about the input,
>>> including assuring the padding is correct.
>>>
>>
>> The reason is I had a binary cert that was correctly decoded by the
>> base64 encoder. I don't know the why's and wherefores but there it is.
> 
> Then testing to see if each byte is in the base64 alphabet would not 
> have prevented this error.

And yet it did in practice. I think you're assuming too much about the 
input testing in base64.b64decode(). It gladly takes binary data, as 
long as it fits the expected padding.

> 
> For a while now I've been feeling like we need to associate a format 
> attribute to the certificate (e.g. DER, PEM, BASE64, etc.).

There is simply no good way to carry that extra data when all you have 
is a blob of data. We'd still need some mechanism to look at it and ask 
"what are you?" That or we simply reject some types of input.

> Or we need to adopt a convention that certs are always in one canonical 
> format and the interface is responsible for assuring what it accepts as 
> input is converted to the canonical form.

Again, something would need to do that and base64.b64decode() is not 
sufficient.

I know this seems rather hacky, I thought as much when I coded it, just 
trying to make it robust.

rob

> 
>> I see what you mean about my regex being a bit weak though, it really
>> should require that the entire string conform. I'll see what I can do.
>>
>> rob
> 
>