-
Notifications
You must be signed in to change notification settings - Fork 110
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
Lint for CABF SMIME 7.1.2.3.h - subjectAlternativeName, all: SHALL be present (7.1.2.3.h) #744
Conversation
… present (7.1.2.3.h)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need some changes for this PR and #747 re: checkApplies+CertificateLint.Execute.
} | ||
|
||
func (l *SubjectAlternativeNameShallBePresent) CheckApplies(c *x509.Certificate) bool { | ||
return util.IsSubscriberCert(c) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, there's a bit of a circular dependency on when this lint might apply. Currently, this lint applies to subscriber certificates with emailProtection EKU. However, by section 1.1 of the SMIME BRs, a certificate isn't an SMIME certificate if it merely contains emailProtection and no subjectAltName extension. So, by failing this lint, it means that it shouldn't apply...
Relevant quote: "An S/MIME Certificate for the purposes of this document can be identified by the existence of an Extended Key Usage (EKU) for id-kp-emailProtection (OID: 1.3.6.1.5.5.7.3.4) and the inclusion of a rfc822Name or an otherName of type id-on-SmtpUTF8Mailbox in the subjectAltName extension."
I guess one option is to refactor lint.base.go to remove the need for emailProtection on SMIME lints or to instead change it to require either (emailProtection AND a SAN extension) OR an SMIME BR policy OID and returning N/A whenever neither of those conditions are met.
It's occurred to me that my #747 PR probably has the same issue as I've a lint requiring emailProtection but if it's not present then the lint would be N/A. For simplicities sake, let's keep the discussion about this here and I'll put a comment on my PR to point to this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this to be a separate issue and I have begun tracking it at #748.
That said, I am inclined to move forward with merging this (and other lints) as it implements 7.1.2.3.h
quite precisely
subjectAlternativeName, all: SHALL be present
Any separate issues with regard to the general applicability of SMIME certificates from within the framework is out of scope of this change.
Resolves #743. Addresses
subjectAlternativeName, all: SHALL be present (7.1.2.3.h)
from #712