1246117 - Return a more descriptive error code from mozilla::pkix for malformed DNS IDs in certificates, instead of ERROR_BAD_DER

Status: NEW

(NSS :: Libraries, enhancement, P5)

Description

[Franziskus Kiefer [:franziskus]]

mozilla::pkix throws ERROR_BAD_DER in cases that have nothing to do with DER. A wrong dns id for example causes a DER error [1]. To reproduce go to [2]. While the certificate at [2] is fine it contains an invalid dns name. It thus fails IsValidDNSID and a DER error is thrown, which is unrelated to the actual problem.

[1] https://dxr.mozilla.org/mozilla-central/rev/584870f1cbc5d060a57e147ce249f736956e2b62/security/pkix/lib/pkixnames.cpp#1052
[2] https://ssl.lenaundniklas.de/

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

"ERROR_BAD_DER" really means "Failed to parse" generically. There is another bug on file that changes the parsing code to use a boolean-ish Input::Result type to make that clearer. (The "BAD_DER" part comes from origin of the parsing code only being for DER.)

On the one hand, it isn't the worst idea in the world for mozilla::pkix to return some other error code. On the other hand, the CA shouldn't have issued such certificates in the first place, and any such certificates should be revoked. IMO, it's not worth adding extra code to mozilla::pkix just to deal with such certificates, which truly are malformed.

Comment 2

[Franziskus Kiefer [:franziskus]]

Agree, we shouldn't add code to allow malformed certs (though safari and chrome are fine with it). The problem on the CA has been fixed already [1]. But a more descriptive error message would be really nice here.

[1] https://github.com/letsencrypt/boulder/issues/1440

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

I mean, I'm not sure it's worthwhile to add extra code to return a better error code. Maybe if it is really, really simple.

Comment 4

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

The platform currently treats bad DER as a hostname mismatch in these cases.