989516 - mozilla::pkix: temporarily accept improper encodings of basicConstraints:cA

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Dana Keeler (she/her) [:keeler]]

As we learned in bug 988633, Go Daddy has issued a number of certificates with an improperly encoded basicConstraints extension (the specific issue being that default values in a SEQUENCE must not be explicitly encoded; the affected end-entity certificates have the value cA:false explicitly encoded).
This is a considerable compatibility concern. I think the best way forward is to temporarily accept these improper encodings while working with Go Daddy to fix this situation as quickly as possible. The changes to mozilla::pkix will be tracked in this bug. Any Go Daddy communications/decisions will be tracked in bug 988633.

Comment 1

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

It would be great if we could change OptionalBoolean so that it takes a flag indicating whether or not to allow the invalid explicit encoding of the default value, instead of always allowing the invalid encoding for every optional boolean. That way, we can identify exactly which parts of certificates and/or OCSP responses require this workaround, which will help us in communicating with CAs and software developers to fix their encoding issues.

Comment 2

[Dana Keeler (she/her) [:keeler]]

Attachment: patch

Comment 3

[Camilo Viecco (:cviecco)]

Comment on attachment 8399545 [details] [diff] [review]
patch

Review of attachment 8399545 [details] [diff] [review]:
-----------------------------------------------------------------

r+ Would like to do the nit.

::: security/pkix/lib/pkixder.h
@@ +379,2 @@
>  inline Result
> +OptionalBoolean(Input& input, bool allowInvalidExplicitEncoding,

I would have placed the optional allowInvalidExplicitEnconding as last parameter (with optional value, defaulted to false).

Comment 4

[Dana Keeler (she/her) [:keeler]]

13:16      keeler | cviecco: for bug 989516, my reasoning was that out parameters should go last. Thoughts?
13:17     cviecco | I like optional parameters last as I can use default values and thus simplifies reading of code that does not have the optionals.  But at
                  | this point just land it.
13:17      keeler | ok - will do

Comment 5

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

(In reply to David Keeler (:keeler) from comment #4)
> 13:16      keeler | cviecco: for bug 989516, my reasoning was that out
> parameters should go last. Thoughts?
> 13:17     cviecco | I like optional parameters last as I can use default
> values and thus simplifies reading of code that does not have the optionals.
> But at
>                   | this point just land it.
> 13:17      keeler | ok - will do

Out parameters should go last, even if/when that means we can't use defaulted values for parameters.

Comment 6

[Dana Keeler (she/her) [:keeler]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a8fd27b3dbae

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a8fd27b3dbae