-
Notifications
You must be signed in to change notification settings - Fork 109
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
CABF SMIME BR 7.1.2.3.m - Adobe Extensions #763
CABF SMIME BR 7.1.2.3.m - Adobe Extensions #763
Conversation
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 this is fine and the only real change needed should be no trouble.
) | ||
|
||
func init() { | ||
lint.RegisterLint(&lint.Lint{ |
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, the lint.Lint type is actually deprecated so this should be creating a lint.CertificateLint which is fairly similar but packages most of the fields that aren't the Lint into a LintMetadata struct. Same comment applies on the other lint too.
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.
addressed
v3/util/oid.go
Outdated
@@ -24,6 +24,8 @@ import ( | |||
|
|||
var ( | |||
//extension OIDs | |||
AdobeTimestampOID = asn1.ObjectIdentifier{1, 2, 840, 113583, 1, 1, 9, 1} // Adobe Timestamp x509 extension |
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.
Is the correct formatting "timestamp" or "time-stamp". RFC3161 has it as "Time-stamp" and most online references I can find follow this trend although I've not found any record of Adobe actually defining what their OID should be called.
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.
addressed
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.
Really, only a couple of minor nits to address in this one.
return &adobeExtensionsLegacyMultipurposeCriticality{} | ||
} | ||
|
||
// CheckApplies returns true if the certificate's policies assert that it conforms to the multipurpose or legacy policy requirements defined in the SMIME BRs |
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.
This comment doesn't mention the extra requirement that the certificate must be a subscriber certificate
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.
addressed
v3/lints/cabf_smime_br/lint_adobe_extensions_legacy_multipurpose_criticality.go
Outdated
Show resolved
Hide resolved
v3/lints/cabf_smime_br/lint_adobe_extensions_legacy_multipurpose_criticality.go
Outdated
Show resolved
Hide resolved
v3/lints/cabf_smime_br/lint_adobe_extensions_strict_presence.go
Outdated
Show resolved
Hide resolved
…se_criticality.go Co-authored-by: Rob <3725956+robplee@users.noreply.github.com>
Co-authored-by: Rob <3725956+robplee@users.noreply.github.com>
…se_criticality.go Co-authored-by: Rob <3725956+robplee@users.noreply.github.com>
Adding lints to check the presence and criticality of the Adobe x509 Extensions to cover SMIME BR 7.1.2.3.m: