Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial review #1

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions docs/BR.md
Original file line number Diff line number Diff line change
Expand Up @@ -3151,7 +3151,7 @@ A full and complete CRL is a list of all revoked certificates issued by the CA f

A partitioned CRL (sometimes referred to as a "sharded CRL") is a list of revoked certificates issued by the CA for any and all reasons constrained to a specific scope (e.g., temporal sharding).

Minimally, CAs MUST issue either a "full and complete" CRL - or "partitioned" CRLs.
Minimally, CAs MUST issue either a "full and complete" CRL - or "partitioned" CRLs. CAs MUST NOT issue indirect CRLs (i.e., the issuer of the CRL is not the issuer of all Certificates that are included in the scope of the CRL).
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a natural consequence of requiring cRLSign to be asserted in the keyUsage extension of both roots and intermediates.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accepting proposed change.

(thank you!)


If using only partitioned CRLs, the full set of partitioned CRLs MUST cover the complete set of public-key certificates issued by the CA. Thus, the complete set of partitioned CRLs MUST be equivalent to a full CRL for the same set of public-key certificates, if the CA was not using partitioned CRLs.

Expand All @@ -3165,9 +3165,9 @@ Table: CRL Fields
|     `version` | MUST be v2(1) |
|     `signature` | See [Section 7.1.3.2](#7132-signature-algorithmidentifier) |
|     `issuer` | MUST be byte-for-byte identical to the `subject` field of the Issuing CA. |
|     `thisUpdate` | utcTime (YYMMDDHHMMSSZ) MUST be used for dates up to and including 2049. generalTime (YYYYMMDDHHMMSSZ) MUST be used for dates after 2049.|
|     `nextUpdate` | utcTime (YYMMDDHHMMSSZ) MUST be used for dates up to and including 2049. generalTime (YYYYMMDDHHMMSSZ) MUST be used for dates after 2049.|
|     `revokedCertificates` | MUST only be present if the CA has issued a certificate that is both revoked unexpired. See the "revokedCertificates" table for additional requirements. |
|     `thisUpdate` | UTCTime (YYMMDDHHMMSSZ) MUST be used for dates up to and including 2049. GeneralizedTime (YYYYMMDDHHMMSSZ) MUST be used for dates after 2049.|
|     `nextUpdate` | This field MUST be present. UTCTime (YYMMDDHHMMSSZ) MUST be used for dates up to and including 2049. GeneralizedTime (YYYYMMDDHHMMSSZ) MUST be used for dates after 2049.|
|     `revokedCertificates` | MUST only be present if the CA has issued a certificate that is both revoked and unexpired. See the "revokedCertificates" table for additional requirements. |
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two issues with requiring that only unexpired certificates may appear:

  1. RFC 5280, section 3.3 requires that expired certificates appear on at least one CRL: https://bugzilla.mozilla.org/show_bug.cgi?id=1581183
  2. Other ecosystems may require that expired entries continue to appear. The Code Signing Baseline Requirements require entries to not be removed until at least 10 years after expiry (https://cabforum.org/wp-content/uploads/Baseline-Requirements-for-the-Issuance-and-Management-of-Code-Signing.v3.2.pdf, section 7.2). This will be less of a concern as single-use hierarchies are stood up, but it would have an impact now.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accepted proposed changes - and added language from 3.3 of 5280 that states "An entry MUST NOT be removed from the CRL until it appears on one regularly scheduled CRL issued beyond the revoked certificate's validity period."

|     `extensions` | See below table. |
| `signatureAlgorithm` | Encoded value MUST be byte-for-byte identical to the `tbsCertList.signature`. |
| `signature` | |
Expand All @@ -3177,17 +3177,17 @@ Table: CRL Extensions

| __Extension__ | __Presence__ | __Critical__ | __Description__ |
| ---- | - | - | ----- |
| `authorityKeyIdentifier` | MUST | N | MUST be byte-for-byte identical to the Subject Key Identifier in the issuing CA certificate. authorityCertIssuer and authorityCertSerialNumber MUST NOT be populated. |
| `CRLNumber` | MUST | N | MUST convey a monotonically increasing sequence. |
| `IssuingDistributionPoint` | * | Y | Partitioned CRLs MUST include at least one of the names from the corresponding distributionPoint field of the cRLDistributionPoints extension of every certificate that is within the scope of this CRL. The encoded value MUST be byte-for-byte identical to the encoding used in the distributionPoint field of the certificate. This extension is NOT RECOMMENDED for full and complete CRLs |
| `authorityKeyIdentifier` | MUST | N | The `keyIdentifier` field MUST be included and its value MUST be byte-for-byte identical to the Subject Key Identifier of the Issuing CA's Certificate. The `authorityCertIssuer` and `authorityCertSerialNumber` fields MUST NOT be populated. |
| `CRLNumber` | MUST | N | MUST contain an INTEGER greater than or equal to 0 and less than 2**159 and convey a monotonically increasing sequence. |
| `IssuingDistributionPoint` | * | Y | Partitioned CRLs MUST include at least one of the names from the corresponding distributionPoint field of the cRLDistributionPoints extension of every certificate that is within the scope of this CRL. The encoded value MUST be byte-for-byte identical to the encoding used in the distributionPoint field of the certificate. The `indirectCRL` and `onlyContainsAttributeCerts` fields MUST be set to `FALSE` (i.e., not asserted). The CA MAY set either of the `onlyContainsUserCerts` and `onlyContainsCACerts` fields to `TRUE`, depending on the scope of the CRL. The CA MUST NOT assert both of the `onlyContainsUserCerts` and `onlyContainsCACerts` fields. The `onlySomeReasons` field SHOULD NOT be included; if included, then the CA MUST provide another CRL whose scope encompasses all revocations regardless of reason code. This extension is NOT RECOMMENDED for full and complete CRLs. |
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we can sunset the use of "onlySomeReasons" entirely, since 5280 requires provisioning a CRL independent of reason code anyway.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accepting proposed changes with minor formatting for consistency with Profiles Ballot (e.g., superscript formatting of 2^159).

| Any other extension | NOT RECOMMENDED | - | |

Table: revokedCertificates Component

| __Component__ | __Presence__ | __Description__ |
| ---- | - | ----- |
| `serialNumber` | MUST | MUST be byte-for-byte identical to the serialNumber contained in the revoked certificate.|
| `revocationDate` | MUST | The date and time which revocation occured. utcTime (YYMMDDHHMMSSZ) MUST be used for dates up to and including 2049. generalTime (YYYYMMDDHHMMSSZ) MUST be used for dates after 2049. |
| `revocationDate` | MUST | The date and time which revocation occured. UTCTime (YYMMDDHHMMSSZ) MUST be used for dates up to and including 2049. GeneralizedTime (YYYYMMDDHHMMSSZ) MUST be used for dates after 2049. |
| `crlEntryExtensions` | * | See crlEntryExtensions table (below) |

Table: crlEntryExtensions Component
Expand Down