Mailbox addresses from san for all br

[mtgag]

This PR is a refactor for e_mailbox_address_shall_contain_an_rfc822_name to support not only mailbox validated certificates but all SMIME BR certificates. First change is to lint only BR certificates and not any certificate with email protection or anyExtendedKeyUsage because the lint should apply only to BR certificates. The second change is to lint all types of SMIME BR certificates because the may contain a mailbox address in the CN. The implementation already checks whether this is a well-formed email address and we can re-use this to filter out CN entries that are not email addresses. To test the implementation two new sponsor validated certificates have been added and a sponsor validated policy identifier has been introduced in two existent certificates.

[mtgag]

Also a minor typo in lint_ext_subject_key_identifier_not_recommended_subscriber.go has been corrected.

[toddgaunt-gs]

Nice catch, I can't believe the wrong check applies function got past all the reviews! This happened because when I was porting the lint we had the IsSMIMEBRCertificate function in a different package so it was modified and I think I selected the wrong one.

[vanbroup]

This lint now triggers where the CN is "AAA@123BBB" which is not an RFC822 mailbox.

The go function mail.ParseAddress will parse this address:
https://go.dev/play/p/8PDWhXDeFvR

The subject would look something like:

..., CN=AAA@123BBB/[email protected]

Maybe the lint should also check if the emailAddress value is present in the SAN.

[vanbroup]

Reconsidering, "AAA@123BBB" seems to be a valid RFC822 mailbox, but the linter should probably look at the subject:emailAddress value instead of the subject:commonName in the case of the Sponsor-validated or Individual-validated certificate profile.

...

https://cabforum.org/working-groups/smime/requirements/#71422-subject-distinguished-name-fields

[mtgag]

Reconsidering, "AAA@123BBB" seems to be a valid RFC822 mailbox, but the linter should probably look at the subject:emailAddress value instead of the subject:commonName in the case of the Sponsor-validated or Individual-validated certificate profile.

https://cabforum.org/working-groups/smime/requirements/#71422-subject-distinguished-name-fields

The implementation extracts mailbox addesses from CN only if it is a well-formed mailbox address:

	if util.IsMailboxAddress(commonName) {
		mailboxAddresses = append(mailboxAddresses, commonName)
	}

Then it operates only on the extracted mailbox addresses. For example, in a sponsor-vaildated certificate with a personal name in CN the personal name is not added to mailboxAddresses, while if the CN contains a mailbox address, this is added and checked later. In all four profiles a mailbox address is allowed in CN. This implementation catches the OR of the specification.

On the other hand your comment gives an alternative look at the lint. Imagine a sponsor-validated certificate with a personal name, as subject:commonName and neither subject:emailAddress nor dirName. I guess the most appropriate result of the lint should be NA, however the current implementation would result into a Pass. This means that the checkApplies method should fill the toFindMailboxAddresses and if this list is not empty then return true combined with the other checks that it already performs.

Should we go for it?

[vanbroup]

The implementation extracts mailbox addesses from CN only if it is a well-formed mailbox address:

	if util.IsMailboxAddress(commonName) {
		mailboxAddresses = append(mailboxAddresses, commonName)
	}

The problem is that this method will catch any value that contains an @ sign within the text, for example in a Pseudonym, which can either be a unique identifier selected by the CA for the Subject of the Certificate, or an identifier selected by the Enterprise RA which uniquely identifies the Subject of the Certificate within the Organization included in the subject:organizationName attribute.

So we could compare the subject:pseudonym with the subject:commonName if the subject:pseudonym is present, but its explicit presence is not a requirement.

Then it operates only on the extracted mailbox addresses. For example, in a sponsor-vaildated certificate with a personal name in CN the personal name is not added to mailboxAddresses, while if the CN contains a mailbox address, this is added and checked later. In all four profiles a mailbox address is allowed in CN. This implementation catches the OR of the specification.

Should we consider ignoring the subject:commonName if there is a subject:emailAddress which does not match the subject:commonName?

On the other hand your comment gives an alternative look at the lint. Imagine a sponsor-validated certificate with a personal name, as subject:commonName and neither subject:emailAddress nor dirName. I guess the most appropriate result of the lint should be NA, however the current implementation would result into a Pass. This means that the checkApplies method should fill the toFindMailboxAddresses and if this list is not empty then return true combined with the other checks that it already performs.

It would make sense to mark this as a NA in that case.

[mtgag]

I see the point.... it boils down to that we cannot reliably and unambiguously identify mailbox addresses in certificates other than mailbox-validated. Therefore, the most appropriate solution would be to check only for mailbox-validated (as in the first implementation) to avoid false positives, leaving on the other hand space to not lint other profiles that do contain mailbox addresses. To avoid false positives I will change it back to check only for mailbox-validated. Is this fine?

[vanbroup]

What if we would just ignore the subject:commonName if there is a subject:emailAddress? This seems like a solution that is not overly restrictive and would be better than just validating the mailbox type.

[mtgag]

What if we would just ignore the subject:commonName if there is a subject:emailAddress? This seems like a solution that is not overly restrictive and would be better than just validating the mailbox type.

sounds good. Also, if it is mailbox-validated then also consider subject:commonName. I will address the discussion here in a new PR.

[mtgag]

@vanbroup would it be possible to briefly take a look into PR #825 (it contains some other commits too, please ignore them, only the mailbox_address_from_san.go are relevant), whether it addresses the discussion here?