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

CABF SMIME BR 7.1.2.3.e - KeyUsages #757

Merged
merged 13 commits into from
Nov 5, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions v3/lints/cabf_smime_br/lint_ecpublickey_key_usages.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* ZLint Copyright 2023 Regents of the University of Michigan
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy
* of the License at http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
* implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package cabf_smime_br

import (
"github.com/zmap/zcrypto/x509"
"github.com/zmap/zlint/v3/lint"
"github.com/zmap/zlint/v3/util"
)

func init() {
lint.RegisterLint(&lint.Lint{
Name: "e_ecpublickey_key_usages",
Description: "For signing only, bit positions SHALL be set for digitalSignature and MAY be set for nonRepudiation. For key management only, bit positions SHALL be set for keyEncipherment.For dual use, bit positions SHALL be set for digitalSignature and keyEncipherment and MAY be set for nonRepudiation.",
Citation: "7.1.2.3.e",
Source: lint.CABFSMIMEBaselineRequirements,
EffectiveDate: util.CABF_SMIME_BRs_1_0_0_Date,
Lint: NewECPublicKeyKeyUsages,
})
}

type ecPublicKeyKeyUsages struct{}

func NewECPublicKeyKeyUsages() lint.LintInterface {
return &ecPublicKeyKeyUsages{}
}

func (l *ecPublicKeyKeyUsages) CheckApplies(c *x509.Certificate) bool {
return util.IsSubscriberCert(c) && util.IsSMIMEBRCertificate(c) && util.IsExtInCert(c, util.KeyUsageOID) && c.PublicKeyAlgorithm == x509.ECDSA
}

func (l *ecPublicKeyKeyUsages) Execute(c *x509.Certificate) *lint.LintResult {
robplee marked this conversation as resolved.
Show resolved Hide resolved
certType := 0
if c.KeyUsage&x509.KeyUsageDigitalSignature != 0 {
certType |= 1
}
if c.KeyUsage&x509.KeyUsageKeyAgreement != 0 {
certType |= 2
}

switch certType {
case 1:
// signing only
mask := 0x1FF ^ (x509.KeyUsageDigitalSignature | x509.KeyUsageContentCommitment)
if c.KeyUsage&mask != 0 {
return &lint.LintResult{Status: lint.Error}
}
case 2:
// key management only
mask := 0x1FF ^ (x509.KeyUsageKeyAgreement | x509.KeyUsageEncipherOnly | x509.KeyUsageDecipherOnly)
if c.KeyUsage&mask != 0 {
return &lint.LintResult{Status: lint.Error}
}
case 3:
// dual use
mask := 0x1FF ^ (x509.KeyUsageDigitalSignature | x509.KeyUsageContentCommitment | x509.KeyUsageKeyAgreement | x509.KeyUsageEncipherOnly | x509.KeyUsageDecipherOnly)
Copy link

Choose a reason for hiding this comment

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

Requirement: "For signing only, bit positions SHALL be set for digitalSignature and MAY be set for nonRepudiation.
For key management only, bit positions SHALL be set for keyAgreement and MAY be set for encipherOnly or decipherOnly. For dual use, bit positions SHALL be set for digitalSignature and keyAgreement and MAY be set for nonRepudiation and for encipherOnly or decipherOnly (only if keyAgreement is set)."

My understanding is that OR is either encipher or decipher, but not both. I think the current code permits the combination of both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think the text is written in a way that makes your interpretation the only acceptable one. Generally speaking "or" means a&&!b, !a&&b, a&&b. I don't think the BRs support the narrower interpretation you're putting on them

Copy link

Choose a reason for hiding this comment

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

Thanks, I agree it's ambiguous - I'll raise this topic in the working group for clarification.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any usecase for doing so however?

The purpose of encipherOnly and decipherOnly is to limit the usage of the key. The absense of both, allows for both options to be used. As such, adding both, would be redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm a bit surprised that it's not written unambiguously anywhere that a cert can't have both EncipherOnly and DecipherOnly KUs. While writing my comment I checked RFC5280 and that doesn't actually say that it's not allowable to have both at the same time which I'm a little surprised by...

if c.KeyUsage&mask != 0 {
return &lint.LintResult{Status: lint.Error}
}
default:
return &lint.LintResult{Status: lint.Error}
}

return &lint.LintResult{Status: lint.Pass}
}
89 changes: 89 additions & 0 deletions v3/lints/cabf_smime_br/lint_ecpublickey_key_usages_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package cabf_smime_br

import (
"testing"

"github.com/zmap/zlint/v3/lint"
"github.com/zmap/zlint/v3/test"
)

func TestECPublicKeyKeyUsage(t *testing.T) {
testCases := []struct {
Name string
InputFilename string
ExpectedResult lint.LintStatus
}{
{
Name: "pass - cert with digitalSignature KU",
InputFilename: "smime/ec_legacy_digital_signature_ku.pem",
ExpectedResult: lint.Pass,
},
{
Name: "pass - cert with digitalSignature and contentCommitment KUs",
InputFilename: "smime/ec_multipurpose_digital_signature_content_commitment_ku.pem",
ExpectedResult: lint.Pass,
},
{
Name: "pass - cert with keyAgreement KU",
InputFilename: "smime/ec_strict_key_agreement_ku.pem",
ExpectedResult: lint.Pass,
},
{
Name: "pass - cert with keyAgreement and encipherOnly KUs",
InputFilename: "smime/ec_legacy_key_agreement_encipher_only_ku.pem",
ExpectedResult: lint.Pass,
},
{
Name: "pass - cert with keyAgreement and decipherOnly KUs",
InputFilename: "smime/ec_multipurpose_key_agreement_decipher_only.pem",
ExpectedResult: lint.Pass,
},
{
Name: "pass - cert with digitalSignature, keyAgreement, contentCommitment, and encipherOnly KUs",
InputFilename: "smime/ec_strict_digital_signature_key_agreement_content_commitment_encipher_only_ku.pem",
ExpectedResult: lint.Pass,
},
{
Name: "pass - cert with digitalSignature, keyAgreement, contentCommitment, and decipherOnly KUs",
InputFilename: "smime/ec_legacy_digital_signature_key_agreement_content_commitment_decipher_only_ku.pem",
ExpectedResult: lint.Pass,
}, {
Name: "NA - cert without KUs",
InputFilename: "smime/without_subject_alternative_name.pem",
ExpectedResult: lint.NA,
},
{
Name: "NE - certificate with valid KUs dated before 2020-09-01",
InputFilename: "smime/ec_multipurpose_valid_ku_august_2023.pem",
ExpectedResult: lint.NE,
},
{
Name: "Error - Signing Certificate with unexpected KU",
InputFilename: "smime/ec_strict_digital_signature_cert_sign_ku.pem",
ExpectedResult: lint.Error,
},
{
Name: "Error - Key Management Certificate with unexpected KU",
InputFilename: "smime/ec_legacy_key_agreement_cert_sign_ku.pem",
ExpectedResult: lint.Error,
},
{
Name: "Error - Dual Use Certificate with unexpected KU",
InputFilename: "smime/ec_multipurpose_digital_signature_key_agreement_cert_sign_ku.pem",
ExpectedResult: lint.Error,
},
{
Name: "Error - Certificate without digitalSignature or keyAgreement KUs",
InputFilename: "smime/ec_strict_cert_sign_ku.pem",
ExpectedResult: lint.Error,
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
result := test.TestLint("e_ecpublickey_key_usages", tc.InputFilename)
if result.Status != tc.ExpectedResult {
t.Errorf("expected result %v was %v - details: %v", tc.ExpectedResult, result.Status, result.Details)
}
})
}
}
56 changes: 56 additions & 0 deletions v3/lints/cabf_smime_br/lint_edwardspublickey_key_usages.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* ZLint Copyright 2023 Regents of the University of Michigan
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy
* of the License at http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
* implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package cabf_smime_br

import (
"github.com/zmap/zcrypto/x509"
"github.com/zmap/zlint/v3/lint"
"github.com/zmap/zlint/v3/util"
)

func init() {
lint.RegisterLint(&lint.Lint{
Name: "e_edwardspublickey_key_usages",
Description: "Bit positions SHALL be set for digitalSignature and MAY be set for nonRepudiation.",
Citation: "7.1.2.3.e",
Source: lint.CABFSMIMEBaselineRequirements,
EffectiveDate: util.CABF_SMIME_BRs_1_0_0_Date,
Lint: NewEdwardsPublicKeyKeyUsages,
})
}

type edwardsPublicKeyKeyUsages struct{}

func NewEdwardsPublicKeyKeyUsages() lint.LintInterface {
return &edwardsPublicKeyKeyUsages{}
}

func (l *edwardsPublicKeyKeyUsages) CheckApplies(c *x509.Certificate) bool {
// TODO add support for curve448 certificate linting
return util.IsSubscriberCert(c) && util.IsSMIMEBRCertificate(c) && util.IsExtInCert(c, util.KeyUsageOID) && c.PublicKeyAlgorithm == x509.Ed25519
}

func (l *edwardsPublicKeyKeyUsages) Execute(c *x509.Certificate) *lint.LintResult {
if c.KeyUsage&x509.KeyUsageDigitalSignature == 0 {
return &lint.LintResult{Status: lint.Error}
}

mask := 0x1FF ^ (x509.KeyUsageDigitalSignature | x509.KeyUsageContentCommitment)
robplee marked this conversation as resolved.
Show resolved Hide resolved
if c.KeyUsage&mask != 0 {
return &lint.LintResult{Status: lint.Error}
}

return &lint.LintResult{Status: lint.Pass}
}
55 changes: 55 additions & 0 deletions v3/lints/cabf_smime_br/lint_edwardspublickey_key_usages_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package cabf_smime_br

import (
"testing"

"github.com/zmap/zlint/v3/lint"
"github.com/zmap/zlint/v3/test"
)

func TestEdwardsPublicKeyKeyUsages(t *testing.T) {
testCases := []struct {
Name string
InputFilename string
ExpectedResult lint.LintStatus
}{
{
Name: "pass - cert with digitalSignature KU",
InputFilename: "smime/ed25519_legacy_digital_signature_ku.pem",
ExpectedResult: lint.Pass,
},
{
Name: "pass - cert with digitalSignature and contentCommitment KUs",
InputFilename: "smime/ed25519_multipurpose_digital_signature_content_commitment_ku.pem",
ExpectedResult: lint.Pass,
},
{
Name: "NA - non-SMIME BR cert",
InputFilename: "smime/domainValidatedWithEmailCommonName.pem",
ExpectedResult: lint.NA,
},
{
Name: "NA - RSA cert",
InputFilename: "smime/rsa_strict_digital_signature_ku.pem",
ExpectedResult: lint.NA,
},
{
Name: "NE - certificate with KU extension dated before 2020-09-01",
InputFilename: "smime/ed25519_strict_valid_ku_august_2023.pem",
ExpectedResult: lint.NE,
},
{
Name: "Error - Certificate without digitalSignature KU",
InputFilename: "smime/ed25519_strict_cert_sign_ku.pem",
ExpectedResult: lint.Error,
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
result := test.TestLint("e_edwardspublickey_key_usages", tc.InputFilename)
if result.Status != tc.ExpectedResult {
t.Errorf("expected result %v was %v - details: %v", tc.ExpectedResult, result.Status, result.Details)
}
})
}
}
52 changes: 52 additions & 0 deletions v3/lints/cabf_smime_br/lint_key_usage_criticality.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* ZLint Copyright 2023 Regents of the University of Michigan
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy
* of the License at http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
* implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package cabf_smime_br

import (
"github.com/zmap/zcrypto/x509"
"github.com/zmap/zlint/v3/lint"
"github.com/zmap/zlint/v3/util"
)

func init() {
lint.RegisterLint(&lint.Lint{
Name: "w_key_usage_criticality",
Description: "keyUsage... This extension SHOULD be marked critical",
Citation: "7.1.2.3.e",
Source: lint.CABFSMIMEBaselineRequirements,
EffectiveDate: util.CABF_SMIME_BRs_1_0_0_Date,
Lint: NewKeyUsageCriticality,
})
}

type keyUsageCriticality struct{}

func NewKeyUsageCriticality() lint.LintInterface {
return &keyUsageCriticality{}
}

func (l *keyUsageCriticality) CheckApplies(c *x509.Certificate) bool {
return util.IsSubscriberCert(c) && util.IsSMIMEBRCertificate(c) && util.IsExtInCert(c, util.KeyUsageOID)

}

func (l *keyUsageCriticality) Execute(c *x509.Certificate) *lint.LintResult {
kuExt := util.GetExtFromCert(c, util.KeyUsageOID)
if !kuExt.Critical {
return &lint.LintResult{Status: lint.Warn}
}

return &lint.LintResult{Status: lint.Pass}
}
45 changes: 45 additions & 0 deletions v3/lints/cabf_smime_br/lint_key_usage_criticality_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package cabf_smime_br

import (
"testing"

"github.com/zmap/zlint/v3/lint"
"github.com/zmap/zlint/v3/test"
)

func TestKeyUsageCriticality(t *testing.T) {
testCases := []struct {
Name string
InputFilename string
ExpectedResult lint.LintStatus
}{
{
Name: "pass - cert with critical KU extension",
InputFilename: "smime/rsa_strict_digital_signature_ku.pem",
ExpectedResult: lint.Pass,
},
{
Name: "NA - non-SMIME BR cert",
InputFilename: "smime/domainValidatedWithEmailCommonName.pem",
ExpectedResult: lint.NA,
},
{
Name: "NE - certificate with KU extension dated before 2020-09-01",
InputFilename: "smime/rsa_strict_valid_ku_august_2023.pem",
ExpectedResult: lint.NE,
},
{
Name: "Warn - certificate with non-critical KU extension",
InputFilename: "smime/with_non_critical_ku_extension.pem",
ExpectedResult: lint.Warn,
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
result := test.TestLint("w_key_usage_criticality", tc.InputFilename)
if result.Status != tc.ExpectedResult {
t.Errorf("expected result %v was %v - details: %v", tc.ExpectedResult, result.Status, result.Details)
}
})
}
}
Loading