Firefox no longer accepts certificates with time values that are invalid according to RFC 5280 but are otherwise valid ASN.1

RESOLVED INVALID

Status

()

defect
RESOLVED INVALID
4 years ago
4 years ago

People

(Reporter: kaie, Unassigned)

Tracking

33 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Reporter

Description

4 years ago
I have an Intranet CA certificate, valid until 2021, imported and marked as trusted, which Firefox / mozilla::pkix rejects as "expired".

The CA is self-signed (same issuer and subject).

The unusual detail about that self-signed CA is that it has subject key ID and authority key ID with identical values.

Is that allowed?

Could it potentially cause mozilla::pkix to loop, trying to find a trusted cert, and eventually hit some limit and stop with a random error?

I can see the error in Firefox 33 and later.

Firefox 31 works fine, regardless of the value of the use-mozilla-pkix preference.
Reporter

Updated

4 years ago
Summary: Imported and trusted CA certificate rejected as expired → Imported and trusted CA certificate, incorrectly rejected as expired
The most likely reason is that the encoding of the notBefore or notAfter time is non-conformant with RFC 5280.

Note that mozilla::pkix presently doesn't even parse the SKI or AKI at all.
Try with Firefox Nightly. I bet you get a better error message. We improved the error message for this recently.
Reporter

Comment 3

4 years ago
Thanks for the quick response.
Ineed, dumpasn1 says the time encoding is bad:

123  34:     SEQUENCE {
125  17:       UTCTime '110803212737+0200'
       :         Error: Time is encoded incorrectly.
144  13:       UTCTime 01/08/2021 19:27:37 GMT
       :       }

Firefox nightly didn't help much, it reports the error as "cannot verify for unknown reasons".
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → INVALID

Comment 4

4 years ago
(In reply to Kai Engert (:kaie) from comment #3)
> Thanks for the quick response.
> Ineed, dumpasn1 says the time encoding is bad:
> 
> 123  34:     SEQUENCE {
> 125  17:       UTCTime '110803212737+0200'
>        :         Error: Time is encoded incorrectly.
> 144  13:       UTCTime 01/08/2021 19:27:37 GMT
>        :       }
> 
> Firefox nightly didn't help much, it reports the error as "cannot verify for
> unknown reasons".

FWIW, I think what Brian meant was that visiting a site chaining up to relevant cert would result in an error page mentioning SEC_ERROR_INVALID_TIME.
Reporter

Comment 5

4 years ago
Adding some more details:

ASN.1 allows a time encoding that includes a trailing time zone offset, but RFC 5280 doesn't allow it.

In the past, Firefox was lenient, NSS still is lenient, but mozilla::pkix is strict. I found a comment in the mozilla::pkix which says the alternative encodings from asn.1 explicitly aren't allowed.
Reporter

Comment 6

4 years ago
Actually, I see the following statement in RFC 5280:

   4.1.2.5.2.  GeneralizedTime

   The generalized time type, GeneralizedTime, is a standard ASN.1 type
   for variable precision representation of time.  Optionally, the
   GeneralizedTime field can include a representation of the time
   differential between local and Greenwich Mean Time.

   For the purposes of this profile, GeneralizedTime values MUST be
   expressed in Greenwich Mean Time (Zulu) and MUST include seconds
   (i.e., times are YYYYMMDDHHMMSSZ), even where the number of seconds
   is zero.  GeneralizedTime values MUST NOT include fractional seconds.


This is slightly confusing to me.

The latter part says, it must be in GMT. However, the former part say, it can include a time differential to GMT.


Reopening until we get clarity.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Reporter

Updated

4 years ago
Summary: Imported and trusted CA certificate, incorrectly rejected as expired → Firefox no longer accepts certificates with time values encoded to include time zone information
Reporter

Comment 7

4 years ago
Ryan, how do you interpret the RFC text that I quoted in comment 6 ?

Do you think the sentence that starts with "Optionally..." could be a sufficient argument for us to continue to be lenient?
Flags: needinfo?(ryan.sleevi)

Comment 8

4 years ago
(In reply to Kai Engert (:kaie) from comment #7)
> Ryan, how do you interpret the RFC text that I quoted in comment 6 ?
> 
> Do you think the sentence that starts with "Optionally..." could be a
> sufficient argument for us to continue to be lenient?

No. It's merely providing a description of the ASN.1 type (for those not familiar with ASN.1) before defining the profile of DER.

The "Optionally" is not normative language, it's descriptive. The MUST is normative language.
Flags: needinfo?(ryan.sleevi)
(In reply to Kai Engert (:kaie) from comment #6)
>    4.1.2.5.2.  GeneralizedTime
> 
>    The generalized time type, GeneralizedTime, is a standard ASN.1 type
>    for variable precision representation of time.  Optionally, the
>    GeneralizedTime field can include a representation of the time
>    differential between local and Greenwich Mean Time.

i.e. ASN.1 allows various encodings to be used...

>    For the purposes of this profile, GeneralizedTime values MUST be
>    expressed in Greenwich Mean Time (Zulu) and MUST include seconds
>    (i.e., times are YYYYMMDDHHMMSSZ), even where the number of seconds
>    is zero.  GeneralizedTime values MUST NOT include fractional seconds.

...but this standard only allows one specific encoding.
Summary: Firefox no longer accepts certificates with time values encoded to include time zone information → Firefox no longer accepts certificates with time values that are invalid according to RFC 5280 but are otherwise valid ASN.1

Comment 10

4 years ago
Isn't these notBefore/notAfter fields are UTCTime and not GeneralizedTime?
(In reply to Alon Bar-Lev from comment #10)
> Isn't these notBefore/notAfter fields are UTCTime and not GeneralizedTime?

Both UTCTime and GeneralizedTime are allowed, but both have the same constraints on encoding. See http://tools.ietf.org/html/rfc5280#section-4.1.2.5.1 for UTCTime.

Comment 12

4 years ago
(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #11)
> (In reply to Alon Bar-Lev from comment #10)
> > Isn't these notBefore/notAfter fields are UTCTime and not GeneralizedTime?
> 
> Both UTCTime and GeneralizedTime are allowed, but both have the same
> constraints on encoding. See
> http://tools.ietf.org/html/rfc5280#section-4.1.2.5.1 for UTCTime.

Thanks!
This is strange coming from RFC and restrict ASN.1 primitive.
(In reply to Alon Bar-Lev from comment #12)
> This is strange coming from RFC and restrict ASN.1 primitive.

Keep in mind that X.509 certificates are DER-encoded and one of the main points of DER encoding is to allow only one encoding of every value, so that the DER encoding can be the canonical value.

Thus, I expect that the specification for DER in ASN.1 also places the same constraint on the encoding.

Comment 14

4 years ago
(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #13)
> (In reply to Alon Bar-Lev from comment #12)
> > This is strange coming from RFC and restrict ASN.1 primitive.
> 
> Keep in mind that X.509 certificates are DER-encoded and one of the main
> points of DER encoding is to allow only one encoding of every value, so that
> the DER encoding can be the canonical value.
> 
> Thus, I expect that the specification for DER in ASN.1 also places the same
> constraint on the encoding.

I do not understand this statement, as there is a CHOICE to use either UTCTime or GeneralizedTime in this field. Encoding of ASN.1 does not change anything of the actual decoded content.

The restriction on UTCTime in this case is within the X.509 RFC, it like having RFC which states that DISPLAY-STRING's 1st letter must be 'a'...

It is not that important now, should be fixed in CA anyway.
Reporter

Comment 15

4 years ago
Regarding Firefox behavior, I'd like to note:

The mozilla::pkix verification code appears to be the only place that doesn't accept the encoding.

The certificate viewer displays the times correctly, which I think is confusing for users, and makes it impossible to use the certificate viewer to view the cause for the rejection.

One could argue, as long as the Firefox certificate viewer displays the time correctly, Firefox should accept it.

Or, if Firefox wants to reject it, then Firefox should change the certificate viewer to display an error instead of a time value.

Maybe mozilla::pkix should accept the encoding until user interface consistency can be achieved?
Reporter

Comment 16

4 years ago
(In reply to Cykesiopka from comment #4)
> FWIW, I think what Brian meant was that visiting a site chaining up to
> relevant cert would result in an error page mentioning
> SEC_ERROR_INVALID_TIME.

Yes indeed. Firefox 38 reports:

  (hostname) uses an invalid security certificate.

  The certificate will not be valid until 08/03/2011 09:27 PM. 
  The current time is 04/10/2015 01:01 PM.

  (Error code: sec_error_invalid_time)

Given that this statement is nonsense, and given that user interface changes are more risky at the last minute, I suggest to keep accepting the encoding in Firefox 38.x.

I recommend to delay the rejection until the user interface (error page and certificat viewer) can be enhanced.
Kai, the stricter parser has been used since Firefox 33. See bug 1019770. We intentionally made the parser stricter, knowing that a very small number of certificates would be rejected by the new parser. We've even had fewer bug reports about the stricter parsing than I expected when we made the change.

I disagree about the relative risk of UI changes vs. parsing changes. Since the UI doesn't affect the security of the connection, it is safer to change it than it is to change the parser. I agree that the UI needs improvement, which should be done in another bug in the "Security: UI" component.

Between Firefox 31 and now, *many, many* changes to how certificates are parsed and processed have been made. This is, by far, not the only change that is not 100% backward compatible.

I think that this bug should RESOLVED INVALID, since things are working as intended.
See Also: → 1019770
Reporter

Comment 18

4 years ago
Posted patch Patch v1Splinter Review
This patch changes mozilla::pkix to look for an environment variable,
with the suggested name
  PKIX_ALLOW_CERT_UTCTIME_OFFSET

If this variable isn't found (unset), the behavior will remain unchanged (time zone encoding reject).

In other words, this patch keeps the current default behaviour.

If the variable is set to any value, then mozilla::pkix will allow and parse the optional time zone offset.

(I hope I got the numbers right, if it's a + offset, then we must distract the offset to get to the UTC numbers.)

We might have to use a patch like this in Firefox 38 on RHEL.

I'm offering this patch for inclusion by Mozilla in official Firefox 38.
(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #17)
> Kai, the stricter parser has been used since Firefox 33. See bug 1019770. We
> intentionally made the parser stricter, knowing that a very small number of
> certificates would be rejected by the new parser. We've even had fewer bug
> reports about the stricter parsing than I expected when we made the change.

It depend how many users have this version and which sites are accessed. We've got this report right after deploying first test 38 ESR package. It looks to me that such certificates may be used in private corporate sites which are accessed from ESR browsers.

Anyway, it's really confusing and hard to find when UI says the cert is okay.

Comment 20

4 years ago
From the peanut gallery - I agree with Brian and don't think patching is a good idea for Mozilla (or for RHEL but that is harder)
A workaround for invalid encodings doesn't appear to be necessary in this case.
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.