From 86bcc674785a38a8bc72dcc306ee5a9572c3c0fb Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Fri, 3 Jan 2020 13:07:16 -0500 Subject: [PATCH] Misc. cleanups, unit test for finding leftover template bits. (#340) * tests: remove gofmt_test.go The golangci-lint pass run in CI includes an equivalent test. If folks want to test for unformatted code locally install the linter and run `golangci-lint run` in the root directory. This will flag findings above and beyond `gofmt` problems ahead of CI failing. * lints: remove commented out code. In three cases, remove a comment ahead of a return that added no useful context. In `lints/community/lint_rsa_exp_negative_test.go` remove a commented out test case for a negative RSA exponent. The test code doesn't build as-is and the referenced test cert (`rsaExpNegative.pem`) doesn't exist in-tree. A TODO is left to indicate there's missing test coverage for later follow-up. * lints: fix "certtificate" comment typo. * lints: fix tabs in ref text for lint_sub_cert_or_sub_ca_using_sha1. * lints: fix field name ref. in lint Descriptions. These two lints mistakenly said in their `Description` that they only check the `DNSNames` field of the certificate when in fact they only check the `IANDNSNames` field. There are two corresponding lints (`lints/community/lint_san_wildcard_not_first.go` and `lints/community/lint_san_bare_wildcard.go`) that check `DNSNames`. * lints: add slice of known LintSources, test for templating leftovers. There should never be finished lint source code that contains template text intended to be replaced by the programmer. A new `TestLeftoverTemplates` unit test is added to make sure we enforce this during CI to lessen the burden on code reviewers to catch this problem. * tests: use full path in TestLeftoverTemplates errs * lints: fix TestLeftoverTemplates findings Prior to these fixes all of the modified files had templating leftovers: ``` === RUN TestLeftoverTemplates --- FAIL: TestLeftoverTemplates (0.01s) template_test.go:49: Lint "cabf_br/lint_root_ca_extended_key_usage_present.go" contains template leftover "// Add actual lint here" template_test.go:49: Lint "cabf_br/lint_root_ca_key_usage_present.go" contains template leftover "// Add actual lint here" template_test.go:49: Lint "cabf_br/lint_sub_cert_cert_policy_empty.go" contains template leftover "// Add actual lint here" template_test.go:49: Lint "cabf_br/lint_sub_cert_certificate_policies_missing.go" contains template leftover "// Add actual lint here" template_test.go:49: Lint "cabf_br/lint_sub_cert_crl_distribution_points_does_not_contain_url.go" contains template leftover "// Add actual lint here" template_test.go:49: Lint "cabf_br/lint_sub_cert_eku_extra_values.go" contains template leftover "// Add actual lint here" template_test.go:49: Lint "cabf_br/lint_sub_cert_eku_missing.go" contains template leftover "// Add actual lint here" template_test.go:49: Lint "cabf_br/lint_sub_cert_eku_server_auth_client_auth_missing.go" contains template leftover "// Add actual lint here" template_test.go:49: Lint "cabf_br/lint_sub_cert_key_usage_cert_sign_bit_set.go" contains template leftover "// Add actual lint here" template_test.go:49: Lint "cabf_br/lint_sub_cert_key_usage_crl_sign_bit_set.go" contains template leftover "// Add actual lint here" template_test.go:49: Lint "rfc/lint_basic_constraints_not_critical.go" contains template leftover "// Add actual lint here" template_test.go:49: Lint "rfc/lint_ext_key_usage_not_critical.go" contains template leftover "// Add actual lint here" template_test.go:49: Lint "rfc/lint_basic_constraints_not_critical.go" contains template leftover "// Add actual lint here" template_test.go:49: Lint "rfc/lint_ext_key_usage_not_critical.go" contains template leftover "// Add actual lint here" template_test.go:49: Lint "rfc/lint_basic_constraints_not_critical.go" contains template leftover "// Add actual lint here" template_test.go:49: Lint "rfc/lint_ext_key_usage_not_critical.go" contains template leftover "// Add actual lint here" FAIL FAIL command-line-arguments 0.017s FAIL ``` * lints: update template test with another string, fix occurrences. ``` === RUN TestLeftoverTemplates --- FAIL: TestLeftoverTemplates (0.01s) template_test.go:50: Lint "cabf_br/lint_sub_ca_name_constraints_not_critical.go" contains template leftover "Change this to match source TEXT" template_test.go:50: Lint "community/lint_validity_time_not_positive.go" contains template leftover "Change this to match source TEXT" template_test.go:50: Lint "community/lint_validity_time_not_positive.go" contains template leftover "Change this to match source TEXT" FAIL FAIL command-line-arguments 0.017s FAIL ``` * lints: move lint_ian_bare_wildcard.go from RFC to community. It cites RFC 5280 but that RFC doesn't prescribe any semantics to the use of wildcards in DNSNames or elsewhere. I suspect this lint actually came from AWSLabs, similar to `lint_ian_wildcard_not_first.go` and `lint_san_bare_wildcard.go`, both of which are already in `lints/community/`. * lints: fix moved lint_ian_bare_wildcard.go source/category/package * lints: fix off-by-one in RFC max length lint Descs. The upper bounds being enforced against in the changed lints are inclusive. The lint tests were doing the right thing but the descriptions incorrectly described the boundary as if it were exclusive. For comparison the following lints already did the right thing already and had the UB+1 in the desc: ``` lints/rfc/lint_subject_given_name_max_length.go lints/rfc/lint_subject_postal_code_max_length.go lints/rfc/lint_subject_street_address_max_length.go lints/rfc/lint_subject_surname_max_length.go ``` * lint: revert accidental whitespace diff --- gofmt_test.go | 40 ------------- lint/base.go | 34 +++++++++++ .../lint_invalid_certificate_version.go | 1 - ...old_root_ca_rsa_mod_less_than_2048_bits.go | 4 -- ...lint_root_ca_extended_key_usage_present.go | 1 - .../cabf_br/lint_root_ca_key_usage_present.go | 1 - .../lint_rsa_mod_less_than_2048_bits.go | 4 -- ...nt_sub_ca_name_constraints_not_critical.go | 9 ++- .../lint_sub_cert_cert_policy_empty.go | 1 - ...t_sub_cert_certificate_policies_missing.go | 1 - ...istribution_points_does_not_contain_url.go | 1 - .../cabf_br/lint_sub_cert_eku_extra_values.go | 1 - lints/cabf_br/lint_sub_cert_eku_missing.go | 1 - ...ert_eku_server_auth_client_auth_missing.go | 1 - ...nt_sub_cert_key_usage_cert_sign_bit_set.go | 1 - ...int_sub_cert_key_usage_crl_sign_bit_set.go | 1 - .../lint_sub_cert_or_sub_ca_using_sha1.go | 2 +- .../lint_ian_bare_wildcard.go | 8 +-- .../lint_ian_bare_wildcard_test.go | 2 +- .../community/lint_ian_wildcard_not_first.go | 2 +- lints/community/lint_rsa_exp_negative_test.go | 13 +---- .../lint_validity_time_not_positive.go | 4 -- .../lint_basic_constraints_not_critical.go | 1 - .../lint_cert_contains_unique_identifier.go | 2 +- ...lint_ext_key_usage_cert_sign_without_ca.go | 2 +- lints/rfc/lint_ext_key_usage_not_critical.go | 1 - .../lint_inhibit_any_policy_not_critical.go | 2 +- .../lint_subject_common_name_max_length.go | 2 +- ...int_subject_dn_serial_number_max_length.go | 2 +- lints/rfc/lint_subject_email_max_length.go | 2 +- .../lint_subject_locality_name_max_length.go | 2 +- ...nt_subject_organization_name_max_length.go | 2 +- ...ect_organizational_unit_name_max_length.go | 2 +- .../rfc/lint_subject_state_name_max_length.go | 2 +- lints/template_test.go | 56 +++++++++++++++++++ 35 files changed, 116 insertions(+), 95 deletions(-) delete mode 100644 gofmt_test.go rename lints/{rfc => community}/lint_ian_bare_wildcard.go (91%) rename lints/{rfc => community}/lint_ian_bare_wildcard_test.go (98%) create mode 100644 lints/template_test.go diff --git a/gofmt_test.go b/gofmt_test.go deleted file mode 100644 index 6fd52f14a..000000000 --- a/gofmt_test.go +++ /dev/null @@ -1,40 +0,0 @@ -/* - * ZLint Copyright 2020 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 zlint - -import ( - "bytes" - "os/exec" - "testing" -) - -func TestGofmt(t *testing.T) { - globs := []string{ - "*.go", - "cmd/*.go", - "lints/*.go", - "util/*.go", - } - for _, glob := range globs { - gofmtCmd := "gofmt -s -l " + glob - cmd := exec.Command("/bin/sh", "-c", gofmtCmd) - var out bytes.Buffer - cmd.Stdout = &out - _ = cmd.Run() - if out.String() != "" { - t.Errorf("glob %s not gofmt'ed", glob) - } - } -} diff --git a/lint/base.go b/lint/base.go index d886b85d1..608b82dfd 100644 --- a/lint/base.go +++ b/lint/base.go @@ -46,6 +46,8 @@ type LintInterface interface { // An Enum to programmatically represent the source of a lint type LintSource int +// NOTE(@cpu): If you are adding a new LintSource make sure you have considered +// updating the Directory() function. const ( UnknownLintSource LintSource = iota CABFBaselineRequirements @@ -59,6 +61,38 @@ const ( AppleCTPolicy // https://support.apple.com/en-us/HT205280 ) +// LintSources contains a list of the valid lint sources we expect to be used +// by ZLint lints. +var LintSources = []LintSource{ + CABFBaselineRequirements, + CABFEVGuidelines, + RFC5280, + RFC5480, + RFC5891, + AppleCTPolicy, + EtsiEsi, + ZLint, + AWSLabs, +} + +// Directory returns the directory name in `lints/` for the LintSource. +func (l LintSource) Directory() string { + switch l { + case CABFBaselineRequirements: + return "cabf_br" + case CABFEVGuidelines: + return "cabf_ev" + case RFC5280, RFC5480, RFC5891: + return "rfc" + case AppleCTPolicy: + return "apple" + case EtsiEsi: + return "etsi" + default: + return "community" + } +} + // A Lint struct represents a single lint, e.g. // "e_basic_constraints_not_critical". It contains an implementation of LintInterface. type Lint struct { diff --git a/lints/cabf_br/lint_invalid_certificate_version.go b/lints/cabf_br/lint_invalid_certificate_version.go index 585c7d3df..60ccee1bd 100644 --- a/lints/cabf_br/lint_invalid_certificate_version.go +++ b/lints/cabf_br/lint_invalid_certificate_version.go @@ -38,7 +38,6 @@ func (l *InvalidCertificateVersion) Execute(cert *x509.Certificate) *lint.LintRe if cert.Version != 3 { return &lint.LintResult{Status: lint.Error} } - //else return &lint.LintResult{Status: lint.Pass} } diff --git a/lints/cabf_br/lint_old_root_ca_rsa_mod_less_than_2048_bits.go b/lints/cabf_br/lint_old_root_ca_rsa_mod_less_than_2048_bits.go index 853fd185a..2faf765e8 100644 --- a/lints/cabf_br/lint_old_root_ca_rsa_mod_less_than_2048_bits.go +++ b/lints/cabf_br/lint_old_root_ca_rsa_mod_less_than_2048_bits.go @@ -14,10 +14,6 @@ package cabf_br * permissions and limitations under the License. */ -/************************************************ -Change this to match source TEXT -************************************************/ - import ( "crypto/rsa" diff --git a/lints/cabf_br/lint_root_ca_extended_key_usage_present.go b/lints/cabf_br/lint_root_ca_extended_key_usage_present.go index 8752e5fdb..71e471452 100644 --- a/lints/cabf_br/lint_root_ca_extended_key_usage_present.go +++ b/lints/cabf_br/lint_root_ca_extended_key_usage_present.go @@ -36,7 +36,6 @@ func (l *rootCAContainsEKU) CheckApplies(c *x509.Certificate) bool { } func (l *rootCAContainsEKU) Execute(c *x509.Certificate) *lint.LintResult { - // Add actual lint here if util.IsExtInCert(c, util.EkuSynOid) { return &lint.LintResult{Status: lint.Error} } else { diff --git a/lints/cabf_br/lint_root_ca_key_usage_present.go b/lints/cabf_br/lint_root_ca_key_usage_present.go index 749922396..a3a7fb0ca 100644 --- a/lints/cabf_br/lint_root_ca_key_usage_present.go +++ b/lints/cabf_br/lint_root_ca_key_usage_present.go @@ -31,7 +31,6 @@ func (l *rootCAKeyUsagePresent) CheckApplies(c *x509.Certificate) bool { } func (l *rootCAKeyUsagePresent) Execute(c *x509.Certificate) *lint.LintResult { - // Add actual lint here if util.IsExtInCert(c, util.KeyUsageOID) { return &lint.LintResult{Status: lint.Pass} } else { diff --git a/lints/cabf_br/lint_rsa_mod_less_than_2048_bits.go b/lints/cabf_br/lint_rsa_mod_less_than_2048_bits.go index 47e75f4e0..b4872d061 100644 --- a/lints/cabf_br/lint_rsa_mod_less_than_2048_bits.go +++ b/lints/cabf_br/lint_rsa_mod_less_than_2048_bits.go @@ -14,10 +14,6 @@ package cabf_br * permissions and limitations under the License. */ -/************************************************ -Change this to match source TEXT -************************************************/ - import ( "crypto/rsa" diff --git a/lints/cabf_br/lint_sub_ca_name_constraints_not_critical.go b/lints/cabf_br/lint_sub_ca_name_constraints_not_critical.go index 9f91badd8..0d2f829e8 100644 --- a/lints/cabf_br/lint_sub_ca_name_constraints_not_critical.go +++ b/lints/cabf_br/lint_sub_ca_name_constraints_not_critical.go @@ -15,7 +15,14 @@ package cabf_br */ /************************************************ -Change this to match source TEXT +CA Brower Forum Baseline Requirements, Section 7.1.2.2: + + f. nameConstraints (optional) +If present, this extension SHOULD be marked critical*. + +* Non-critical Name Constraints are an exception to RFC 5280 (4.2.1.10), however, they MAY be used until the +Name Constraints extension is supported by Application Software Suppliers whose software is used by a +substantial portion of Relying Parties worldwide ************************************************/ import ( diff --git a/lints/cabf_br/lint_sub_cert_cert_policy_empty.go b/lints/cabf_br/lint_sub_cert_cert_policy_empty.go index 66aed3dcf..12aa2a7a0 100644 --- a/lints/cabf_br/lint_sub_cert_cert_policy_empty.go +++ b/lints/cabf_br/lint_sub_cert_cert_policy_empty.go @@ -39,7 +39,6 @@ func (l *subCertPolicyEmpty) CheckApplies(c *x509.Certificate) bool { } func (l *subCertPolicyEmpty) Execute(c *x509.Certificate) *lint.LintResult { - // Add actual lint here if util.IsExtInCert(c, util.CertPolicyOID) && c.PolicyIdentifiers != nil { return &lint.LintResult{Status: lint.Pass} } else { diff --git a/lints/cabf_br/lint_sub_cert_certificate_policies_missing.go b/lints/cabf_br/lint_sub_cert_certificate_policies_missing.go index 04b16b926..7a5e4a7c7 100644 --- a/lints/cabf_br/lint_sub_cert_certificate_policies_missing.go +++ b/lints/cabf_br/lint_sub_cert_certificate_policies_missing.go @@ -37,7 +37,6 @@ func (l *subCertPolicy) CheckApplies(c *x509.Certificate) bool { } func (l *subCertPolicy) Execute(c *x509.Certificate) *lint.LintResult { - // Add actual lint here if util.IsExtInCert(c, util.CertPolicyOID) { return &lint.LintResult{Status: lint.Pass} } else { diff --git a/lints/cabf_br/lint_sub_cert_crl_distribution_points_does_not_contain_url.go b/lints/cabf_br/lint_sub_cert_crl_distribution_points_does_not_contain_url.go index 69516481f..eeae62215 100644 --- a/lints/cabf_br/lint_sub_cert_crl_distribution_points_does_not_contain_url.go +++ b/lints/cabf_br/lint_sub_cert_crl_distribution_points_does_not_contain_url.go @@ -40,7 +40,6 @@ func (l *subCRLDistNoURL) CheckApplies(c *x509.Certificate) bool { } func (l *subCRLDistNoURL) Execute(c *x509.Certificate) *lint.LintResult { - // Add actual lint here for _, s := range c.CRLDistributionPoints { if strings.HasPrefix(s, "http://") { return &lint.LintResult{Status: lint.Pass} diff --git a/lints/cabf_br/lint_sub_cert_eku_extra_values.go b/lints/cabf_br/lint_sub_cert_eku_extra_values.go index c3ff24ff8..15ae62bbe 100644 --- a/lints/cabf_br/lint_sub_cert_eku_extra_values.go +++ b/lints/cabf_br/lint_sub_cert_eku_extra_values.go @@ -37,7 +37,6 @@ func (l *subExtKeyUsageLegalUsage) CheckApplies(c *x509.Certificate) bool { } func (l *subExtKeyUsageLegalUsage) Execute(c *x509.Certificate) *lint.LintResult { - // Add actual lint here for _, kp := range c.ExtKeyUsage { if kp == x509.ExtKeyUsageServerAuth || kp == x509.ExtKeyUsageClientAuth || diff --git a/lints/cabf_br/lint_sub_cert_eku_missing.go b/lints/cabf_br/lint_sub_cert_eku_missing.go index 81d9e153d..be6ad5c32 100644 --- a/lints/cabf_br/lint_sub_cert_eku_missing.go +++ b/lints/cabf_br/lint_sub_cert_eku_missing.go @@ -37,7 +37,6 @@ func (l *subExtKeyUsage) CheckApplies(c *x509.Certificate) bool { } func (l *subExtKeyUsage) Execute(c *x509.Certificate) *lint.LintResult { - // Add actual lint here if util.IsExtInCert(c, util.EkuSynOid) { return &lint.LintResult{Status: lint.Pass} } else { diff --git a/lints/cabf_br/lint_sub_cert_eku_server_auth_client_auth_missing.go b/lints/cabf_br/lint_sub_cert_eku_server_auth_client_auth_missing.go index bb15c1d9a..8426e3776 100644 --- a/lints/cabf_br/lint_sub_cert_eku_server_auth_client_auth_missing.go +++ b/lints/cabf_br/lint_sub_cert_eku_server_auth_client_auth_missing.go @@ -37,7 +37,6 @@ func (l *subExtKeyUsageClientOrServer) CheckApplies(c *x509.Certificate) bool { } func (l *subExtKeyUsageClientOrServer) Execute(c *x509.Certificate) *lint.LintResult { - // Add actual lint here for _, kp := range c.ExtKeyUsage { if kp == x509.ExtKeyUsageServerAuth || kp == x509.ExtKeyUsageClientAuth { // If we find either of ServerAuth or ClientAuth, lint.Pass diff --git a/lints/cabf_br/lint_sub_cert_key_usage_cert_sign_bit_set.go b/lints/cabf_br/lint_sub_cert_key_usage_cert_sign_bit_set.go index d5686ce09..2b2229fd8 100644 --- a/lints/cabf_br/lint_sub_cert_key_usage_cert_sign_bit_set.go +++ b/lints/cabf_br/lint_sub_cert_key_usage_cert_sign_bit_set.go @@ -37,7 +37,6 @@ func (l *subCertKeyUsageBitSet) CheckApplies(c *x509.Certificate) bool { } func (l *subCertKeyUsageBitSet) Execute(c *x509.Certificate) *lint.LintResult { - // Add actual lint here if (c.KeyUsage & x509.KeyUsageCertSign) == x509.KeyUsageCertSign { return &lint.LintResult{Status: lint.Error} } else { //key usage doesn't allow cert signing or isn't present diff --git a/lints/cabf_br/lint_sub_cert_key_usage_crl_sign_bit_set.go b/lints/cabf_br/lint_sub_cert_key_usage_crl_sign_bit_set.go index 797badca7..030257606 100644 --- a/lints/cabf_br/lint_sub_cert_key_usage_crl_sign_bit_set.go +++ b/lints/cabf_br/lint_sub_cert_key_usage_crl_sign_bit_set.go @@ -37,7 +37,6 @@ func (l *subCrlSignAllowed) CheckApplies(c *x509.Certificate) bool { } func (l *subCrlSignAllowed) Execute(c *x509.Certificate) *lint.LintResult { - // Add actual lint here if (c.KeyUsage & x509.KeyUsageCRLSign) == x509.KeyUsageCRLSign { return &lint.LintResult{Status: lint.Error} } else { //key usage doesn't allow cert signing or isn't present diff --git a/lints/cabf_br/lint_sub_cert_or_sub_ca_using_sha1.go b/lints/cabf_br/lint_sub_cert_or_sub_ca_using_sha1.go index 7aa774f66..869fec78e 100644 --- a/lints/cabf_br/lint_sub_cert_or_sub_ca_using_sha1.go +++ b/lints/cabf_br/lint_sub_cert_or_sub_ca_using_sha1.go @@ -16,7 +16,7 @@ package cabf_br /************************************************************************************************** BRs: 7.1.3 -SHA‐1 MAY be used with RSA keys in accordance with the criteria defined in Section 7.1.3. +SHA‐1 MAY be used with RSA keys in accordance with the criteria defined in Section 7.1.3. **************************************************************************************************/ import ( diff --git a/lints/rfc/lint_ian_bare_wildcard.go b/lints/community/lint_ian_bare_wildcard.go similarity index 91% rename from lints/rfc/lint_ian_bare_wildcard.go rename to lints/community/lint_ian_bare_wildcard.go index ae6d0fb9e..dfab411a6 100644 --- a/lints/rfc/lint_ian_bare_wildcard.go +++ b/lints/community/lint_ian_bare_wildcard.go @@ -1,4 +1,4 @@ -package rfc +package community /* * ZLint Copyright 2020 Regents of the University of Michigan @@ -44,9 +44,9 @@ func (l *brIANBareWildcard) Execute(c *x509.Certificate) *lint.LintResult { func init() { lint.RegisterLint(&lint.Lint{ Name: "e_ian_bare_wildcard", - Description: "A wildcard MUST be accompanied by other data to its right (Only checks DNSName)", - Citation: "RFC5280", - Source: lint.RFC5280, + Description: "A wildcard MUST be accompanied by other data to its right (Only checks IANDNSNames)", + Citation: "awslabs certlint", + Source: lint.AWSLabs, EffectiveDate: util.ZeroDate, Lint: &brIANBareWildcard{}, }) diff --git a/lints/rfc/lint_ian_bare_wildcard_test.go b/lints/community/lint_ian_bare_wildcard_test.go similarity index 98% rename from lints/rfc/lint_ian_bare_wildcard_test.go rename to lints/community/lint_ian_bare_wildcard_test.go index 54817c000..409aa3962 100644 --- a/lints/rfc/lint_ian_bare_wildcard_test.go +++ b/lints/community/lint_ian_bare_wildcard_test.go @@ -1,4 +1,4 @@ -package rfc +package community /* * ZLint Copyright 2020 Regents of the University of Michigan diff --git a/lints/community/lint_ian_wildcard_not_first.go b/lints/community/lint_ian_wildcard_not_first.go index 4fea86ecd..43bad4caa 100644 --- a/lints/community/lint_ian_wildcard_not_first.go +++ b/lints/community/lint_ian_wildcard_not_first.go @@ -44,7 +44,7 @@ func (l *brIANWildcardFirst) Execute(c *x509.Certificate) *lint.LintResult { func init() { lint.RegisterLint(&lint.Lint{ Name: "e_ian_wildcard_not_first", - Description: "A wildcard MUST be in the first label of FQDN (ie not: www.*.com) (Only checks DNSName)", + Description: "A wildcard MUST be in the first label of FQDN (ie not: www.*.com) (Only checks IANDNSNames)", Citation: "awslabs certlint", Source: lint.AWSLabs, EffectiveDate: util.ZeroDate, diff --git a/lints/community/lint_rsa_exp_negative_test.go b/lints/community/lint_rsa_exp_negative_test.go index bf6c0aee6..49010c9b4 100644 --- a/lints/community/lint_rsa_exp_negative_test.go +++ b/lints/community/lint_rsa_exp_negative_test.go @@ -21,18 +21,7 @@ import ( "github.com/zmap/zlint/util" ) -// func TestRsaExpNegative(t *testing.T) { -// inputPath := "../../testlint/testCerts/rsaExpNegative.pem" -// expected := lint.Error -// out := lint.Lints["rsa_exp_negative"].ExecuteTest(util.ReadCertificate(inputPath)) -// if out.Result != expected { -// t.Error( -// "For", inputPath, -// "expected", expected, -// "got", out.Result, -// ) -// } -// } +// TODO: There should be a test for negative RSA exp. func TestRsaExpPositive(t *testing.T) { inputPath := "../../testlint/testCerts/IANURIValid.pem" diff --git a/lints/community/lint_validity_time_not_positive.go b/lints/community/lint_validity_time_not_positive.go index 39ef4a1af..c8146f569 100644 --- a/lints/community/lint_validity_time_not_positive.go +++ b/lints/community/lint_validity_time_not_positive.go @@ -14,10 +14,6 @@ package community * permissions and limitations under the License. */ -/************************************************ -Change this to match source TEXT -************************************************/ - import ( "github.com/zmap/zcrypto/x509" "github.com/zmap/zlint/lint" diff --git a/lints/rfc/lint_basic_constraints_not_critical.go b/lints/rfc/lint_basic_constraints_not_critical.go index 7c0216d5e..9350a2463 100644 --- a/lints/rfc/lint_basic_constraints_not_critical.go +++ b/lints/rfc/lint_basic_constraints_not_critical.go @@ -43,7 +43,6 @@ func (l *basicConstCrit) CheckApplies(c *x509.Certificate) bool { } func (l *basicConstCrit) Execute(c *x509.Certificate) *lint.LintResult { - // Add actual lint here if e := util.GetExtFromCert(c, util.BasicConstOID); e != nil { if e.Critical { return &lint.LintResult{Status: lint.Pass} diff --git a/lints/rfc/lint_cert_contains_unique_identifier.go b/lints/rfc/lint_cert_contains_unique_identifier.go index ddc1adbd4..5b1db11f0 100644 --- a/lints/rfc/lint_cert_contains_unique_identifier.go +++ b/lints/rfc/lint_cert_contains_unique_identifier.go @@ -46,7 +46,7 @@ func (l *CertContainsUniqueIdentifier) CheckApplies(cert *x509.Certificate) bool func (l *CertContainsUniqueIdentifier) Execute(cert *x509.Certificate) *lint.LintResult { if cert.IssuerUniqueId.Bytes == nil && cert.SubjectUniqueId.Bytes == nil { return &lint.LintResult{Status: lint.Pass} - } //else + } return &lint.LintResult{Status: lint.Error} } diff --git a/lints/rfc/lint_ext_key_usage_cert_sign_without_ca.go b/lints/rfc/lint_ext_key_usage_cert_sign_without_ca.go index a58a8da96..191e92d8c 100644 --- a/lints/rfc/lint_ext_key_usage_cert_sign_without_ca.go +++ b/lints/rfc/lint_ext_key_usage_cert_sign_without_ca.go @@ -43,7 +43,7 @@ func (l *keyUsageCertSignNoCa) CheckApplies(c *x509.Certificate) bool { func (l *keyUsageCertSignNoCa) Execute(c *x509.Certificate) *lint.LintResult { if (c.KeyUsage & x509.KeyUsageCertSign) != 0 { - if c.BasicConstraintsValid && util.IsCACert(c) { //CA certs may assert certtificate signing usage + if c.BasicConstraintsValid && util.IsCACert(c) { //CA certs may assert certificate signing usage return &lint.LintResult{Status: lint.Pass} } else { return &lint.LintResult{Status: lint.Error} diff --git a/lints/rfc/lint_ext_key_usage_not_critical.go b/lints/rfc/lint_ext_key_usage_not_critical.go index 3178b4907..9bf50d99e 100644 --- a/lints/rfc/lint_ext_key_usage_not_critical.go +++ b/lints/rfc/lint_ext_key_usage_not_critical.go @@ -33,7 +33,6 @@ func (l *checkKeyUsageCritical) CheckApplies(c *x509.Certificate) bool { } func (l *checkKeyUsageCritical) Execute(c *x509.Certificate) *lint.LintResult { - // Add actual lint here keyUsage := util.GetExtFromCert(c, util.KeyUsageOID) if keyUsage == nil { return &lint.LintResult{Status: lint.NA} diff --git a/lints/rfc/lint_inhibit_any_policy_not_critical.go b/lints/rfc/lint_inhibit_any_policy_not_critical.go index 07a5ec5c7..436ee98a2 100644 --- a/lints/rfc/lint_inhibit_any_policy_not_critical.go +++ b/lints/rfc/lint_inhibit_any_policy_not_critical.go @@ -48,7 +48,7 @@ func (l *InhibitAnyPolicyNotCritical) CheckApplies(cert *x509.Certificate) bool func (l *InhibitAnyPolicyNotCritical) Execute(cert *x509.Certificate) *lint.LintResult { if anyPol := util.GetExtFromCert(cert, util.InhibitAnyPolicyOID); !anyPol.Critical { return &lint.LintResult{Status: lint.Error} - } //else + } return &lint.LintResult{Status: lint.Pass} } diff --git a/lints/rfc/lint_subject_common_name_max_length.go b/lints/rfc/lint_subject_common_name_max_length.go index fdcddfbb5..2931ebd7c 100644 --- a/lints/rfc/lint_subject_common_name_max_length.go +++ b/lints/rfc/lint_subject_common_name_max_length.go @@ -50,7 +50,7 @@ func (l *subjectCommonNameMaxLength) Execute(c *x509.Certificate) *lint.LintResu func init() { lint.RegisterLint(&lint.Lint{ Name: "e_subject_common_name_max_length", - Description: "The commonName field of the subject MUST be less than 64 characters", + Description: "The commonName field of the subject MUST be less than 65 characters", Citation: "RFC 5280: A.1", Source: lint.RFC5280, EffectiveDate: util.RFC2459Date, diff --git a/lints/rfc/lint_subject_dn_serial_number_max_length.go b/lints/rfc/lint_subject_dn_serial_number_max_length.go index 150319f2f..2e49b2228 100644 --- a/lints/rfc/lint_subject_dn_serial_number_max_length.go +++ b/lints/rfc/lint_subject_dn_serial_number_max_length.go @@ -42,7 +42,7 @@ func (l *SubjectDNSerialNumberMaxLength) Execute(c *x509.Certificate) *lint.Lint func init() { lint.RegisterLint(&lint.Lint{ Name: "e_subject_dn_serial_number_max_length", - Description: "The 'Serial Number' field of the subject MUST be less than 64 characters", + Description: "The 'Serial Number' field of the subject MUST be less than 65 characters", Citation: "RFC 5280: Appendix A", Source: lint.RFC5280, EffectiveDate: util.ZeroDate, diff --git a/lints/rfc/lint_subject_email_max_length.go b/lints/rfc/lint_subject_email_max_length.go index 7fd495e81..3e83bafca 100644 --- a/lints/rfc/lint_subject_email_max_length.go +++ b/lints/rfc/lint_subject_email_max_length.go @@ -59,7 +59,7 @@ func (l *subjectEmailMaxLength) Execute(c *x509.Certificate) *lint.LintResult { func init() { lint.RegisterLint(&lint.Lint{ Name: "e_subject_email_max_length", - Description: "The 'Email' field of the subject MUST be less than 255 characters", + Description: "The 'Email' field of the subject MUST be less than 256 characters", Citation: "RFC 5280: A.1", Source: lint.RFC5280, EffectiveDate: util.RFC2459Date, diff --git a/lints/rfc/lint_subject_locality_name_max_length.go b/lints/rfc/lint_subject_locality_name_max_length.go index 08d8f1d8f..1738f5341 100644 --- a/lints/rfc/lint_subject_locality_name_max_length.go +++ b/lints/rfc/lint_subject_locality_name_max_length.go @@ -52,7 +52,7 @@ func (l *subjectLocalityNameMaxLength) Execute(c *x509.Certificate) *lint.LintRe func init() { lint.RegisterLint(&lint.Lint{ Name: "e_subject_locality_name_max_length", - Description: "The 'Locality Name' field of the subject MUST be less than 128 characters", + Description: "The 'Locality Name' field of the subject MUST be less than 129 characters", Citation: "RFC 5280: A.1", Source: lint.RFC5280, EffectiveDate: util.RFC2459Date, diff --git a/lints/rfc/lint_subject_organization_name_max_length.go b/lints/rfc/lint_subject_organization_name_max_length.go index 93ea3f423..30c87524a 100644 --- a/lints/rfc/lint_subject_organization_name_max_length.go +++ b/lints/rfc/lint_subject_organization_name_max_length.go @@ -52,7 +52,7 @@ func (l *subjectOrganizationNameMaxLength) Execute(c *x509.Certificate) *lint.Li func init() { lint.RegisterLint(&lint.Lint{ Name: "e_subject_organization_name_max_length", - Description: "The 'Organization Name' field of the subject MUST be less than 64 characters", + Description: "The 'Organization Name' field of the subject MUST be less than 65 characters", Citation: "RFC 5280: A.1", Source: lint.RFC5280, EffectiveDate: util.RFC2459Date, diff --git a/lints/rfc/lint_subject_organizational_unit_name_max_length.go b/lints/rfc/lint_subject_organizational_unit_name_max_length.go index baa1618be..b3d71d8e7 100644 --- a/lints/rfc/lint_subject_organizational_unit_name_max_length.go +++ b/lints/rfc/lint_subject_organizational_unit_name_max_length.go @@ -52,7 +52,7 @@ func (l *subjectOrganizationalUnitNameMaxLength) Execute(c *x509.Certificate) *l func init() { lint.RegisterLint(&lint.Lint{ Name: "e_subject_organizational_unit_name_max_length", - Description: "The 'Organizational Unit Name' field of the subject MUST be less than 64 characters", + Description: "The 'Organizational Unit Name' field of the subject MUST be less than 65 characters", Citation: "RFC 5280: A.1", Source: lint.RFC5280, EffectiveDate: util.RFC2459Date, diff --git a/lints/rfc/lint_subject_state_name_max_length.go b/lints/rfc/lint_subject_state_name_max_length.go index d072d1bba..14cafa4a3 100644 --- a/lints/rfc/lint_subject_state_name_max_length.go +++ b/lints/rfc/lint_subject_state_name_max_length.go @@ -52,7 +52,7 @@ func (l *subjectStateNameMaxLength) Execute(c *x509.Certificate) *lint.LintResul func init() { lint.RegisterLint(&lint.Lint{ Name: "e_subject_state_name_max_length", - Description: "The 'State Name' field of the subject MUST be less than 128 characters", + Description: "The 'State Name' field of the subject MUST be less than 129 characters", Citation: "RFC 5280: A.1", Source: lint.RFC5280, EffectiveDate: util.RFC2459Date, diff --git a/lints/template_test.go b/lints/template_test.go new file mode 100644 index 000000000..98f072592 --- /dev/null +++ b/lints/template_test.go @@ -0,0 +1,56 @@ +package lints + +import ( + "bytes" + "io/ioutil" + "path/filepath" + "strings" + "testing" + + "github.com/zmap/zlint/lint" +) + +// TestLeftoverTemplates tests that no .go files for each of the +// lint.LintSources contain leftovers from the new lint template that are +// intended to be replaced by the programmer. +func TestLeftoverTemplates(t *testing.T) { + // See the `template` file in the root directory of ZLint. + // None of these strings should appear outside of the template. They indicate + // the programmer forgot to replace template text. + leftovers := []string{ + `"Fill this in..."`, + `"Change this..."`, + "// Add conditions for application here", + "// Add actual lint here", + "Change this to match source TEXT", + } + + for _, lintSrc := range lint.LintSources { + files, err := ioutil.ReadDir(lintSrc.Directory()) + if err != nil { + t.Fatalf("Failed to read directory %q", lintSrc.Directory()) + } + + for _, f := range files { + // Skip non-Go files + if !strings.HasSuffix(f.Name(), ".go") { + continue + } + + srcPath := filepath.Join(lintSrc.Directory(), f.Name()) + src, err := ioutil.ReadFile(srcPath) + if err != nil { + t.Errorf("Failed to read src file %q: %v", + f.Name(), err) + continue + } + + for _, leftover := range leftovers { + if bytes.Contains(src, []byte(leftover)) { + t.Errorf("Lint %q contains template leftover %q", + srcPath, leftover) + } + } + } + } +}