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

Remove references to certificate contents from issuance process #245

Merged
merged 4 commits into from
Oct 23, 2024

Conversation

aarongable
Copy link
Contributor

Sections 4.3.1 and 4.4.2 do not need to describe certificate contents, only the actions undertaken to issue those certificates.

Sections 4.3.1 and 4.4.2 do not need to describe certificate contents, only the actions undertaken to issue those certificates.
CP-CPS.md Outdated
@@ -314,7 +314,7 @@ No stipulation.

### 4.3.1 CA actions during certificate issuance

At a high level, the following steps are taken during issuance of a Subscriber Certificate. ISRG's automated processes confirm that all names which will appear in the Common Name and/or list of SANs of the certificate have been properly validated to be controlled by the Subscriber requesting the certificate. The to-be-signed certificate is linted, then signed by a Subordinate CA in an HSM. After issuance is complete, the certificate is stored in a database and made available to the Subscriber.
At a high level, the following steps are taken during issuance of a Subscriber Certificate. ISRG's automated processes confirm that all names that will be listed in the certificate have been properly validated to be controlled by the Subscriber requesting the certificate. The to-be-signed certificate is linted, then signed by a Subordinate CA in an HSM. After issuance is complete, the certificate is stored in a database and made available to the Subscriber.
Copy link
Contributor

@bdaehlie bdaehlie Oct 21, 2024

Choose a reason for hiding this comment

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

This doesn't seem correct, but maybe I am misreading. With this edit we're saying that all names in the cert have been properly validated as controlled by the subscriber. What about the names of OCSP endpoints for example? Any other names in the certs like that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point -- for example, the "CRLDistributionPoints" extension contains a GeneralName field, so definitely counts as a "name". That said, I borrowed this language from Section 3.2.2.4 of the BRs, which says "The CA SHALL confirm that prior to issuance, the CA has validated each Fully-Qualified Domain Name (FQDN) listed in the Certificate...:. CRLDPs are also FQDNs, but no one thinks that Section 3.2.2.4 means that they need to be validated as controlled by the Applicant.

What if we said "all names associated with the subject", meaning both in the subject and in subjectAltNames and any other future extensions, or something like that?

Copy link
Contributor

Choose a reason for hiding this comment

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

I just pointed out this same issue in another PR.

Maybe the most clear way to handle this is to create a defined term, like Subscriber Domains (I am not stuck on that particular wording). Then we can use the term in a number of places and there would be no ambiguity.

Copy link
Member

@beautifulentropy beautifulentropy Oct 22, 2024

Choose a reason for hiding this comment

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

Perhaps we can lift the phrase "every requested identifier" from Section 4.2.2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Brainstorming other ideas:

  • Authorization Domain Name: By definition all the identifiers that we check are Authorization Domain names, and the BRs definition of that term defines it as "The FQDN used to obtain authorization for a given FQDN to be included in a Certificate.". So it gets a bit circular, but because OCSP and CRL URLs are not validated in the same way, they can't be Authorization Domain Names, so they wouldn't be included if we used that term.
  • "applied-for names": The BRs use this phrasing in the definition of "Base Domain Name". It has a very clear meaning, but is used less ubiquitously than "requested" (1 appearance vs 11).

I think I like Samantha's suggestion of using "requested" to qualify the names. It's simple, obvious, and reflects language used throughout the BRs.

Copy link
Member

Choose a reason for hiding this comment

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

"Authorization Domain Name" is the most viable alternative and I feel it's a little too verbose.

@aarongable aarongable requested a review from bdaehlie October 22, 2024 16:10
@@ -314,7 +314,7 @@ No stipulation.

### 4.3.1 CA actions during certificate issuance

At a high level, the following steps are taken during issuance of a Subscriber Certificate. ISRG's automated processes confirm that all names which will appear in the Common Name and/or list of SANs of the certificate have been properly validated to be controlled by the Subscriber requesting the certificate. The to-be-signed certificate is linted, then signed by a Subordinate CA in an HSM. After issuance is complete, the certificate is stored in a database and made available to the Subscriber.
At a high level, the following steps are taken during issuance of a Subscriber Certificate. ISRG's automated processes confirm that all requested names have been properly validated to be controlled by the Subscriber requesting the certificate. The to-be-signed certificate is linted, then signed by a Subordinate CA in an HSM. After issuance is complete, the certificate is stored in a database and made available to the Subscriber.

Choose a reason for hiding this comment

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

I worry about using a non-defined term "names" here. While intuitively I know what it means (CN and SANs), I worry it could be unclear. For example, is an IP Address a name? (yes: it's a SAN, but is it well defined enough to make that clear?)

Copy link
Contributor

Choose a reason for hiding this comment

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

It makes me uncomfortable as well because of the lack of clarity that Matthew mentioned, the plain English reading is confusing, and it raises the question of "requested by who?" without a readily available answer.

"requested names" is a clear improvement over what we had before with minimal work but we should probably take it further. If there is a reason to postpone a more complete solution for the future though, I could live with it for now.

Copy link
Contributor Author

@aarongable aarongable Oct 22, 2024

Choose a reason for hiding this comment

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

I think the definition of "name" here is "anything encoded as an ASN.1 GeneralName". The term is used that way throughout the BRs already:

The permittedSubtrees MUST contain at least one GeneralSubtree for both of the dNSName and iPAddress GeneralName name types, UNLESS the specified GeneralName name type appears within the excludedSubtrees to exclude all names of that name type.

CAs SHALL NOT include additional names unless the CA is aware of a reason for including the data in the Certificate.

High Risk Certificate Request: A Request that the CA flags for additional scrutiny by reference to internal criteria and databases maintained by the CA, which may include names at higher risk for phishing or other fraudulent usage

including one that explicitly counts IP Addresses as names:

CAs SHALL NOT issue certificates containing Internal Names or Reserved IP Addresses, as such names cannot be validated according to Section 3.2.2.4 or Section 3.2.2.5.

CP-CPS.md Outdated Show resolved Hide resolved
@aarongable aarongable requested a review from bdaehlie October 22, 2024 17:49
CP-CPS.md Show resolved Hide resolved
CP-CPS.md Outdated Show resolved Hide resolved
@aarongable aarongable requested review from jsha and bdaehlie October 22, 2024 23:05
@aarongable aarongable merged commit bd16234 into main Oct 23, 2024
3 checks passed
@aarongable aarongable deleted the aarongable-patch-7 branch October 23, 2024 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants