On Thu, Dec 10, 2015 at 9:40 AM, David Howells <[email protected]> wrote: > David Howells <[email protected]> wrote: > >> > the leap second support still looks a bit suspect, as mktime64 will convert >> > mm/dd/YYYY HH/MM/60 and mm/dd/YYYY HH/MM+1/00 to the same time64_t, >> > essentially meaning that two different inputs can yield the same output, >> > possibly violating ASN.1 CER and DER rules. >> >> That's a 'bug' in mktime64() not my parsing of the ASN.1. If it's valid >> ASN.1 >> then we should accept it. > > Any thoughts on how to handle this? I really want to push this patch > upstream. > > David
I suppose the first question to ask would be, would anything break security-wise if we did relax it? Because if it's fine to relax it, we should. >From how I understand ASN.1, unless we already require CER or DER, we can safely accept non canonical representations. Given this code also calls asn1_ber_decoder, I don't think it expects much canonicality to begin with. This would mean we can also safely support leap seconds and the 24:00:00 representation. Signature checking is also done against the TBS data in the form as supplied by the caller, and not against a re-serialized timestamp. Thus the timestamp conversion not being injective at least doesn't SEEM like an issue. What seems to confirm this is that there is currently no kernel code that can serialize a time64_t into ASN1_UNITIM - and if there is no such code to begin with, I can't see how non-injecitivity could be a problem. So I'd say: go ahead, but also support 24:00:00 while at it. Possibly add a comment that the time parsing isn't injective so nobody in the future expects it to be. Best regards, Rudolf Polzer -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
