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

Add lints for S/MIME BR 7.1.2.3l #805

Merged
merged 5 commits into from
Mar 24, 2024
Merged

Conversation

bitlux
Copy link
Contributor

@bitlux bitlux commented Feb 27, 2024

This PR adds lints for S/MIME BR 7.1.2.3l legal entity identifier.

This PR also organizes the functions in smime_policies.go to group them by validation type and cert type.

Copy link
Member

@christopher-henderson christopher-henderson left a comment

Choose a reason for hiding this comment

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

Some help with legibility and one possible discussion regarding the presence of an LEI role for an organization validated certificate.

return util.IsSubscriberCert(c) && util.IsSMIMEBRCertificate(c)
}

func (l *legalEntityIdentifier) Execute(c *x509.Certificate) *lint.LintResult {

Choose a reason for hiding this comment

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

If you are agreeable, then I believe that moving some of these util calls to named variables will help with legibility.

func (l *legalEntityIdentifier) Execute(c *x509.Certificate) *lint.LintResult {
	lei := util.GetExtFromCert(c, util.LegalEntityIdentifierOID)
	lei_present := util.IsExtInCert(c, util.LegalEntityIdentifierOID)
	lei_role := util.GetExtFromCert(c, util.LegalEntityIdentifierRoleOID)
	lei_role_present := util.IsExtInCert(c, util.LegalEntityIdentifierRoleOID)
	switch {
	case util.IsMailboxValidatedCertificate(c), util.IsIndividualValidatedCertificate(c):
		if lei_present {
			// Mailbox-validated and Individualvalidated prohibited.
			return &lint.LintResult{Status: lint.Error, Details: "Legal Entity Identifier extension present"}
		}
	case util.IsOrganizationValidatedCertificate(c):
		if lei_present && lei.Critical {
			// LEI (1.3.6.1.4.1.52266.1) MAY be present and SHALL NOT be marked critical.
			return &lint.LintResult{Status: lint.Error, Details: "Legal Entity Identifier extension resent and critical"}
		}
		if lei_role_present {
			// This is affirming the negative. Sponsor validated certificates MAY have an LEI Role, so
			// it is being taken here that not explicitly as such for organization validated certificates
			// implies that they are not allowed.
			return &lint.LintResult{Status: lint.Error, Details: "Legal Entity Identifier Role extension present"}
		}
	case util.IsSponsorValidatedCertificate(c):
		if lei_present && lei.Critical {
			// LEI (1.3.6.1.4.1.52266.1) MAY be present and SHALL NOT be marked critical.
			return &lint.LintResult{Status: lint.Error, Details: "Legal Entity Identifier extension present and critical"}
		}
		if lei_role_present && lei_role.Critical {
			// LEI Role (1.3.6.1.4.1.52266.2) MAY be present and SHALL NOT be marked critical.
			return &lint.LintResult{Status: lint.Error, Details: "Legal Entity Identifier Role extension present and critical"}
		}
	default:
		return &lint.LintResult{Status: lint.Error, Details: "Unknown validation type"}
	}

	return &lint.LintResult{Status: lint.Pass}
}

Note this comment in particular,

This is affirming the negative. Sponsor validated certificates MAY have an LEI Role, so it is being taken here that not explicitly as such for organization validated certificates implies that they are not allowed.

That is, the lack of a MAY is not necessarily the presence of a MUST NOT. This is, unfortunately, ambiguity in the original source text. This is technically a logical fallacy, although I am somewhat inclined to believe that it was the original intent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review. I agree with your assessment of the fallacy in the organization-validated certificate logic.

@christopher-henderson christopher-henderson merged commit 990a074 into zmap:master Mar 24, 2024
4 checks passed
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.

2 participants