refactor of SMIME aia contains #777
Conversation
This reverts commit 6c23670.
util: gtld_map autopull updates for 2021-10-21T07:25:20 UTC
|
Separating the scheme check to a separate lint does seem appropriate. There are several lints for AIA checking that have a similar structure so we will want to make sure they all have consistent behavior. |
|
I did not find any text in the SMIME CABF Specifications which deals with internal names in AIA/OCSP. I propose to remove this part in the lint and the smime package and if this is a requirement from another specification, implement it as a new lint in the corresponding package. This proposal is implemented in commit a1eee50 |
|
I'll disagree with that one. I don't think a requirement is spelled out in any of the BRs, however the ocsp has to be publicly accessible as does the aia. Is there a use case you're thinking of where an internal name would or should be allowed? Pinging @CBonnell for visibility. |
|
I agree with @cardonator. While the S/MIME BRs do not explicitly state that the URIs contained within the CRLDP and AIA extensions must be publicly accessible, the language cannot reasonably be interpreted as allowing the inclusion of such internal domain names. If one reads the current language in the SMBRs as permitting internal domain names in CRLDP and AIA extensions, then one would also interpret the SMBRs as allowing the issuance of publicly trusted S/MIME certificates that have no publicly available revocation information. More generally, the inclusion of such internal domain names is of no use (and actively harmful due to connection attempts that will inevitably fail) in the common case, as the referenced resources cannot be accessed by most Relying Parties. |
|
The motivation to remove the check about internal names was to keep different aspects of lints separated and left the removed part to future work. Commit 2a6b887 tries to address the issues discussed in this PR. On the one hand two distinct lints exist to keep a narrow focus on each lint, and on the other hand the check for internal names is extended to all SMIME certificates. I hope that the current proposal and implementation reflects the discussion. |
…g/zlint into smime_aia_contains_refactor
v3/lints/cabf_smime_br/lint_strict_aia_contains_internal_names.go
Outdated
Show resolved
Hide resolved
| ExpectedResult: lint.Warn, | ||
| }, | ||
| { | ||
| Name: "warn - aia with valid names, one is ldap", |
There was a problem hiding this comment.
I believe this version of the lint will now cover Legacy profile, which currently allows LDAP scheme to appear in the AIA. That may be unintentional or just missed. Along with that, I think if we're going to do this refactor for the Strict/MP profiles, that we should also do it for the legacy profiles which are additional lints that should be reviewed.
There was a problem hiding this comment.
In the contains_internal_names lint, it is checked whether it is a BR-SMIME certificate, the scheme part is not checked at all and only the host part of the URL is checked. The certificate containing the LDAP URL is a Pass Test. I believe the current implementation covers these apsects. Has something not been taken into account? If there is something missing or is not correct I could extend the implementation.
There was a problem hiding this comment.
Ah, thanks. So, with this lint now covering Legacy, I think we should refactor the Legacy profile lint to only ensure the proper schemes, right? I would think we could just check that at least one OCSP URL and issuer url have the http scheme and remove the internal name checking.
There was a problem hiding this comment.
Commit a5711df covers this.
|
A future browser alignment ballot might help here, as it is my opinion is that a non-public OCSP or CRL server for public S/MIME would violate Mozilla policy: "CA operators MUST NOT issue certificates that have: [...]; or In the context of Mozilla policy for public S/MIME, I would interpret "operational" to include "publicly accessible". Because PUBLIC S/MIME. And this is all just another data point showing that the correct reading of the S/MIME BRs includes the common implicit assumption that CRL and OCSP servers need to be publicly available and operating in a manner generally consistent with how CRL and OCSP servers are generally accepted to operate. |
…acy AIA has one HTTP moved to a new lint, added isSubscriberCert for all checkApplies
|
Great, this LGTM now. @christopher-henderson do you want to take a look? |
|
Thank you @cardonator, @mtgag, and @CBonnell for the thorough policy discussion - it always helps a ton. |
| @@ -40,40 +39,37 @@ For Legacy: When provided, at least one accessMethod SHALL have the URI scheme H | |||
| func init() { | |||
| lint.RegisterCertificateLint(&lint.CertificateLint{ | |||
| LintMetadata: lint.LintMetadata{ | |||
| Name: "w_smime_legacy_aia_contains_internal_names", | |||
| Name: "e_smime_legacy_aia_shall_have_one_http", | |||
There was a problem hiding this comment.
Good catch on the change from w to e. It really should have been marked as an error class in the first place.
the effective date of this lint has been updated and in checkApplies a check to see whether the extension is present has been added.
Also, another proposal would be to break this lint into two lints. One that is checking whether the scheme is HTTP and produces either error or pass and a second lint that is checking the internal names and produces either warn or pass. I believe that most lints try to focus on one aspect and not to lint many aspects in one lint. I would be willing to contribute on this.
Best,
Vangelis