From 0ba0e8fc85e900dc2767b32b10d76125948196bb Mon Sep 17 00:00:00 2001 From: christopher-henderson Date: Fri, 9 Jul 2021 12:21:24 -0700 Subject: [PATCH 01/18] Support for configurable lints --- CONTRIBUTING.md | 101 ++ README.md | 6 + cosigned.pem | 8 + v3/cmd/zlint/ctx.toml | 35 + v3/cmd/zlint/main.go | 19 + v3/go.mod | 1 + v3/go.sum | 2 + v3/lint/base.go | 26 +- v3/lint/configuration.go | 376 ++++++++ v3/lint/configuration_test.go | 882 ++++++++++++++++++ v3/lint/global_configurations.go | 84 ++ v3/lint/registration.go | 67 +- v3/lint/registration_test.go | 6 +- ...lint_dsa_correct_order_in_subgroup_test.go | 2 +- ..._dsa_unique_correct_representation_test.go | 2 +- v3/resultset.go | 2 +- v3/test/configuration_test_framework_test.go | 203 ++++ v3/test/helpers.go | 31 +- 18 files changed, 1840 insertions(+), 13 deletions(-) create mode 100644 cosigned.pem create mode 100644 v3/cmd/zlint/ctx.toml create mode 100644 v3/lint/configuration.go create mode 100644 v3/lint/configuration_test.go create mode 100644 v3/lint/global_configurations.go create mode 100644 v3/test/configuration_test_framework_test.go diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 07e1c5248..9f2ace831 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -106,6 +106,84 @@ func (l *caCRLSignNotSet) Execute(c *x509.Certificate) *lint.LintResult { } ``` +Making your Lint Configurable +------------- +Lints may implement an optional interface - `Configurable`... + +```go +type Configurable interface { + Configure() interface{} +} +``` + +...where the returned `interface{}` is a pointer to the target struct to deserialize your configuration into. + +This struct may encode any arbitrary data that may be deserialized from [TOML](https://toml.io/en/). Examples may include: + +* PEM encoded certificates or certificate chains +* File paths +* Resolvable DNS entries or URIs +* Dates or Unix timestamps + +...and so on. How stable and/or appropriate a given configuration field is is left as a code review exercise on a per-lint basis. + +If a lint is `Configurable` then a new step is injected at the beginning of its lifecycle. + +--- +##### Non-Configurable Lifecycle +> * CheckApplies +> * CheckEffective +> * Execute + +##### Configurable Lifecycle +> * Configure +> * CheckApplies +> * CheckEffective +> * Execute + +### Higher Scoped Configurations + +Lints may embed within theselves either pointers or structs to the following definitions within the `lint` package. + +```go +type Global struct {} +type RFC5280Config struct{} +type RFC5480Config struct{} +type RFC5891Config struct{} +type CABFBaselineRequirementsConfig struct {} +type CABFEVGuidelinesConfig struct{} +type MozillaRootStorePolicyConfig struct{} +type AppleRootStorePolicyConfig struct{} +type CommunityConfig struct{} +type EtsiEsiConfig struct{} +``` + +Doing so will enable receiving a _copy_ of any such defintions from a higher scope within the configuration. + +```toml +# Top level (non-scoped) fields will be copied into any Global struct that you declare within your lint. +something_global = 5 +something_else_global = "The funniest joke in the world." + +[RFC5280] +# Top level (non-scoped) fields will be copied into any RFC5280Config struct that you declare within your lint. +wildcard_allowed = true + +[MyLint] +# You can also embed comments! +my_config = "Some arbitrary data." +``` + +An example of the above might be... + +```go +type MyLint struct { + Global lint.Global + RFC lint.RFC5280Config + MyConfig string `toml:"my_config",comment:"You can also embed comments!"` +} +``` + Testing Lints ------------- @@ -167,6 +245,29 @@ Please see the [integration tests README] for more information. [CI]: https://travis-ci.org/zmap/zlint [integration tests README]: https://github.com/zmap/zlint/blob/master/v3/integration/README.md +### Testing Configurable Lints + +Testing a lint that is configurable is much the same as testing on that is not. However, if you wish exercise +various configurations then you may do by utilizing the `test.TestLintWithConfig` function which takes in an extra +string which is the raw TOML of your target test configuration. + +```go +func TestCaCommonNameNotMissing2(t *testing.T) { + inputPath := "caCommonNameNotMissing.pem" + expected := lint.Pass + config := ` + [e_ca_common_name_missing2] + BeerHall = "liedershousen" + ` + out := test.TestLintWithConfig("e_ca_common_name_missing2", inputPath, config) + if out.Details != "liedershousen" { + panic("noooooooooooo") + } + if out.Status != expected { + t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status) + } +} +``` Updating the TLD Map -------------------- diff --git a/README.md b/README.md index fa67f158b..86af1a05c 100644 --- a/README.md +++ b/README.md @@ -96,6 +96,12 @@ Example ZLint CLI usage: echo "Lint mycert.pem with all of the lints except for ETSI ESI sourced lints" zlint -excludeSources=ETSI_ESI mycert.pem + + echo "Receive a copy of the full (default) configuration for all configurable lints" + zlint -exampleConfing + + echo "Lint mycert.pem using a custom configuration for any configurable lints" + zlint -config configFile.toml mycert.pem See `zlint -h` for all available command line options. diff --git a/cosigned.pem b/cosigned.pem new file mode 100644 index 000000000..8a21cedd4 --- /dev/null +++ b/cosigned.pem @@ -0,0 +1,8 @@ +-----BEGIN CERTIFICATE----- +MIIBBDCBrKADAgECAgEBMAoGCCqGSM49BAMCMAAwIhgPMDAwMTAxMDEwMDAwMDBa +GA85OTk4MTEzMDAwMDAwMFowADBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABDaq +E7tJ1D0lKPlIYcMylZ47WsEAFypRlBVUni+Q8ZY4S+NPVHFDxpZ/Qzx8qnXmHvf6 +8caqXJthylrSzH1HVGGjEzARMA8GA1UdEwEB/wQFMAMBAf8wCgYIKoZIzj0EAwID +RwAwRAIgJklSU+eEIOc+oUHa/ugiNsBpTsrrrJudXuZ41hayNQsCIEwxc3quig63 +ZI7LhYni7wlyXJBiXUlQkuacCGdqkv+/ +-----END CERTIFICATE----- diff --git a/v3/cmd/zlint/ctx.toml b/v3/cmd/zlint/ctx.toml new file mode 100644 index 000000000..641bbdf3a --- /dev/null +++ b/v3/cmd/zlint/ctx.toml @@ -0,0 +1,35 @@ + +[e_ca_too_few_beers] +# This MUST be set to the number of bottles of beer that were on the signing CAs wall at the time of issuance. +BottlesOfBeerOnTheWall = 0 +Cosigner = """ +-----BEGIN CERTIFICATE----- +MIIDAzCCAmygAwIBAQIETLCaTzANBgkqhkiG9w0BAQQFADCBuDELMAkGA1UEBhMC +SUwxCzAJBgNVBAgTAklMMRUwEwYDVQQHEwxEZW1vIEFkZHJlc3MxDjAMBgNVBBET +BTEyMzQ1MRgwFgYDVQQUEw8oMDAwLTApIDAwMDAwMDAxHDAaBgkqhkiG9w0BCQEW +DURlbW9AZGVtby5jb20xEjAQBgNVBAoTCURlbW8gSW5jLjENMAsGA1UECxMERGVt +bzEaMBgGA1UEAxMRdHlwby5zZ2RwYmVsbC5jb20wHhcNMTIwNDI3MDAwMDAwWhcN +MjIwNDI3MDUwMDAwWjCBuDELMAkGA1UEBhMCSUwxCzAJBgNVBAgTAklMMRUwEwYD +VQQHEwxEZW1vIEFkZHJlc3MxDjAMBgNVBBETBTEyMzQ1MRgwFgYDVQQUEw8oMDAw +LTApIDAwMDAwMDAxHDAaBgkqhkiG9w0BCQEWDURlbW9AZGVtby5jb20xEjAQBgNV +BAoTCURlbW8gSW5jLjENMAsGA1UECxMERGVtbzEaMBgGA1UEAxMRdHlwby5zZ2Rw +YmVsbC5jb20wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAKyiNTxe6BhY3Aau +rbflrC8M6DoFcdbhgIZnQq8Mf7Jiogxg0k0dawCHglBUzMuszsvTWIAxJu/hyA2W +oLLqNZglnWNjNEAywKtizq0ErePKG3pphXErKWIv7ZKPyWbdqQBxkrQ9sQPIlPBB +Mu8PGjMpcFqhYH1QijYemWHiu0ezAgMBAAGBCwNraXNoa3VzaGloggsDa2lzaGt1 +c2hpaDANBgkqhkiG9w0BAQQFAAOBgQCN1DUWa976qhQV3JQ6YF6jNJEepYKpqzLE +tQ7fZgh36wJR6kWtCETxQwKnowWPggxUwx27r4fRHrFa1WxUCGNlMKjpcEw+EK0Y +1WGePq4919wMyMdaiz6vhNVbnOhKwEcgUKrYlksDMNKPUlDUjfm/w+HdPqMxsnA+ +S5jc+5/gQg== +-----END CERTIFICATE----- +""" + +[e_ca_too_few_beers.i_spy_with_my_little_eye] +# Any number of descriptions that are valid for the subject. +Descriptions = ["larger than a bread box", "smaller than a barn"] +Subject = "A car" + +[w_web_pki_cert] +# Indicates that the certificate is intended for the Web PKI. +is_web_pki = false + diff --git a/v3/cmd/zlint/main.go b/v3/cmd/zlint/main.go index 53355e3d4..7d81bf217 100644 --- a/v3/cmd/zlint/main.go +++ b/v3/cmd/zlint/main.go @@ -47,6 +47,8 @@ var ( // flags includeSources string excludeSources string printVersion bool + config string + exampleConfig bool // version is replaced by GoReleaser or `make` using an LDFlags option at // build time. Here we supply a default value for folks that `go install` or @@ -66,6 +68,8 @@ func init() { flag.StringVar(&includeSources, "includeSources", "", "Comma-separated list of lint sources to include") flag.StringVar(&excludeSources, "excludeSources", "", "Comma-separated list of lint sources to exclude") flag.BoolVar(&printVersion, "version", false, "Print ZLint version and exit") + flag.StringVar(&config, "config", "", "A path to valid a TOML file that is to service as the configuration for a single run of ZLint") + flag.BoolVar(&exampleConfig, "exampleConfig", false, "Print a complete example of a configuration that is usable via the '-config' flag and exit. All values listed in this example will be set to their default.") flag.BoolVar(&prettyprint, "pretty", false, "Pretty-print JSON output") flag.Usage = func() { @@ -95,6 +99,16 @@ func main() { return } + if exampleConfig { + b, err := registry.DefaultConfiguration() + if err != nil { + fmt.Println(err) + os.Exit(1) + } + fmt.Printf("%s\n", string(b)) + return + } + if listLintSources { sources := registry.Sources() sort.Sort(sources) @@ -200,6 +214,11 @@ func trimmedList(raw string) []string { // includeNames, excludeNames, includeSources, and excludeSources flag values in // use. func setLints() (lint.Registry, error) { + ctx, err := lint.NewConfigFromFile(config) + if err != nil { + return nil, err + } + lint.GlobalRegistry().SetContext(ctx) // If there's no filter options set, use the global registry as-is if nameFilter == "" && includeNames == "" && excludeNames == "" && includeSources == "" && excludeSources == "" { return lint.GlobalRegistry(), nil diff --git a/v3/go.mod b/v3/go.mod index 4976416e8..296622774 100644 --- a/v3/go.mod +++ b/v3/go.mod @@ -3,6 +3,7 @@ module github.com/zmap/zlint/v3 require ( github.com/kr/text v0.2.0 // indirect github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect + github.com/pelletier/go-toml v1.9.3 github.com/sirupsen/logrus v1.8.1 github.com/stretchr/testify v1.7.0 // indirect github.com/zmap/zcrypto v0.0.0-20210811211718-6f9bc4aff20f diff --git a/v3/go.sum b/v3/go.sum index 476bcd2a5..879a3864e 100644 --- a/v3/go.sum +++ b/v3/go.sum @@ -12,6 +12,8 @@ github.com/mreiferson/go-httpclient v0.0.0-20160630210159-31f0106b4474/go.mod h1 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= github.com/op/go-logging v0.0.0-20160315200505-970db520ece7/go.mod h1:HzydrMdWErDVzsI23lYNej1Htcns9BCg93Dk0bBINWk= +github.com/pelletier/go-toml v1.9.3 h1:zeC5b1GviRUyKYd6OJPvBU/mcVDVoL1OhT17FCt5dSQ= +github.com/pelletier/go-toml v1.9.3/go.mod h1:u1nR/EPcESfeI/szUZKdtJ0xRNbUoANCkoOuaOx1Y+c= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/sirupsen/logrus v1.3.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= diff --git a/v3/lint/base.go b/v3/lint/base.go index 2db487926..2b5903720 100644 --- a/v3/lint/base.go +++ b/v3/lint/base.go @@ -15,6 +15,7 @@ package lint */ import ( + "fmt" "time" "github.com/zmap/zcrypto/x509" @@ -34,6 +35,10 @@ type LintInterface interface { Execute(c *x509.Certificate) *LintResult } +type Configurable interface { + Configure() interface{} +} + // A Lint struct represents a single lint, e.g. // "e_basic_constraints_not_critical". It contains an implementation of LintInterface. type Lint struct { @@ -90,11 +95,30 @@ func (l *Lint) CheckEffective(c *x509.Certificate) bool { // CheckApplies() // CheckEffective() // Execute() -func (l *Lint) Execute(cert *x509.Certificate) *LintResult { +func (l *Lint) Execute(cert *x509.Certificate, config Configuration) *LintResult { if l.Source == CABFBaselineRequirements && !util.IsServerAuthCert(cert) { return &LintResult{Status: NA} } lint := l.Lint() + switch configurable := lint.(type) { + case Configurable: + err := config.Configure(configurable.Configure(), l.Name) + if err != nil { + details := fmt.Sprintf( + "A fatal error occurred while attempting to configure %s. Please visit the [%s] section of "+ + "your provided configuration and compare it with the output of `zlint -exampleConfig`. Error: %s", + l.Name, + l.Name, + err.Error()) + return &LintResult{ + Status: Fatal, + Details: details} + } + } + return l.execute(lint, cert) +} + +func (l *Lint) execute(lint LintInterface, cert *x509.Certificate) *LintResult { if !lint.CheckApplies(cert) { return &LintResult{Status: NA} } else if !l.CheckEffective(cert) { diff --git a/v3/lint/configuration.go b/v3/lint/configuration.go new file mode 100644 index 000000000..dcd4e0325 --- /dev/null +++ b/v3/lint/configuration.go @@ -0,0 +1,376 @@ +package lint + +import ( + "fmt" + "io" + "os" + "reflect" + "strings" + + "github.com/pelletier/go-toml" +) + +type Configuration struct { + tree *toml.Tree +} + +func (c Configuration) Configure(lint interface{}, namespace string) error { + return c.deserializeConfigInto(lint, namespace) +} + +// NewConfig attempts to instantiate a configuration by consuming the contents of the provided reader. +// +// The contents of the provided reader MUST be in a valid TOML format. The caller of this function +// is responsible for closing the reader, if appropriate. +func NewConfig(r io.Reader) (Configuration, error) { + tree, err := toml.LoadReader(r) + if err != nil { + return Configuration{}, err + } + return Configuration{tree}, nil +} + +// NewConfigFromFile attempts to instantiate a configuration from the provided filesystem path. +// +// The file pointed to by `path` MUST be valid TOML file. +func NewConfigFromFile(path string) (Configuration, error) { + if path == "" { + return NewEmptyConfig(), nil + } + f, err := os.Open(path) + if err != nil { + return Configuration{}, fmt.Errorf("failed to open the provided configuration at %s. Error: %s", path, err.Error()) + } + defer f.Close() + return NewConfig(f) +} + +// NewConfigFromString attempts to instantiate a configuration from the provided string. +// +// The provided string MUST be in a valid TOML format. +func NewConfigFromString(config string) (Configuration, error) { + return NewConfig(strings.NewReader(config)) +} + +// NewEmptyConfig returns a configuration that is backed by an entirely empty TOML tree. +// +// This is useful of no particular configuration is set at all by the user of ZLint as +// any attempt to resolve a namespace in `deserializeConfigInto` fails and thus results +// in all defaults for all lints being maintained. +func NewEmptyConfig() Configuration { + ctx, _ := NewConfigFromString("") + return ctx +} + +// deserializeConfigInto deserializes the section labeled by the provided `namespace` +// into the provided target `interface{}`. +// +// For example, given the following configuration... +// +// ``` +// [e_some_lint] +// field = 1 +// flag = false +// +// [w_some_other_lint] +// is_web_pki = true +// ``` +// +// And the following struct definition... +// +// ``` +// type SomeOtherLint { +// IsWebPKI bool `toml:"is_web_pki"` +// } +// ``` +// +// Then the invocation of this function should be +// +// ``` +// lint := &SomeOtherLink{} +// deserializeConfigInto(link, "w_some_other_lint") +// ``` +// +// If there is no such namespace found in this configuration then provided the namespace specific data encoded +// within `target` is left unmodified. However, configuration of higher scoped fields will still be attempted. +func (c Configuration) deserializeConfigInto(target interface{}, namespace string) error { + if tree := c.tree.Get(namespace); tree != nil { + err := tree.(*toml.Tree).Unmarshal(target) + if err != nil { + return err + } + } + return c.resolveHigherScopedReferences(target) +} + +// resolveHigherScopeReferences takes in an interface{} value and attempts to +// find any field within its inner value that is either a struct or a pointer +// to a struct that is one of our global configurable types. If such a field +// exists then that higher scoped configuration will be deserialized, in-place, +// into the value held by the provided interface{}. +// +// This procedure is recursive. +// +// gocyclo is disabled because the relatively tall switch gives this function an outsized +// cyclomatic complexity rating despite it being relatively simple to understand (if not redundant). +// This redundancy is largely because Go neither has templating nor types as values. +// +// This procedure is certainly subject to compression if there is a more clever way to do this, although +// the Golang reflect package is rather unwieldy to being with, so it may be tricky. +// +//nolint:gocyclo +func (c Configuration) resolveHigherScopedReferences(i interface{}) error { + value := reflect.Indirect(reflect.ValueOf(i)) + if value.Kind() != reflect.Struct { + // Our target higher scoped configurations are either structs + // or are fields of structs. Any other Kind is simply cannot + // be a target for deserialization here. For example, an interface + // does not make sense since an interface cannot have fields nor + // are any of our higher scoped configurations interfaces themselves. + // + // For a comprehensive list of Kinds you may review type.go in the + // `reflect` package. + return nil + } + // Iterate through every field within the struct held by the provided interface{}. + // If the field is either one of our higher scoped configurations (or a pointer to one) + // then deserialize that higher scoped configuration into that field. If the field + // is not one of our higher scoped configurations then recursively pass it to this function + // in an attempt to resolve it. + for field := 0; field < value.NumField(); field++ { + field := value.Field(field) + if !field.CanInterface() { + continue + } + var val reflect.Value + switch t := field.Interface().(type) { + case Global: + err := c.deserializeConfigInto(&t, "") + if err != nil { + return err + } + val = reflect.ValueOf(t) + case *Global: + if t == nil { + t = &Global{} + } + err := c.deserializeConfigInto(t, "") + if err != nil { + return err + } + val = reflect.ValueOf(t) + case RFC5280Config: + err := c.deserializeConfigInto(&t, string(RFC5280)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case *RFC5280Config: + if t == nil { + t = &RFC5280Config{} + } + err := c.deserializeConfigInto(t, string(RFC5280)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case RFC5480Config: + err := c.deserializeConfigInto(&t, string(RFC5480)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case *RFC5480Config: + if t == nil { + t = &RFC5480Config{} + } + err := c.deserializeConfigInto(t, string(RFC5480)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case RFC5891Config: + err := c.deserializeConfigInto(&t, string(RFC5891)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case *RFC5891Config: + if t == nil { + t = &RFC5891Config{} + } + err := c.deserializeConfigInto(t, string(RFC5891)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case CABFBaselineRequirementsConfig: + err := c.deserializeConfigInto(&t, string(CABFBaselineRequirements)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case *CABFBaselineRequirementsConfig: + if t == nil { + t = &CABFBaselineRequirementsConfig{} + } + err := c.deserializeConfigInto(t, string(CABFBaselineRequirements)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case CABFEVGuidelinesConfig: + err := c.deserializeConfigInto(&t, string(CABFEVGuidelines)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case *CABFEVGuidelinesConfig: + if t == nil { + t = &CABFEVGuidelinesConfig{} + } + err := c.deserializeConfigInto(t, string(CABFEVGuidelines)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case MozillaRootStorePolicyConfig: + err := c.deserializeConfigInto(&t, string(MozillaRootStorePolicy)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case *MozillaRootStorePolicyConfig: + if t == nil { + t = &MozillaRootStorePolicyConfig{} + } + err := c.deserializeConfigInto(t, string(MozillaRootStorePolicy)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case AppleRootStorePolicyConfig: + err := c.deserializeConfigInto(&t, string(AppleRootStorePolicy)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case *AppleRootStorePolicyConfig: + if t == nil { + t = &AppleRootStorePolicyConfig{} + } + err := c.deserializeConfigInto(t, string(AppleRootStorePolicy)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case CommunityConfig: + err := c.deserializeConfigInto(&t, string(Community)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case *CommunityConfig: + if t == nil { + t = &CommunityConfig{} + } + err := c.deserializeConfigInto(t, string(Community)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case EtsiEsiConfig: + err := c.deserializeConfigInto(&t, string(EtsiEsi)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + case *EtsiEsiConfig: + if t == nil { + t = &EtsiEsiConfig{} + } + err := c.deserializeConfigInto(t, string(EtsiEsi)) + if err != nil { + return err + } + val = reflect.ValueOf(t) + default: + // In order to deserialize into a field it does indeed need to be addressable. + if !field.CanAddr() { + continue + } + err := c.resolveHigherScopedReferences(field.Addr().Interface()) + if err != nil { + return err + } + continue + } + field.Set(val) + } + return nil +} + +// stripGlobalsFromExample takes in an interface{} and returns a mapping that is +// the provided struct but with all references to higher scoped configurations scrubbed. +// +// This is intended only for use when constructing an example configuration file via the +// `-exampleConfig` flag. This is to avoid visually redundant, and possibliy incorrect, +// examples such as the following... +// +// ``` +// [Global] +// something = false +// something_else = "" +// +// [e_some_lint] +// my_data = 0 +// my_flag = false +// globals = { something = false, something_else = "" } +// ``` +// +// gocyclo is disabled because the relatively tall switch gives this function an outsized +// cyclomatic complexity rating despite it being relatively simple to understand. +// +//nolint:gocyclo +func stripGlobalsFromExample(i interface{}) interface{} { + value := reflect.Indirect(reflect.ValueOf(i)) + if value.Kind() != reflect.Struct { + return i + } + m := map[string]interface{}{} + for field := 0; field < value.NumField(); field++ { + name := value.Type().Field(field).Name + field := value.Field(field) + if field.Kind() == reflect.Ptr { + field = reflect.Zero(field.Type().Elem()) + } + if !field.CanInterface() { + continue + } + switch t := field.Interface().(type) { + case Global: + case *Global: + case RFC5280Config: + case *RFC5280Config: + case RFC5480Config: + case *RFC5480Config: + case RFC5891Config: + case *RFC5891Config: + case CABFBaselineRequirementsConfig: + case *CABFBaselineRequirementsConfig: + case CABFEVGuidelinesConfig: + case *CABFEVGuidelinesConfig: + case MozillaRootStorePolicyConfig: + case *MozillaRootStorePolicyConfig: + case AppleRootStorePolicyConfig: + case *AppleRootStorePolicyConfig: + case CommunityConfig: + case *CommunityConfig: + case EtsiEsiConfig: + case *EtsiEsiConfig: + default: + m[name] = stripGlobalsFromExample(t) + } + } + return m +} diff --git a/v3/lint/configuration_test.go b/v3/lint/configuration_test.go new file mode 100644 index 000000000..c9df07371 --- /dev/null +++ b/v3/lint/configuration_test.go @@ -0,0 +1,882 @@ +package lint + +import ( + "io" + "io/ioutil" + "reflect" + "sync" + "testing" + + "github.com/pelletier/go-toml" +) + +func TestInt(t *testing.T) { + type Test struct { + A int + } + c, err := NewConfigFromString(` +[Test] +A = 5`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if test.A != 5 { + t.Fatalf("wanted 5 got %d", test.A) + } +} + +func TestIntNegative(t *testing.T) { + type Test struct { + A int + } + c, err := NewConfigFromString(` +[Test] +A = -5`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if test.A != -5 { + t.Fatalf("wanted -5 got %d", test.A) + } +} + +func TestUint(t *testing.T) { + type Test struct { + A uint + } + c, err := NewConfigFromString(` +[Test] +A = 5`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if test.A != 5 { + t.Fatalf("wanted 5 got %d", test.A) + } +} + +func TestUintNegative(t *testing.T) { + type Test struct { + A uint + } + c, err := NewConfigFromString(` +[Test] +A = -5`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err == nil { + t.Fatalf("expected an error when deserializing a negative number into a uint, got %v", test) + } +} + +func TestSmallInt(t *testing.T) { + type Test struct { + A uint8 + } + c, err := NewConfigFromString(` +[Test] +A = 300`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err == nil { + t.Fatalf("expected an error when deserializing a number too large to fit in a uint8, got %v", test) + } +} + +func TestByte(t *testing.T) { + type Test struct { + A byte + } + c, err := NewConfigFromString(` +[Test] +A = 255`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if test.A != 255 { + t.Fatalf("wanted 255 got %d", test.A) + } +} + +func TestBool(t *testing.T) { + type Test struct { + A bool + } + c, err := NewConfigFromString(` +[Test] +A = true`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !test.A { + t.Fatalf("wanted true got %v", test.A) + } +} + +func TestString(t *testing.T) { + type Test struct { + A string + } + c, err := NewConfigFromString(` +[Test] +A = "the greatest song in the world"`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if test.A != "the greatest song in the world" { + t.Fatalf("wanted \"the greatest song in the world\" got %v", test.A) + } +} + +func TestArrayInt(t *testing.T) { + type Test struct { + A []int + } + c, err := NewConfigFromString(` +[Test] +A = [1, 2, 3, 4]`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test.A, []int{1, 2, 3, 4}) { + t.Fatalf("wanted [1, 2, 3, 4] got %v", test.A) + } +} + +func TestArrayString(t *testing.T) { + type Test struct { + A []string + } + c, err := NewConfigFromString(` +[Test] +A = ["1", "2", "3", "4"]`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test.A, []string{"1", "2", "3", "4"}) { + t.Fatalf("wanted [\"1\", \"2\", \"3\", \"4\"] got %v", test.A) + } +} + +func TestMapInt(t *testing.T) { + type Test struct { + A map[string]int + } + c, err := NewConfigFromString(` +[Test] +A = { version = 42 }`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test.A, map[string]int{"version": 42}) { + t.Fatalf("wanted { \"version\": 42 } got %v", test.A) + } +} + +func TestMapString(t *testing.T) { + type Test struct { + A map[string]string + } + c, err := NewConfigFromString(` +[Test] +A = { version = "1.2.3" }`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test.A, map[string]string{"version": "1.2.3"}) { + t.Fatalf("wanted { \"version\": \"1.2.3\" } got %v", test.A) + } +} + +func TestMapArray(t *testing.T) { + type Test struct { + A map[string][]int + } + c, err := NewConfigFromString(` +[Test] +A = { version = [1, 2 ,3] }`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test.A, map[string][]int{"version": {1, 2, 3}}) { + t.Fatalf("wanted { \"version\": [1, 2 ,3] } got %v", test.A) + } +} + +func TestMapMap(t *testing.T) { + type Test struct { + A map[string]map[string]string + } + c, err := NewConfigFromString(` +[Test] +A = { version = { commit = "29c848e565ebfa2a376767919bb0880be46b3c0f" } }`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test.A, map[string]map[string]string{"version": {"commit": "29c848e565ebfa2a376767919bb0880be46b3c0f"}}) { + t.Fatalf("wanted {\"versio\": { \"commit\": \"29c848e565ebfa2a376767919bb0880be46b3c0f\" } } got %v", test.A) + } +} + +func TestStruct(t *testing.T) { + type Inner struct { + B int + } + type Test struct { + A Inner + } + c, err := NewConfigFromString(` +[Test] +A = { B = 1 }`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{Inner{1}}) { + t.Fatalf("wanted {A {1}} got %v", test) + } +} + +func TestPointer(t *testing.T) { + type Test struct { + A *int + } + c, err := NewConfigFromString(` +[Test] +A = 1`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if test.A == nil { + t.Fatal("wanted a pointer to 1, got nil") + } + if *test.A != 1 { + t.Fatalf("wanted a pointer to 1, got a point to %d", *test.A) + } +} + +func TestInterface(t *testing.T) { + type Test struct { + A bool + B io.Reader + } + c, err := NewConfigFromString(` +[Test] +A = true`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{true, nil}) { + t.Fatalf("wanted {true nil} got %v", test) + } +} + +func TestSmokeExamplePrinting(t *testing.T) { + type Inner struct { + Things []int + } + type Test struct { + A bool + B io.Reader + C *int + D Inner + } + mapping := stripGlobalsFromExample(&Test{}) + rr, w := io.Pipe() + var err error + wg := sync.WaitGroup{} + wg.Add(1) + go func() { + defer wg.Done() + defer w.Close() + err = toml.NewEncoder(w).Indentation("").CompactComments(true).Encode(mapping) + }() + if err != nil { + t.Fatal(err) + } + b, err := ioutil.ReadAll(rr) + if err != nil { + t.Fatal(err) + } + want := `A = false +C = 0 + +[D] +Things = [] +` + if want != string(b) { + t.Fatalf("wanted `%s` got '%s'", want, string(b)) + } +} + +func TestRecursiveStruct(t *testing.T) { + type Test struct { + A *Test + B bool + } + c, err := NewConfigFromString(` +[Test] +A = { B = true } +B = true +`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{&Test{nil, true}, true}) { + t.Fatalf("wanted Test{&Test{nil, true}, true} got %v", test) + } +} + +func TestBadToml(t *testing.T) { + _, err := NewConfigFromString(`(┛ಠ_ಠ)┛彡┻━┻`) + if err == nil { + t.Fatal("expected a parsing, however received a nil error") + } +} + +func TestEmbedGlobal(t *testing.T) { + type Test struct { + Global Global + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{Global: Global{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{Global: Global{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedRFC5280Config(t *testing.T) { + type Test struct { + RFC5280Config RFC5280Config + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{RFC5280Config: RFC5280Config{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{RFC5280Config: RFC5280Config{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedRFC5480Config(t *testing.T) { + type Test struct { + RFC5480Config RFC5480Config + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{RFC5480Config: RFC5480Config{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{RFC5480Config: RFC5480Config{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedRFC5891Config(t *testing.T) { + type Test struct { + RFC5891Config RFC5891Config + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{RFC5891Config: RFC5891Config{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{RFC5891Config: RFC5891Config{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedCABFBaselineRequirementsConfig(t *testing.T) { + type Test struct { + CABFBaselineRequirementsConfig CABFBaselineRequirementsConfig + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{CABFBaselineRequirementsConfig: CABFBaselineRequirementsConfig{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{CABFBaselineRequirementsConfig: CABFBaselineRequirementsConfig{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedCABFEVGuidelinesConfig(t *testing.T) { + type Test struct { + CABFEVGuidelinesConfig CABFEVGuidelinesConfig + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{CABFEVGuidelinesConfig: CABFEVGuidelinesConfig{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{CABFEVGuidelinesConfig: CABFEVGuidelinesConfig{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedMozillaRootStorePolicyConfig(t *testing.T) { + type Test struct { + MozillaRootStorePolicyConfig MozillaRootStorePolicyConfig + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{MozillaRootStorePolicyConfig: MozillaRootStorePolicyConfig{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{MozillaRootStorePolicyConfig: MozillaRootStorePolicyConfig{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedAppleRootStorePolicyConfig(t *testing.T) { + type Test struct { + AppleRootStorePolicyConfig AppleRootStorePolicyConfig + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{AppleRootStorePolicyConfig: AppleRootStorePolicyConfig{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{AppleRootStorePolicyConfig: AppleRootStorePolicyConfig{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedCommunityConfig(t *testing.T) { + type Test struct { + CommunityConfig CommunityConfig + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{CommunityConfig: CommunityConfig{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{CommunityConfig: CommunityConfig{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedEtsiEsiConfig(t *testing.T) { + type Test struct { + EtsiEsiConfig EtsiEsiConfig + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{EtsiEsiConfig: EtsiEsiConfig{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{EtsiEsiConfig: EtsiEsiConfig{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedPtrToGlobal(t *testing.T) { + type Test struct { + Global *Global + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{Global: &Global{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{Global: &Global{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedPtrToRFC5280Config(t *testing.T) { + type Test struct { + RFC5280Config *RFC5280Config + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{RFC5280Config: &RFC5280Config{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{RFC5280Config: &RFC5280Config{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedPtrToRFC5480Config(t *testing.T) { + type Test struct { + RFC5480Config *RFC5480Config + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{RFC5480Config: &RFC5480Config{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{RFC5480Config: &RFC5480Config{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedPtrToRFC5891Config(t *testing.T) { + type Test struct { + RFC5891Config *RFC5891Config + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{RFC5891Config: &RFC5891Config{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{RFC5891Config: &RFC5891Config{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedPtrToCABFBaselineRequirementsConfig(t *testing.T) { + type Test struct { + CABFBaselineRequirementsConfig *CABFBaselineRequirementsConfig + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{CABFBaselineRequirementsConfig: &CABFBaselineRequirementsConfig{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{CABFBaselineRequirementsConfig: &CABFBaselineRequirementsConfig{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedPtrToCABFEVGuidelinesConfig(t *testing.T) { + type Test struct { + CABFEVGuidelinesConfig *CABFEVGuidelinesConfig + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{CABFEVGuidelinesConfig: &CABFEVGuidelinesConfig{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{CABFEVGuidelinesConfig: &CABFEVGuidelinesConfig{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedPtrToMozillaRootStorePolicyConfig(t *testing.T) { + type Test struct { + MozillaRootStorePolicyConfig *MozillaRootStorePolicyConfig + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{MozillaRootStorePolicyConfig: &MozillaRootStorePolicyConfig{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{MozillaRootStorePolicyConfig: &MozillaRootStorePolicyConfig{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedPtrToAppleRootStorePolicyConfig(t *testing.T) { + type Test struct { + AppleRootStorePolicyConfig *AppleRootStorePolicyConfig + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{AppleRootStorePolicyConfig: &AppleRootStorePolicyConfig{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{AppleRootStorePolicyConfig: &AppleRootStorePolicyConfig{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedPtrToCommunityConfig(t *testing.T) { + type Test struct { + CommunityConfig *CommunityConfig + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{CommunityConfig: &CommunityConfig{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{CommunityConfig: &CommunityConfig{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestEmbedPtrToEtsiEsiConfig(t *testing.T) { + type Test struct { + EtsiEsiConfig *EtsiEsiConfig + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{EtsiEsiConfig: &EtsiEsiConfig{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{EtsiEsiConfig: &EtsiEsiConfig{}, SomethingElse: \"cool\"}} got %v", test) + } +} + +func TestGlobalStripper(t *testing.T) { + type Test struct { + EtsiEsiConfig *EtsiEsiConfig + SomethingElse string + } + c, err := NewConfigFromString(` + [Test] + SomethingElse = "cool" + `) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{EtsiEsiConfig: &EtsiEsiConfig{}, SomethingElse: "cool"}) { + t.Fatalf("wanted Test{EtsiEsiConfig: &EtsiEsiConfig{}, SomethingElse: \"cool\"}} got %v", test) + } +} diff --git a/v3/lint/global_configurations.go b/v3/lint/global_configurations.go new file mode 100644 index 000000000..6a9730b7f --- /dev/null +++ b/v3/lint/global_configurations.go @@ -0,0 +1,84 @@ +package lint + +// Global is what one would intuitive think of as being the global context of the configuration file. +// That is, given the following configuration... +// +// some_flag = true +// some_string = "the greatest song in the world" +// +// [e_some_lint] +// some_other_flag = false +// +// The fields `some_flag` and `some_string` will be targeted to land into this struct. +type Global struct{} + +// RFC5280Config is the higher scoped configuration which services as the deserialization target for... +// +// [RFC5280Config] +// ... +// ... +type RFC5280Config struct{} + +// RFC5480Config is the higher scoped configuration which services as the deserialization target for... +// +// [RFC5480Config] +// ... +// ... +type RFC5480Config struct{} + +// RFC5891Config is the higher scoped configuration which services as the deserialization target for... +// +// [RFC5891Config] +// ... +// ... +type RFC5891Config struct{} + +// CABFBaselineRequirementsConfig is the higher scoped configuration which services as the deserialization target for... +// +// [CABFBaselineRequirementsConfig] +// ... +// ... +type CABFBaselineRequirementsConfig struct{} + +// CABFEVGuidelinesConfig is the higher scoped configuration which services as the deserialization target for... +// +// [CABFEVGuidelinesConfig] +// ... +// ... +type CABFEVGuidelinesConfig struct{} + +// MozillaRootStorePolicyConfig is the higher scoped configuration which services as the deserialization target for... +// +// [MozillaRootStorePolicyConfig] +// ... +// ... +type MozillaRootStorePolicyConfig struct{} + +// AppleRootStorePolicyConfig is the higher scoped configuration which services as the deserialization target for... +// +// [AppleRootStorePolicyConfig] +// ... +// ... +type AppleRootStorePolicyConfig struct{} + +// CommunityConfig is the higher scoped configuration which services as the deserialization target for... +// +// [CommunityConfig] +// ... +// ... +type CommunityConfig struct{} + +// EtsiEsiConfig is the higher scoped configuration which services as the deserialization target for... +// +// [EtsiEsiConfig] +// ... +// ... +type EtsiEsiConfig struct{} + +func (e *EtsiEsiConfig) namespace() string { + return string(EtsiEsi) +} + +type namespacer interface { + namespace() string +} diff --git a/v3/lint/registration.go b/v3/lint/registration.go index ed08b5cb3..85e727664 100644 --- a/v3/lint/registration.go +++ b/v3/lint/registration.go @@ -15,6 +15,7 @@ package lint import ( + "bytes" "encoding/json" "errors" "fmt" @@ -23,6 +24,8 @@ import ( "sort" "strings" "sync" + + "github.com/pelletier/go-toml" ) // FilterOptions is a struct used by Registry.Filter to create a sub registry @@ -75,6 +78,8 @@ type Registry interface { // Sources returns a SourceList of registered LintSources. The list is not // sorted but can be sorted by the caller with sort.Sort() if required. Sources() SourceList + // @TODO + DefaultConfiguration() ([]byte, error) // ByName returns a pointer to the registered lint with the given name, or nil // if there is no such lint registered in the registry. ByName(name string) *Lint @@ -87,6 +92,8 @@ type Registry interface { // WriteJSON writes a description of each registered lint as // a JSON object, one object per line, to the provided writer. WriteJSON(w io.Writer) + SetContext(ctx Configuration) + GetContext() Configuration } // registryImpl implements the Registry interface to provide a global collection @@ -102,6 +109,7 @@ type registryImpl struct { // lintsBySource is a map of all registered lints by source category. Lints // are added to the lintsBySource map by RegisterLint. lintsBySource map[LintSource][]*Lint + configuration Configuration } var ( @@ -136,7 +144,7 @@ func (r *registryImpl) register(l *Lint) error { if l == nil { return errNilLint } - if l.Lint == nil { + if l.Lint() == nil { return errNilLintPtr } if l.Name == "" { @@ -232,6 +240,7 @@ func (r *registryImpl) Filter(opts FilterOptions) (Registry, error) { } filteredRegistry := NewRegistry() + filteredRegistry.SetContext(r.configuration) sourceExcludes := sourceListToMap(opts.ExcludeSources) sourceIncludes := sourceListToMap(opts.IncludeSources) @@ -290,6 +299,60 @@ func (r *registryImpl) WriteJSON(w io.Writer) { } } +func (r *registryImpl) SetContext(ctx Configuration) { + r.configuration = ctx +} + +func (r *registryImpl) GetContext() Configuration { + return r.configuration +} + +// DefaultConfiguration returns a serialized copy of the default configuration for ZLint. +// +// This is especially useful combined with the -exampleConfig CLI argument which prints this +// to stdout. In this way, operators can quickly see what lints are configurable and what their +// fields are without having to dig through documentation or, even worse, code. +func (r *registryImpl) DefaultConfiguration() ([]byte, error) { + configurables := map[string]interface{}{} + for name, lint := range r.lintsByName { + switch configurable := lint.Lint().(type) { + case Configurable: + configurables[name] = stripGlobalsFromExample(configurable.Configure()) + default: + // At this point, most lints are not configurable. + } + } + // We're just using stripGlobalsFromExample here as a convenient way to + // recursively turn the `Global` struct type into a map. + // + // We have to do this because if simply followed the pattern below and did... + // + // configurables["Global"] = &Global{} + // + // ...then we would end up with a [Global] section in the resulting configuration, + // which is not what we are looking for (we simply want it to flattened out into + // the top most context of the configuration file). + for k, v := range stripGlobalsFromExample(&Global{}).(map[string]interface{}) { + configurables[k] = v + } + configurables[string(CABFBaselineRequirements)] = &CABFBaselineRequirementsConfig{} + configurables[string(RFC5280)] = &RFC5280Config{} + configurables[string(RFC5480)] = &RFC5480Config{} + configurables[string(RFC5891)] = &RFC5891Config{} + configurables[string(CABFBaselineRequirements)] = &CABFBaselineRequirementsConfig{} + configurables[string(CABFEVGuidelines)] = &CABFEVGuidelinesConfig{} + configurables[string(MozillaRootStorePolicy)] = &MozillaRootStorePolicyConfig{} + configurables[string(AppleRootStorePolicy)] = &AppleRootStorePolicyConfig{} + configurables[string(Community)] = &CommunityConfig{} + configurables[string(EtsiEsi)] = &EtsiEsiConfig{} + w := &bytes.Buffer{} + err := toml.NewEncoder(w).Indentation("").CompactComments(true).Encode(configurables) + if err != nil { + return nil, err + } + return w.Bytes(), nil +} + // NewRegistry constructs a Registry implementation that can be used to register // lints. func NewRegistry() *registryImpl { @@ -310,7 +373,7 @@ var globalRegistry *registryImpl = NewRegistry() // registration process. // // IMPORTANT: RegisterLint will panic if given a nil lint, or a lint with a nil -// Lint pointer, or if the lint's Initialize function errors, or if the lint +// Lint pointer, or if the lint's Initialize function errors, or if the lint` // name matches a previously registered lint's name. These conditions all // indicate a bug that should be addressed by a developer. func RegisterLint(l *Lint) { diff --git a/v3/lint/registration_test.go b/v3/lint/registration_test.go index 3976b69ce..615afb319 100644 --- a/v3/lint/registration_test.go +++ b/v3/lint/registration_test.go @@ -92,8 +92,10 @@ func TestRegister(t *testing.T) { expectErr: errNilLint, }, { - name: "nil lint ptr", - lint: &Lint{}, + name: "nil lint ptr", + lint: &Lint{ + Lint: func() LintInterface { return nil }, + }, expectErr: errNilLintPtr, }, { diff --git a/v3/lints/cabf_br/lint_dsa_correct_order_in_subgroup_test.go b/v3/lints/cabf_br/lint_dsa_correct_order_in_subgroup_test.go index c271caed0..97c8c09b6 100644 --- a/v3/lints/cabf_br/lint_dsa_correct_order_in_subgroup_test.go +++ b/v3/lints/cabf_br/lint_dsa_correct_order_in_subgroup_test.go @@ -41,7 +41,7 @@ func TestDSANotCorrectOrderSubgroup(t *testing.T) { pMinusOne.Sub(dsaKey.P, big.NewInt(1)) dsaKey.Y = pMinusOne expected := lint.Error - out := test.TestLintCert("e_dsa_correct_order_in_subgroup", c) + out := test.TestLintCert("e_dsa_correct_order_in_subgroup", c, lint.NewEmptyConfig()) if out.Status != expected { t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status) } diff --git a/v3/lints/cabf_br/lint_dsa_unique_correct_representation_test.go b/v3/lints/cabf_br/lint_dsa_unique_correct_representation_test.go index 990fedc6d..b663f565a 100644 --- a/v3/lints/cabf_br/lint_dsa_unique_correct_representation_test.go +++ b/v3/lints/cabf_br/lint_dsa_unique_correct_representation_test.go @@ -45,7 +45,7 @@ func TestDSANotUniqueCorrectRepresentation(t *testing.T) { // Expect failure expected := lint.Error - out := test.TestLintCert("e_dsa_unique_correct_representation", c) + out := test.TestLintCert("e_dsa_unique_correct_representation", c, lint.NewEmptyConfig()) if out.Status != expected { t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status) } diff --git a/v3/resultset.go b/v3/resultset.go index f1e4db3c9..b01de58a2 100644 --- a/v3/resultset.go +++ b/v3/resultset.go @@ -38,7 +38,7 @@ func (z *ResultSet) execute(cert *x509.Certificate, registry lint.Registry) { z.Results = make(map[string]*lint.LintResult, len(registry.Names())) // Run each lints from the registry. for _, name := range registry.Names() { - res := registry.ByName(name).Execute(cert) + res := registry.ByName(name).Execute(cert, registry.GetContext()) z.Results[name] = res z.updateErrorStatePresent(res) } diff --git a/v3/test/configuration_test_framework_test.go b/v3/test/configuration_test_framework_test.go new file mode 100644 index 000000000..ac60adb70 --- /dev/null +++ b/v3/test/configuration_test_framework_test.go @@ -0,0 +1,203 @@ +package test + +/* + * ZLint Copyright 2021 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. + */ + +import ( + "fmt" + "math/rand" + "strconv" + "sync" + "testing" + + "github.com/zmap/zcrypto/x509" + "github.com/zmap/zlint/v3/util" + + "github.com/zmap/zlint/v3/lint" +) + +type caCommonNameMissing struct { + BeerHall string + Working *lint.CABFBaselineRequirementsConfig +} + +func init() { + lint.RegisterLint(&lint.Lint{ + Name: "e_ca_common_name_missing2", + Description: "CA Certificates common name MUST be included.", + Citation: "BRs: 7.1.4.3.1", + Source: lint.CABFBaselineRequirements, + EffectiveDate: util.CABV148Date, + Lint: NewCaCommonNameMissing, + }) +} + +func (l *caCommonNameMissing) Configure() interface{} { + return l +} + +func NewCaCommonNameMissing() lint.LintInterface { + return &caCommonNameMissing{} +} + +func (l *caCommonNameMissing) CheckApplies(c *x509.Certificate) bool { + return util.IsCACert(c) +} + +func (l *caCommonNameMissing) Execute(c *x509.Certificate) *lint.LintResult { + if c.Subject.CommonName == "" { + return &lint.LintResult{Status: lint.Error, Details: l.BeerHall} + } else { + return &lint.LintResult{Status: lint.Pass, Details: l.BeerHall} + } +} + +func TestCaCommonNameMissing(t *testing.T) { + inputPath := "caCommonNameMissing.pem" + expected := lint.Error + out := TestLint("e_ca_common_name_missing2", inputPath) + if out.Status != expected { + t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status) + } +} + +func TestCaCommonNameNotMissing(t *testing.T) { + inputPath := "caCommonNameNotMissing.pem" + expected := lint.Pass + out := TestLint("e_ca_common_name_missing2", inputPath) + if out.Status != expected { + t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status) + } +} + +func TestCaCommonNameNotMissing2(t *testing.T) { + inputPath := "caCommonNameNotMissing.pem" + expected := lint.Pass + config := ` +[e_ca_common_name_missing2] +BeerHall = "liedershousen" +` + out := TestLintWithConfig("e_ca_common_name_missing2", inputPath, config) + if out.Details != "liedershousen" { + panic("noooooooooooo") + } + if out.Status != expected { + t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status) + } +} + +func TestCaCommonNameNotMissing3(t *testing.T) { + inputPath := "caCommonNameNotMissing.pem" + expected := lint.Pass + config := ` +[e_ca_common_name_missing2] +BeerHall = "liedershousenssss" +` + out := TestLintWithConfig("e_ca_common_name_missing2", inputPath, config) + if out.Details != "liedershousenssss" { + panic("noooooooooooo") + } + if out.Status != expected { + t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status) + } +} + +// This exercises the thread safety our configurable lints. This is because +// the lints use to be global singletons before we swapped them over to +// running as single instances. However, it is a good exercise to keep around. +func TestConcurrency(t *testing.T) { + inputPath := "caCommonNameNotMissing.pem" + expected := lint.Pass + wg := sync.WaitGroup{} + wg.Add(1000) + for i := 0; i < 1000; i++ { + go func() { + defer wg.Done() + num := strconv.Itoa(rand.Intn(9999)) + config := fmt.Sprintf(` +[e_ca_common_name_missing2] +BeerHall = "%s" +`, num) + out := TestLintWithConfig("e_ca_common_name_missing2", inputPath, config) + if out.Details != num { + t.Errorf("wanted %s got %s", num, out.Details) + } + if out.Status != expected { + t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status) + } + }() + } + wg.Wait() +} + +func TestCaCommonNameNotMissing4(t *testing.T) { + inputPath := "caCommonNameNotMissing.pem" + expected := lint.Pass + config := ` +[CABF_BR] +DoesItWork = "yes, yes it does" + +[e_ca_common_name_missing2] +BeerHall = "liedershousenssss" +` + out := TestLintWithConfig("e_ca_common_name_missing2", inputPath, config) + if out.Status != expected { + t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status) + } + if out.Details != "liedershousenssss" { + panic("noooooooooooo") + } +} + +type LintEmbedsAConfiguration struct { + configuration embeddedConfiguration + SomeOtherFieldThatWeDontWantToExpose int +} + +type embeddedConfiguration struct { + IsWebPKI bool `toml:"is_web_pki" comment:"Indicates that the certificate is intended for the Web PKI."` +} + +func init() { + lint.RegisterLint(&lint.Lint{ + Name: "w_web_pki_cert", + Description: "CA Certificates SHOULD....something....about the web pki", + Citation: "BRs: 7.1.4.3.1", + Source: lint.CABFBaselineRequirements, + EffectiveDate: util.CABV148Date, + Lint: NewLintEmbedsAConfiguration, + }) +} + +// A pointer to an embedded struct may be passed to the framework +// if the author does not wish to expose certain fields in their primary struct. +func (l *LintEmbedsAConfiguration) Configure() interface{} { + return &l.configuration +} + +func NewLintEmbedsAConfiguration() lint.LintInterface { + return &LintEmbedsAConfiguration{configuration: embeddedConfiguration{}} +} + +func (l *LintEmbedsAConfiguration) CheckApplies(c *x509.Certificate) bool { + return util.IsCACert(c) +} + +func (l *LintEmbedsAConfiguration) Execute(c *x509.Certificate) *lint.LintResult { + if l.configuration.IsWebPKI { + return &lint.LintResult{Status: lint.Warn, Details: "Time for a beer run!"} + } else { + return &lint.LintResult{Status: lint.Pass} + } +} diff --git a/v3/test/helpers.go b/v3/test/helpers.go index 45ada28fd..9f8308152 100644 --- a/v3/test/helpers.go +++ b/v3/test/helpers.go @@ -17,9 +17,13 @@ package test // Contains resources necessary to the Unit Test Cases import ( + "bytes" "encoding/pem" "fmt" "os" + + "os/exec" + "path" "strings" "github.com/zmap/zcrypto/x509" @@ -34,7 +38,15 @@ import ( // lintName is not known or if the testCertFilename can not be loaded, or if the // lint result is nil. func TestLint(lintName string, testCertFilename string) *lint.LintResult { - return TestLintCert(lintName, ReadTestCert(testCertFilename)) + return TestLintWithConfig(lintName, testCertFilename, "") +} + +func TestLintWithConfig(lintName string, testCertFilename string, context string) *lint.LintResult { + ctx, err := lint.NewConfigFromString(context) + if err != nil { + panic(err) + } + return TestLintCert(lintName, ReadTestCert(testCertFilename), ctx) } // TestLintCert executes a lint with the given name against an already parsed @@ -43,7 +55,7 @@ func TestLint(lintName string, testCertFilename string) *lint.LintResult { // // Important: TestLintCert is only appropriate for unit tests. It will panic if // the lintName is not known or if the lint result is nil. -func TestLintCert(lintName string, cert *x509.Certificate) *lint.LintResult { +func TestLintCert(lintName string, cert *x509.Certificate, ctx lint.Configuration) *lint.LintResult { l := lint.GlobalRegistry().ByName(lintName) if l == nil { panic(fmt.Sprintf( @@ -51,8 +63,7 @@ func TestLintCert(lintName string, cert *x509.Certificate) *lint.LintResult { "Did you forget to RegisterLint?\n", lintName)) } - - res := l.Execute(cert) + res := l.Execute(cert, ctx) // We never expect a lint to return a nil LintResult if res == nil { panic(fmt.Sprintf( @@ -62,13 +73,23 @@ func TestLintCert(lintName string, cert *x509.Certificate) *lint.LintResult { return res } +var testDir = "" + // ReadTestCert loads a x509.Certificate from the given inPath which is assumed // to be relative to `testdata/`. // // Important: ReadTestCert is only appropriate for unit tests. It will panic if // the inPath file can not be loaded. func ReadTestCert(inPath string) *x509.Certificate { - fullPath := fmt.Sprintf("../../testdata/%s", inPath) + if testDir == "" { + cmd := exec.Command("git", "rev-parse", "--show-toplevel") + out, err := cmd.CombinedOutput() + if err != nil { + panic(fmt.Sprintf("error when attempting to find the root directory of the repository: %v, output: '%s'", err, out)) + } + testDir = path.Join(string(bytes.TrimSpace(out)), "v3", "testdata") + } + fullPath := path.Join(testDir, inPath) data, err := os.ReadFile(fullPath) if err != nil { From 253cf9e55525941ccdae11cb51e121dcfe7163b6 Mon Sep 17 00:00:00 2001 From: christopher-henderson Date: Sun, 28 Nov 2021 11:50:46 -0800 Subject: [PATCH 02/18] removing vestigial test interface --- v3/lint/global_configurations.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/v3/lint/global_configurations.go b/v3/lint/global_configurations.go index 6a9730b7f..5e5bc0cc7 100644 --- a/v3/lint/global_configurations.go +++ b/v3/lint/global_configurations.go @@ -74,11 +74,3 @@ type CommunityConfig struct{} // ... // ... type EtsiEsiConfig struct{} - -func (e *EtsiEsiConfig) namespace() string { - return string(EtsiEsi) -} - -type namespacer interface { - namespace() string -} From fa256fb0d6b59772a6fc6ae5bd7691343a8af8fb Mon Sep 17 00:00:00 2001 From: christopher-henderson Date: Sun, 28 Nov 2021 11:54:06 -0800 Subject: [PATCH 03/18] removing test files and making naming more consistent --- cosigned.pem | 8 -------- v3/cmd/zlint/ctx.toml | 35 ----------------------------------- v3/cmd/zlint/main.go | 4 ++-- v3/lint/registration.go | 10 +++++----- v3/resultset.go | 2 +- 5 files changed, 8 insertions(+), 51 deletions(-) delete mode 100644 cosigned.pem delete mode 100644 v3/cmd/zlint/ctx.toml diff --git a/cosigned.pem b/cosigned.pem deleted file mode 100644 index 8a21cedd4..000000000 --- a/cosigned.pem +++ /dev/null @@ -1,8 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIBBDCBrKADAgECAgEBMAoGCCqGSM49BAMCMAAwIhgPMDAwMTAxMDEwMDAwMDBa -GA85OTk4MTEzMDAwMDAwMFowADBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABDaq -E7tJ1D0lKPlIYcMylZ47WsEAFypRlBVUni+Q8ZY4S+NPVHFDxpZ/Qzx8qnXmHvf6 -8caqXJthylrSzH1HVGGjEzARMA8GA1UdEwEB/wQFMAMBAf8wCgYIKoZIzj0EAwID -RwAwRAIgJklSU+eEIOc+oUHa/ugiNsBpTsrrrJudXuZ41hayNQsCIEwxc3quig63 -ZI7LhYni7wlyXJBiXUlQkuacCGdqkv+/ ------END CERTIFICATE----- diff --git a/v3/cmd/zlint/ctx.toml b/v3/cmd/zlint/ctx.toml deleted file mode 100644 index 641bbdf3a..000000000 --- a/v3/cmd/zlint/ctx.toml +++ /dev/null @@ -1,35 +0,0 @@ - -[e_ca_too_few_beers] -# This MUST be set to the number of bottles of beer that were on the signing CAs wall at the time of issuance. -BottlesOfBeerOnTheWall = 0 -Cosigner = """ ------BEGIN CERTIFICATE----- -MIIDAzCCAmygAwIBAQIETLCaTzANBgkqhkiG9w0BAQQFADCBuDELMAkGA1UEBhMC -SUwxCzAJBgNVBAgTAklMMRUwEwYDVQQHEwxEZW1vIEFkZHJlc3MxDjAMBgNVBBET -BTEyMzQ1MRgwFgYDVQQUEw8oMDAwLTApIDAwMDAwMDAxHDAaBgkqhkiG9w0BCQEW -DURlbW9AZGVtby5jb20xEjAQBgNVBAoTCURlbW8gSW5jLjENMAsGA1UECxMERGVt -bzEaMBgGA1UEAxMRdHlwby5zZ2RwYmVsbC5jb20wHhcNMTIwNDI3MDAwMDAwWhcN -MjIwNDI3MDUwMDAwWjCBuDELMAkGA1UEBhMCSUwxCzAJBgNVBAgTAklMMRUwEwYD -VQQHEwxEZW1vIEFkZHJlc3MxDjAMBgNVBBETBTEyMzQ1MRgwFgYDVQQUEw8oMDAw -LTApIDAwMDAwMDAxHDAaBgkqhkiG9w0BCQEWDURlbW9AZGVtby5jb20xEjAQBgNV -BAoTCURlbW8gSW5jLjENMAsGA1UECxMERGVtbzEaMBgGA1UEAxMRdHlwby5zZ2Rw -YmVsbC5jb20wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAKyiNTxe6BhY3Aau -rbflrC8M6DoFcdbhgIZnQq8Mf7Jiogxg0k0dawCHglBUzMuszsvTWIAxJu/hyA2W -oLLqNZglnWNjNEAywKtizq0ErePKG3pphXErKWIv7ZKPyWbdqQBxkrQ9sQPIlPBB -Mu8PGjMpcFqhYH1QijYemWHiu0ezAgMBAAGBCwNraXNoa3VzaGloggsDa2lzaGt1 -c2hpaDANBgkqhkiG9w0BAQQFAAOBgQCN1DUWa976qhQV3JQ6YF6jNJEepYKpqzLE -tQ7fZgh36wJR6kWtCETxQwKnowWPggxUwx27r4fRHrFa1WxUCGNlMKjpcEw+EK0Y -1WGePq4919wMyMdaiz6vhNVbnOhKwEcgUKrYlksDMNKPUlDUjfm/w+HdPqMxsnA+ -S5jc+5/gQg== ------END CERTIFICATE----- -""" - -[e_ca_too_few_beers.i_spy_with_my_little_eye] -# Any number of descriptions that are valid for the subject. -Descriptions = ["larger than a bread box", "smaller than a barn"] -Subject = "A car" - -[w_web_pki_cert] -# Indicates that the certificate is intended for the Web PKI. -is_web_pki = false - diff --git a/v3/cmd/zlint/main.go b/v3/cmd/zlint/main.go index 7d81bf217..ac1f3f194 100644 --- a/v3/cmd/zlint/main.go +++ b/v3/cmd/zlint/main.go @@ -214,11 +214,11 @@ func trimmedList(raw string) []string { // includeNames, excludeNames, includeSources, and excludeSources flag values in // use. func setLints() (lint.Registry, error) { - ctx, err := lint.NewConfigFromFile(config) + config, err := lint.NewConfigFromFile(config) if err != nil { return nil, err } - lint.GlobalRegistry().SetContext(ctx) + lint.GlobalRegistry().SetConfiguration(config) // If there's no filter options set, use the global registry as-is if nameFilter == "" && includeNames == "" && excludeNames == "" && includeSources == "" && excludeSources == "" { return lint.GlobalRegistry(), nil diff --git a/v3/lint/registration.go b/v3/lint/registration.go index 85e727664..1fc153d40 100644 --- a/v3/lint/registration.go +++ b/v3/lint/registration.go @@ -92,8 +92,8 @@ type Registry interface { // WriteJSON writes a description of each registered lint as // a JSON object, one object per line, to the provided writer. WriteJSON(w io.Writer) - SetContext(ctx Configuration) - GetContext() Configuration + SetConfiguration(config Configuration) + GetConfiguration() Configuration } // registryImpl implements the Registry interface to provide a global collection @@ -240,7 +240,7 @@ func (r *registryImpl) Filter(opts FilterOptions) (Registry, error) { } filteredRegistry := NewRegistry() - filteredRegistry.SetContext(r.configuration) + filteredRegistry.SetConfiguration(r.configuration) sourceExcludes := sourceListToMap(opts.ExcludeSources) sourceIncludes := sourceListToMap(opts.IncludeSources) @@ -299,11 +299,11 @@ func (r *registryImpl) WriteJSON(w io.Writer) { } } -func (r *registryImpl) SetContext(ctx Configuration) { +func (r *registryImpl) SetConfiguration(ctx Configuration) { r.configuration = ctx } -func (r *registryImpl) GetContext() Configuration { +func (r *registryImpl) GetConfiguration() Configuration { return r.configuration } diff --git a/v3/resultset.go b/v3/resultset.go index b01de58a2..0fb3c7594 100644 --- a/v3/resultset.go +++ b/v3/resultset.go @@ -38,7 +38,7 @@ func (z *ResultSet) execute(cert *x509.Certificate, registry lint.Registry) { z.Results = make(map[string]*lint.LintResult, len(registry.Names())) // Run each lints from the registry. for _, name := range registry.Names() { - res := registry.ByName(name).Execute(cert, registry.GetContext()) + res := registry.ByName(name).Execute(cert, registry.GetConfiguration()) z.Results[name] = res z.updateErrorStatePresent(res) } From 363a88b80b865c4d756010ebd2ef6796b3fc6985 Mon Sep 17 00:00:00 2001 From: christopher-henderson Date: Mon, 20 Dec 2021 06:46:17 -0800 Subject: [PATCH 04/18] drastically simplified deserialization logic --- v3/lint/base.go | 1 + v3/lint/configuration.go | 233 ++++++------------------------- v3/lint/global_configurations.go | 44 ++++++ 3 files changed, 86 insertions(+), 192 deletions(-) diff --git a/v3/lint/base.go b/v3/lint/base.go index 2b5903720..5eac4c240 100644 --- a/v3/lint/base.go +++ b/v3/lint/base.go @@ -35,6 +35,7 @@ type LintInterface interface { Execute(c *x509.Certificate) *LintResult } +// Configurable lints return a pointer into a struct that they wish to receive their configuration into. type Configurable interface { Configure() interface{} } diff --git a/v3/lint/configuration.go b/v3/lint/configuration.go index dcd4e0325..6a8d05903 100644 --- a/v3/lint/configuration.go +++ b/v3/lint/configuration.go @@ -10,10 +10,37 @@ import ( "github.com/pelletier/go-toml" ) +// Configuration is a ZLint configuration which serves as a target +// to hold the full TOML tree that is a physical ZLint configuration./ type Configuration struct { tree *toml.Tree } +// Configure attempts to deserialize the provided namespace into the provided empty interface +// +// For example, let's say that the name if your lint is MyLint, then the configuration +// file might looks something like the following. +// +// ``` +// [MyLint] +// A = 1 +// B = 2 +// ``` +// +// Given this, our target struct may look like the following. +// +// ``` +// type MytLint struct { +// A int +// B uint +// } +// ``` +// +// So deserializing into this struct would look like, +// +// ``` +// configuration.Configure(&myLint, myLint.Name()) +// ``` func (c Configuration) Configure(lint interface{}, namespace string) error { return c.deserializeConfigInto(lint, namespace) } @@ -110,15 +137,6 @@ func (c Configuration) deserializeConfigInto(target interface{}, namespace strin // into the value held by the provided interface{}. // // This procedure is recursive. -// -// gocyclo is disabled because the relatively tall switch gives this function an outsized -// cyclomatic complexity rating despite it being relatively simple to understand (if not redundant). -// This redundancy is largely because Go neither has templating nor types as values. -// -// This procedure is certainly subject to compression if there is a more clever way to do this, although -// the Golang reflect package is rather unwieldy to being with, so it may be tricky. -// -//nolint:gocyclo func (c Configuration) resolveHigherScopedReferences(i interface{}) error { value := reflect.Indirect(reflect.ValueOf(i)) if value.Kind() != reflect.Struct { @@ -142,159 +160,16 @@ func (c Configuration) resolveHigherScopedReferences(i interface{}) error { if !field.CanInterface() { continue } - var val reflect.Value - switch t := field.Interface().(type) { - case Global: - err := c.deserializeConfigInto(&t, "") - if err != nil { - return err - } - val = reflect.ValueOf(t) - case *Global: - if t == nil { - t = &Global{} - } - err := c.deserializeConfigInto(t, "") - if err != nil { - return err - } - val = reflect.ValueOf(t) - case RFC5280Config: - err := c.deserializeConfigInto(&t, string(RFC5280)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - case *RFC5280Config: - if t == nil { - t = &RFC5280Config{} - } - err := c.deserializeConfigInto(t, string(RFC5280)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - case RFC5480Config: - err := c.deserializeConfigInto(&t, string(RFC5480)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - case *RFC5480Config: - if t == nil { - t = &RFC5480Config{} - } - err := c.deserializeConfigInto(t, string(RFC5480)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - case RFC5891Config: - err := c.deserializeConfigInto(&t, string(RFC5891)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - case *RFC5891Config: - if t == nil { - t = &RFC5891Config{} + if config, ok := field.Interface().(GlobalConfiguration); ok { + if field.Kind() == reflect.Ptr && field.IsZero() { + config = reflect.New(field.Type().Elem()).Interface().(GlobalConfiguration) } - err := c.deserializeConfigInto(t, string(RFC5891)) + err := c.deserializeConfigInto(config, config.namespace()) if err != nil { return err } - val = reflect.ValueOf(t) - case CABFBaselineRequirementsConfig: - err := c.deserializeConfigInto(&t, string(CABFBaselineRequirements)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - case *CABFBaselineRequirementsConfig: - if t == nil { - t = &CABFBaselineRequirementsConfig{} - } - err := c.deserializeConfigInto(t, string(CABFBaselineRequirements)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - case CABFEVGuidelinesConfig: - err := c.deserializeConfigInto(&t, string(CABFEVGuidelines)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - case *CABFEVGuidelinesConfig: - if t == nil { - t = &CABFEVGuidelinesConfig{} - } - err := c.deserializeConfigInto(t, string(CABFEVGuidelines)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - case MozillaRootStorePolicyConfig: - err := c.deserializeConfigInto(&t, string(MozillaRootStorePolicy)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - case *MozillaRootStorePolicyConfig: - if t == nil { - t = &MozillaRootStorePolicyConfig{} - } - err := c.deserializeConfigInto(t, string(MozillaRootStorePolicy)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - case AppleRootStorePolicyConfig: - err := c.deserializeConfigInto(&t, string(AppleRootStorePolicy)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - case *AppleRootStorePolicyConfig: - if t == nil { - t = &AppleRootStorePolicyConfig{} - } - err := c.deserializeConfigInto(t, string(AppleRootStorePolicy)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - case CommunityConfig: - err := c.deserializeConfigInto(&t, string(Community)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - case *CommunityConfig: - if t == nil { - t = &CommunityConfig{} - } - err := c.deserializeConfigInto(t, string(Community)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - case EtsiEsiConfig: - err := c.deserializeConfigInto(&t, string(EtsiEsi)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - case *EtsiEsiConfig: - if t == nil { - t = &EtsiEsiConfig{} - } - err := c.deserializeConfigInto(t, string(EtsiEsi)) - if err != nil { - return err - } - val = reflect.ValueOf(t) - default: + field.Set(reflect.ValueOf(config)) + } else { // In order to deserialize into a field it does indeed need to be addressable. if !field.CanAddr() { continue @@ -305,7 +180,6 @@ func (c Configuration) resolveHigherScopedReferences(i interface{}) error { } continue } - field.Set(val) } return nil } @@ -314,7 +188,7 @@ func (c Configuration) resolveHigherScopedReferences(i interface{}) error { // the provided struct but with all references to higher scoped configurations scrubbed. // // This is intended only for use when constructing an example configuration file via the -// `-exampleConfig` flag. This is to avoid visually redundant, and possibliy incorrect, +// `-exampleConfig` flag. This is to avoid visually redundant, and possibly incorrect, // examples such as the following... // // ``` @@ -326,12 +200,7 @@ func (c Configuration) resolveHigherScopedReferences(i interface{}) error { // my_data = 0 // my_flag = false // globals = { something = false, something_else = "" } -// ``` -// -// gocyclo is disabled because the relatively tall switch gives this function an outsized -// cyclomatic complexity rating despite it being relatively simple to understand. -// -//nolint:gocyclo +// ``` o func stripGlobalsFromExample(i interface{}) interface{} { value := reflect.Indirect(reflect.ValueOf(i)) if value.Kind() != reflect.Struct { @@ -341,36 +210,16 @@ func stripGlobalsFromExample(i interface{}) interface{} { for field := 0; field < value.NumField(); field++ { name := value.Type().Field(field).Name field := value.Field(field) - if field.Kind() == reflect.Ptr { - field = reflect.Zero(field.Type().Elem()) - } if !field.CanInterface() { continue } - switch t := field.Interface().(type) { - case Global: - case *Global: - case RFC5280Config: - case *RFC5280Config: - case RFC5480Config: - case *RFC5480Config: - case RFC5891Config: - case *RFC5891Config: - case CABFBaselineRequirementsConfig: - case *CABFBaselineRequirementsConfig: - case CABFEVGuidelinesConfig: - case *CABFEVGuidelinesConfig: - case MozillaRootStorePolicyConfig: - case *MozillaRootStorePolicyConfig: - case AppleRootStorePolicyConfig: - case *AppleRootStorePolicyConfig: - case CommunityConfig: - case *CommunityConfig: - case EtsiEsiConfig: - case *EtsiEsiConfig: - default: - m[name] = stripGlobalsFromExample(t) + if _, ok := field.Interface().(GlobalConfiguration); ok { + continue + } + if field.Kind() == reflect.Ptr && field.IsZero() { + field = reflect.New(field.Type().Elem()) } + m[name] = stripGlobalsFromExample(field.Interface()) } return m } diff --git a/v3/lint/global_configurations.go b/v3/lint/global_configurations.go index 5e5bc0cc7..9bb250bdf 100644 --- a/v3/lint/global_configurations.go +++ b/v3/lint/global_configurations.go @@ -12,6 +12,10 @@ package lint // The fields `some_flag` and `some_string` will be targeted to land into this struct. type Global struct{} +func (g Global) namespace() string { + return "Global" +} + // RFC5280Config is the higher scoped configuration which services as the deserialization target for... // // [RFC5280Config] @@ -19,6 +23,10 @@ type Global struct{} // ... type RFC5280Config struct{} +func (r RFC5280Config) namespace() string { + return "RFC5280Config" +} + // RFC5480Config is the higher scoped configuration which services as the deserialization target for... // // [RFC5480Config] @@ -26,6 +34,10 @@ type RFC5280Config struct{} // ... type RFC5480Config struct{} +func (r RFC5480Config) namespace() string { + return "RFC5480Config" +} + // RFC5891Config is the higher scoped configuration which services as the deserialization target for... // // [RFC5891Config] @@ -33,6 +45,10 @@ type RFC5480Config struct{} // ... type RFC5891Config struct{} +func (r RFC5891Config) namespace() string { + return "RFC5891Config" +} + // CABFBaselineRequirementsConfig is the higher scoped configuration which services as the deserialization target for... // // [CABFBaselineRequirementsConfig] @@ -40,6 +56,10 @@ type RFC5891Config struct{} // ... type CABFBaselineRequirementsConfig struct{} +func (c CABFBaselineRequirementsConfig) namespace() string { + return "CABFBaselineRequirementsConfig" +} + // CABFEVGuidelinesConfig is the higher scoped configuration which services as the deserialization target for... // // [CABFEVGuidelinesConfig] @@ -47,6 +67,10 @@ type CABFBaselineRequirementsConfig struct{} // ... type CABFEVGuidelinesConfig struct{} +func (c CABFEVGuidelinesConfig) namespace() string { + return "CABFEVGuidelinesConfig" +} + // MozillaRootStorePolicyConfig is the higher scoped configuration which services as the deserialization target for... // // [MozillaRootStorePolicyConfig] @@ -54,6 +78,10 @@ type CABFEVGuidelinesConfig struct{} // ... type MozillaRootStorePolicyConfig struct{} +func (m MozillaRootStorePolicyConfig) namespace() string { + return "MozillaRootStorePolicyConfig" +} + // AppleRootStorePolicyConfig is the higher scoped configuration which services as the deserialization target for... // // [AppleRootStorePolicyConfig] @@ -61,6 +89,10 @@ type MozillaRootStorePolicyConfig struct{} // ... type AppleRootStorePolicyConfig struct{} +func (a AppleRootStorePolicyConfig) namespace() string { + return "AppleRootStorePolicyConfig" +} + // CommunityConfig is the higher scoped configuration which services as the deserialization target for... // // [CommunityConfig] @@ -68,9 +100,21 @@ type AppleRootStorePolicyConfig struct{} // ... type CommunityConfig struct{} +func (c CommunityConfig) namespace() string { + return "CommunityConfig" +} + // EtsiEsiConfig is the higher scoped configuration which services as the deserialization target for... // // [EtsiEsiConfig] // ... // ... type EtsiEsiConfig struct{} + +func (e EtsiEsiConfig) namespace() string { + return "EtsiEsiConfig" +} + +type GlobalConfiguration interface { + namespace() string +} From 6dd03c6be945f987417d15eade92918c771332ca Mon Sep 17 00:00:00 2001 From: christopher-henderson Date: Mon, 20 Dec 2021 06:56:26 -0800 Subject: [PATCH 05/18] that's a bad lint --- v3/lint/configuration.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/v3/lint/configuration.go b/v3/lint/configuration.go index 6a8d05903..ccf682aa1 100644 --- a/v3/lint/configuration.go +++ b/v3/lint/configuration.go @@ -160,6 +160,8 @@ func (c Configuration) resolveHigherScopedReferences(i interface{}) error { if !field.CanInterface() { continue } + // The linter doesn't like there is an if-statement inside a for-loop, which is frankly a bogus and useless lint. + //nolint:nestif if config, ok := field.Interface().(GlobalConfiguration); ok { if field.Kind() == reflect.Ptr && field.IsZero() { config = reflect.New(field.Type().Elem()).Interface().(GlobalConfiguration) From bfa8e73a3e2020e7f94dec9226e3624b8d65e00a Mon Sep 17 00:00:00 2001 From: christopher-henderson Date: Sat, 8 Jan 2022 12:36:04 -0800 Subject: [PATCH 06/18] fixing typos, adding more tests, adding more utils for tricky code --- CONTRIBUTING.md | 7 ++-- v3/lint/base.go | 10 +++--- v3/lint/configuration.go | 61 ++++++++++++++++++++------------ v3/lint/configuration_test.go | 31 ++++++++++++++++ v3/lint/global_configurations.go | 5 +++ v3/lint/registration.go | 34 ++++++++++-------- 6 files changed, 100 insertions(+), 48 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9f2ace831..95e8e05f2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -247,8 +247,8 @@ Please see the [integration tests README] for more information. ### Testing Configurable Lints -Testing a lint that is configurable is much the same as testing on that is not. However, if you wish exercise -various configurations then you may do by utilizing the `test.TestLintWithConfig` function which takes in an extra +Testing a lint that is configurable is much the same as testing one that is not. However, if you wish to exercise +various configurations then you may do so by utilizing the `test.TestLintWithConfig` function which takes in an extra string which is the raw TOML of your target test configuration. ```go @@ -260,9 +260,6 @@ func TestCaCommonNameNotMissing2(t *testing.T) { BeerHall = "liedershousen" ` out := test.TestLintWithConfig("e_ca_common_name_missing2", inputPath, config) - if out.Details != "liedershousen" { - panic("noooooooooooo") - } if out.Status != expected { t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status) } diff --git a/v3/lint/base.go b/v3/lint/base.go index 5eac4c240..084ff58de 100644 --- a/v3/lint/base.go +++ b/v3/lint/base.go @@ -93,6 +93,7 @@ func (l *Lint) CheckEffective(c *x509.Certificate) bool { // if they are within the purview of the BRs. See LintInterface for details // about the other methods called. The ordering is as follows: // +// Configure() ----> only if the lint implements Configurable // CheckApplies() // CheckEffective() // Execute() @@ -100,7 +101,10 @@ func (l *Lint) Execute(cert *x509.Certificate, config Configuration) *LintResult if l.Source == CABFBaselineRequirements && !util.IsServerAuthCert(cert) { return &LintResult{Status: NA} } - lint := l.Lint() + return l.execute(l.Lint(), cert, config) +} + +func (l *Lint) execute(lint LintInterface, cert *x509.Certificate, config Configuration) *LintResult { switch configurable := lint.(type) { case Configurable: err := config.Configure(configurable.Configure(), l.Name) @@ -116,10 +120,6 @@ func (l *Lint) Execute(cert *x509.Certificate, config Configuration) *LintResult Details: details} } } - return l.execute(lint, cert) -} - -func (l *Lint) execute(lint LintInterface, cert *x509.Certificate) *LintResult { if !lint.CheckApplies(cert) { return &LintResult{Status: NA} } else if !l.CheckEffective(cert) { diff --git a/v3/lint/configuration.go b/v3/lint/configuration.go index ccf682aa1..2708c23ee 100644 --- a/v3/lint/configuration.go +++ b/v3/lint/configuration.go @@ -16,10 +16,10 @@ type Configuration struct { tree *toml.Tree } -// Configure attempts to deserialize the provided namespace into the provided empty interface +// Configure attempts to deserialize the provided namespace into the provided empty interface. // -// For example, let's say that the name if your lint is MyLint, then the configuration -// file might looks something like the following. +// For example, let's say that the name of your lint is MyLint, then the configuration +// file might look something like the following... // // ``` // [MyLint] @@ -27,7 +27,7 @@ type Configuration struct { // B = 2 // ``` // -// Given this, our target struct may look like the following. +// Given this, our target struct may look like the following... // // ``` // type MytLint struct { @@ -36,7 +36,7 @@ type Configuration struct { // } // ``` // -// So deserializing into this struct would look like, +// So deserializing into this struct would look like... // // ``` // configuration.Configure(&myLint, myLint.Name()) @@ -59,7 +59,8 @@ func NewConfig(r io.Reader) (Configuration, error) { // NewConfigFromFile attempts to instantiate a configuration from the provided filesystem path. // -// The file pointed to by `path` MUST be valid TOML file. +// The file pointed to by `path` MUST be valid TOML file. If `path` is the empty string then +// an empty configuration is returned. func NewConfigFromFile(path string) (Configuration, error) { if path == "" { return NewEmptyConfig(), nil @@ -81,7 +82,7 @@ func NewConfigFromString(config string) (Configuration, error) { // NewEmptyConfig returns a configuration that is backed by an entirely empty TOML tree. // -// This is useful of no particular configuration is set at all by the user of ZLint as +// This is useful if no particular configuration is set at all by the user of ZLint as // any attempt to resolve a namespace in `deserializeConfigInto` fails and thus results // in all defaults for all lints being maintained. func NewEmptyConfig() Configuration { @@ -111,7 +112,7 @@ func NewEmptyConfig() Configuration { // } // ``` // -// Then the invocation of this function should be +// Then the invocation of this function should be... // // ``` // lint := &SomeOtherLink{} @@ -133,21 +134,20 @@ func (c Configuration) deserializeConfigInto(target interface{}, namespace strin // resolveHigherScopeReferences takes in an interface{} value and attempts to // find any field within its inner value that is either a struct or a pointer // to a struct that is one of our global configurable types. If such a field -// exists then that higher scoped configuration will be deserialized, in-place, -// into the value held by the provided interface{}. +// exists then that higher scoped configuration will be copied into the value +// held by the provided interface{}. // // This procedure is recursive. func (c Configuration) resolveHigherScopedReferences(i interface{}) error { value := reflect.Indirect(reflect.ValueOf(i)) if value.Kind() != reflect.Struct { // Our target higher scoped configurations are either structs - // or are fields of structs. Any other Kind is simply cannot + // or are fields of structs. Any other Kind simply cannot // be a target for deserialization here. For example, an interface // does not make sense since an interface cannot have fields nor // are any of our higher scoped configurations interfaces themselves. // - // For a comprehensive list of Kinds you may review type.go in the - // `reflect` package. + // For a comprehensive list of Kinds, please see `type.go` in the `reflect` package. return nil } // Iterate through every field within the struct held by the provided interface{}. @@ -160,19 +160,23 @@ func (c Configuration) resolveHigherScopedReferences(i interface{}) error { if !field.CanInterface() { continue } - // The linter doesn't like there is an if-statement inside a for-loop, which is frankly a bogus and useless lint. + // The linter doesn't like that there is an if-statement inside a for-loop, + // which is frankly kind of a bogus and useless lint. + // //nolint:nestif if config, ok := field.Interface().(GlobalConfiguration); ok { - if field.Kind() == reflect.Ptr && field.IsZero() { - config = reflect.New(field.Type().Elem()).Interface().(GlobalConfiguration) - } + // It's one of our higher level configurations. + config = initializePtr(field).Interface().(GlobalConfiguration) err := c.deserializeConfigInto(config, config.namespace()) if err != nil { return err } field.Set(reflect.ValueOf(config)) } else { - // In order to deserialize into a field it does indeed need to be addressable. + // This is just another member of some kind that is not one of our higher level configurations. + // + // In order to deserialize into a field it does indeed need to be addressable, otherwise + // boxing it into an interface{} fails fantastically. if !field.CanAddr() { continue } @@ -180,7 +184,6 @@ func (c Configuration) resolveHigherScopedReferences(i interface{}) error { if err != nil { return err } - continue } } return nil @@ -202,7 +205,10 @@ func (c Configuration) resolveHigherScopedReferences(i interface{}) error { // my_data = 0 // my_flag = false // globals = { something = false, something_else = "" } -// ``` o +// ``` +// +// Notice how the above has Global effectively listed twice - once externally and once internally, which +// defeats the whole point of having globals to begin with. func stripGlobalsFromExample(i interface{}) interface{} { value := reflect.Indirect(reflect.ValueOf(i)) if value.Kind() != reflect.Struct { @@ -218,10 +224,19 @@ func stripGlobalsFromExample(i interface{}) interface{} { if _, ok := field.Interface().(GlobalConfiguration); ok { continue } - if field.Kind() == reflect.Ptr && field.IsZero() { - field = reflect.New(field.Type().Elem()) - } + field = initializePtr(field) m[name] = stripGlobalsFromExample(field.Interface()) } return m } + +// initializePtr checks whether the provided reflect.Value is a pointer type and is nil. If so, it returns +// a new reflect.Value that has an initialized pointer. +// +// If the provided reflect.Value is not a nil pointer, then the original reflect.Value is returned. +func initializePtr(value reflect.Value) reflect.Value { + if value.Kind() == reflect.Ptr && value.IsZero() { + return reflect.New(value.Type().Elem()) + } + return value +} diff --git a/v3/lint/configuration_test.go b/v3/lint/configuration_test.go index c9df07371..e2668d00e 100644 --- a/v3/lint/configuration_test.go +++ b/v3/lint/configuration_test.go @@ -880,3 +880,34 @@ func TestGlobalStripper(t *testing.T) { t.Fatalf("wanted Test{EtsiEsiConfig: &EtsiEsiConfig{}, SomethingElse: \"cool\"}} got %v", test) } } + +func TestPrintConfiguration(t *testing.T) { + gotBytes, err := NewRegistry().DefaultConfiguration() + if err != nil { + t.Fatal(err) + } + got := string(gotBytes) + // I'm not a huge fan of this sort of test since it will have to be updated + // on the slightest change, but it's better than not have a test for printing + // out the configuration file. + want := ` +[AppleRootStorePolicyConfig] + +[CABFBaselineRequirementsConfig] + +[CABFEVGuidelinesConfig] + +[CommunityConfig] + +[MozillaRootStorePolicyConfig] + +[RFC5280Config] + +[RFC5480Config] + +[RFC5891Config] +` + if got != want { + t.Fatalf("wanted '%s' but got '%s'", want, got) + } +} diff --git a/v3/lint/global_configurations.go b/v3/lint/global_configurations.go index 9bb250bdf..4c0a7b7f3 100644 --- a/v3/lint/global_configurations.go +++ b/v3/lint/global_configurations.go @@ -115,6 +115,11 @@ func (e EtsiEsiConfig) namespace() string { return "EtsiEsiConfig" } +// GlobalConfiguration acts both as an interface that can be used to obtain the TOML namespace of configuration +// as well as a way to mark a fielf in a struct as one of our own, higher scoped, configurations. +// +// the interface itself is public, however the singular `namespace` method is package private, meaning that +// normal lint struct cannot accidentally implement this. type GlobalConfiguration interface { namespace() string } diff --git a/v3/lint/registration.go b/v3/lint/registration.go index 1fc153d40..e6713e53d 100644 --- a/v3/lint/registration.go +++ b/v3/lint/registration.go @@ -135,7 +135,7 @@ func (e errDuplicateName) Error() string { e.lintName) } -// register adds the provided lint to the Registry. If initialize is true then +// register adds the provided lint to the Registry. If initializePtr is true then // the lint's Initialize() function will be called before registering the lint. // // An error is returned if the lint or lint's Lint pointer is nil, if the Lint @@ -280,7 +280,7 @@ func (r *registryImpl) Filter(opts FilterOptions) (Registry, error) { } // when adding lints to a filtered registry we do not want Initialize() to - // be called a second time, so provide false as the initialize argument. + // be called a second time, so provide false as the initializePtr argument. if err := filteredRegistry.register(l); err != nil { return nil, err } @@ -322,29 +322,33 @@ func (r *registryImpl) DefaultConfiguration() ([]byte, error) { // At this point, most lints are not configurable. } } + higherScopedConfigs := []GlobalConfiguration{ + &CABFBaselineRequirementsConfig{}, + &RFC5280Config{}, + &RFC5480Config{}, + &RFC5891Config{}, + &CABFBaselineRequirementsConfig{}, + &CABFEVGuidelinesConfig{}, + &MozillaRootStorePolicyConfig{}, + &AppleRootStorePolicyConfig{}, + &CommunityConfig{}, + } + for _, config := range higherScopedConfigs { + configurables[config.namespace()] = config + } // We're just using stripGlobalsFromExample here as a convenient way to // recursively turn the `Global` struct type into a map. // - // We have to do this because if simply followed the pattern below and did... + // We have to do this because if we simply followed the pattern above and did... // // configurables["Global"] = &Global{} // // ...then we would end up with a [Global] section in the resulting configuration, - // which is not what we are looking for (we simply want it to flattened out into + // which is not what we are looking for (we simply want it to be flattened out into // the top most context of the configuration file). for k, v := range stripGlobalsFromExample(&Global{}).(map[string]interface{}) { configurables[k] = v } - configurables[string(CABFBaselineRequirements)] = &CABFBaselineRequirementsConfig{} - configurables[string(RFC5280)] = &RFC5280Config{} - configurables[string(RFC5480)] = &RFC5480Config{} - configurables[string(RFC5891)] = &RFC5891Config{} - configurables[string(CABFBaselineRequirements)] = &CABFBaselineRequirementsConfig{} - configurables[string(CABFEVGuidelines)] = &CABFEVGuidelinesConfig{} - configurables[string(MozillaRootStorePolicy)] = &MozillaRootStorePolicyConfig{} - configurables[string(AppleRootStorePolicy)] = &AppleRootStorePolicyConfig{} - configurables[string(Community)] = &CommunityConfig{} - configurables[string(EtsiEsi)] = &EtsiEsiConfig{} w := &bytes.Buffer{} err := toml.NewEncoder(w).Indentation("").CompactComments(true).Encode(configurables) if err != nil { @@ -377,7 +381,7 @@ var globalRegistry *registryImpl = NewRegistry() // name matches a previously registered lint's name. These conditions all // indicate a bug that should be addressed by a developer. func RegisterLint(l *Lint) { - // RegisterLint always sets initialize to true. It's assumed this is called by + // RegisterLint always sets initializePtr to true. It's assumed this is called by // the package init() functions and therefore must be doing the first // initialization of a lint. if err := globalRegistry.register(l); err != nil { From 3f5549caeadcdbed7f287d770713b2cfcaae0c3a Mon Sep 17 00:00:00 2001 From: christopher-henderson Date: Sat, 8 Jan 2022 12:58:15 -0800 Subject: [PATCH 07/18] linter complained about name --- v3/lint/configuration.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/v3/lint/configuration.go b/v3/lint/configuration.go index 2708c23ee..103256303 100644 --- a/v3/lint/configuration.go +++ b/v3/lint/configuration.go @@ -164,9 +164,9 @@ func (c Configuration) resolveHigherScopedReferences(i interface{}) error { // which is frankly kind of a bogus and useless lint. // //nolint:nestif - if config, ok := field.Interface().(GlobalConfiguration); ok { + if _, ok := field.Interface().(GlobalConfiguration); ok { // It's one of our higher level configurations. - config = initializePtr(field).Interface().(GlobalConfiguration) + config := initializePtr(field).Interface().(GlobalConfiguration) err := c.deserializeConfigInto(config, config.namespace()) if err != nil { return err From a15789be72979a98e792171e6cc49f8bd8d185a5 Mon Sep 17 00:00:00 2001 From: christopher-henderson Date: Sun, 9 Jan 2022 11:37:38 -0800 Subject: [PATCH 08/18] fixing goland's overzealous comment refactoring --- v3/lint/registration.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/v3/lint/registration.go b/v3/lint/registration.go index e6713e53d..d87cd66cc 100644 --- a/v3/lint/registration.go +++ b/v3/lint/registration.go @@ -135,7 +135,7 @@ func (e errDuplicateName) Error() string { e.lintName) } -// register adds the provided lint to the Registry. If initializePtr is true then +// register adds the provided lint to the Registry. If initialize is true then // the lint's Initialize() function will be called before registering the lint. // // An error is returned if the lint or lint's Lint pointer is nil, if the Lint @@ -280,7 +280,7 @@ func (r *registryImpl) Filter(opts FilterOptions) (Registry, error) { } // when adding lints to a filtered registry we do not want Initialize() to - // be called a second time, so provide false as the initializePtr argument. + // be called a second time, so provide false as the initialize argument. if err := filteredRegistry.register(l); err != nil { return nil, err } @@ -381,7 +381,7 @@ var globalRegistry *registryImpl = NewRegistry() // name matches a previously registered lint's name. These conditions all // indicate a bug that should be addressed by a developer. func RegisterLint(l *Lint) { - // RegisterLint always sets initializePtr to true. It's assumed this is called by + // RegisterLint always sets initialize to true. It's assumed this is called by // the package init() functions and therefore must be doing the first // initialization of a lint. if err := globalRegistry.register(l); err != nil { From b1cf6056525699c0afa3d44d91e6901260b24f80 Mon Sep 17 00:00:00 2001 From: christopher-henderson Date: Sat, 22 Jan 2022 11:18:08 -0800 Subject: [PATCH 09/18] made it to 100% code coverage for the core logic --- v3/lint/configuration.go | 26 ++- v3/lint/configuration_test.go | 274 +++++++++++++++++++++++++++++++ v3/lint/global_configurations.go | 30 ++++ v3/lint/registration.go | 53 +++--- 4 files changed, 348 insertions(+), 35 deletions(-) diff --git a/v3/lint/configuration.go b/v3/lint/configuration.go index 103256303..1b5347fdc 100644 --- a/v3/lint/configuration.go +++ b/v3/lint/configuration.go @@ -1,3 +1,17 @@ +/* + * ZLint Copyright 2021 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 lint import ( @@ -157,7 +171,8 @@ func (c Configuration) resolveHigherScopedReferences(i interface{}) error { // in an attempt to resolve it. for field := 0; field < value.NumField(); field++ { field := value.Field(field) - if !field.CanInterface() { + if !field.CanSet() { + // This skips fields that are either not addressable or are private data members. continue } // The linter doesn't like that there is an if-statement inside a for-loop, @@ -165,7 +180,8 @@ func (c Configuration) resolveHigherScopedReferences(i interface{}) error { // //nolint:nestif if _, ok := field.Interface().(GlobalConfiguration); ok { - // It's one of our higher level configurations. + // It's one of our higher level configurations, so we need to pull out a different + // subtree from our TOML document and inject it int othis struct. config := initializePtr(field).Interface().(GlobalConfiguration) err := c.deserializeConfigInto(config, config.namespace()) if err != nil { @@ -174,12 +190,6 @@ func (c Configuration) resolveHigherScopedReferences(i interface{}) error { field.Set(reflect.ValueOf(config)) } else { // This is just another member of some kind that is not one of our higher level configurations. - // - // In order to deserialize into a field it does indeed need to be addressable, otherwise - // boxing it into an interface{} fails fantastically. - if !field.CanAddr() { - continue - } err := c.resolveHigherScopedReferences(field.Addr().Interface()) if err != nil { return err diff --git a/v3/lint/configuration_test.go b/v3/lint/configuration_test.go index e2668d00e..ac3f2f66b 100644 --- a/v3/lint/configuration_test.go +++ b/v3/lint/configuration_test.go @@ -1,8 +1,23 @@ +/* + * ZLint Copyright 2021 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 lint import ( "io" "io/ioutil" + "os" "reflect" "sync" "testing" @@ -419,6 +434,32 @@ func TestBadToml(t *testing.T) { } } +func TestPrivateMembers(t *testing.T) { + type Test struct { + private string + NotPrivate string + } + c, err := NewConfigFromString(` +[Test] +private = "this still should not show up" +NotPrivate = "just a string" +`) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if test.private != "" { + t.Errorf("wanted '' got '%s'", test.private) + } + if test.NotPrivate != "just a string" { + t.Errorf("wanted 'just a string' got '%s'", test.NotPrivate) + } +} + func TestEmbedGlobal(t *testing.T) { type Test struct { Global Global @@ -911,3 +952,236 @@ func TestPrintConfiguration(t *testing.T) { t.Fatalf("wanted '%s' but got '%s'", want, got) } } + +type TestGlobalConfigurable struct { + A int + B string +} + +func (t *TestGlobalConfigurable) namespace() string { + return "this_is_a_test" +} + +func TestNewGlobal(t *testing.T) { + type test struct { + SomethingElse string `toml:"something_else"` + T *TestGlobalConfigurable + } + c, err := NewConfigFromString(` +[this_is_a_test] +A = 1 +B = "the temples of syrinx" + +[Test] +something_else = "fills our hallowed halls" +`) + if err != nil { + t.Fatal(err) + } + got := test{} + err = c.Configure(&got, "Test") + if err != nil { + t.Fatal(err) + } + if got.SomethingElse != "fills our hallowed halls" { + t.Errorf("got '%s' want 'fills our hallowed halls", got.SomethingElse) + } + if got.T.A != 1 { + t.Errorf("got %d want 1", got.T.A) + } + if got.T.B != "the temples of syrinx" { + t.Errorf("got '%s' want 'the temples of syrinx", got.T.B) + } +} + +type TestGlobalConfigurableWithPrivates struct { + A int + B string + c string +} + +func (t *TestGlobalConfigurableWithPrivates) namespace() string { + return "this_is_a_test" +} + +func TestNewGlobalWithPrivateMembersDontGetPrinted(t *testing.T) { + gotBytes, err := NewRegistry().defaultConfiguration([]GlobalConfiguration{&TestGlobalConfigurableWithPrivates{ + 1, "2", "3", + }}) + if err != nil { + t.Fatal(err) + } + got := string(gotBytes) + // I'm not a huge fan of this sort of test since it will have to be updated + // on the slightest change, but it's better than not have a test for printing + // out the configuration file. + want := ` +[this_is_a_test] +A = 1 +B = "2" +` + if got != want { + t.Fatalf("wanted '%s' but got '%s'", want, got) + } +} + +func TestFailedGlobalDeser(t *testing.T) { + type test struct { + SomethingElse string `toml:"something_else"` + T *TestGlobalConfigurable + } + c, err := NewConfigFromString(` +[this_is_a_test] +A = "1" # It should be an int, not a string +B = "the temples of syrinx" + +[Test] +something_else = "fills our hallowed halls" +`) + if err != nil { + t.Fatal(err) + } + got := test{} + err = c.Configure(&got, "Test") + if err == nil { + t.Fatalf("expected error, but got %v", got) + } +} + +func TestFailedNestedGlobalDeser(t *testing.T) { + type test struct { + SomethingElse string `toml:"something_else"` + Inner struct { + T *TestGlobalConfigurable + } + } + c, err := NewConfigFromString(` +[this_is_a_test] +A = "1" # It should be an int, not a string +B = "the temples of syrinx" + +[Test] +something_else = "fills our hallowed halls" +`) + if err != nil { + t.Fatal(err) + } + got := test{} + err = c.Configure(&got, "Test") + if err == nil { + t.Fatalf("expected error, but got %v", got) + } +} + +func TestStripGlobalsFromStructWithPrivates(t *testing.T) { + type Test struct { + A string + B Global + C int + d int + } + test := Test{} + got := stripGlobalsFromExample(&test).(map[string]interface{}) + want := map[string]interface{}{ + "A": "", + "C": 0, + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("wanted map[A: C:0], got %v", got) + } +} + +func TestNewEmptyConfig(t *testing.T) { + c := NewEmptyConfig() + got, err := c.tree.Marshal() + if err != nil { + t.Fatal(err) + } + if got != nil { + t.Fatalf("wanted nil byte slice, got %s", string(got)) + } +} + +func TestConfigFromFile(t *testing.T) { + type Test struct { + A *Test + B bool + } + f, err := os.CreateTemp("", "") + if err != nil { + t.Fatal(err) + } + defer os.Remove(f.Name()) + _, err = f.WriteString(` +[Test] +A = { B = true } +B = true +`) + if err != nil { + f.Close() + t.Fatal(err) + } + err = f.Close() + if err != nil { + t.Fatal(err) + } + c, err := NewConfigFromFile(f.Name()) + if err != nil { + t.Fatal(err) + } + test := Test{} + err = c.Configure(&test, "Test") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(test, Test{&Test{nil, true}, true}) { + t.Fatalf("wanted Test{&Test{nil, true}, true} got %v", test) + } +} + +func TestBadConfigFromFile(t *testing.T) { + f, err := os.CreateTemp("", "") + if err != nil { + t.Fatal(err) + } + defer os.Remove(f.Name()) + _, err = f.WriteString(` +nope not gonna work +[Test] +A = { B = true } +B = true +`) + if err != nil { + f.Close() + t.Fatal(err) + } + err = f.Close() + if err != nil { + t.Fatal(err) + } + c, err := NewConfigFromFile(f.Name()) + if err == nil { + t.Fatalf("expected error, got %v", c) + } +} + +func TestEmptyConfigFromEmptyPath(t *testing.T) { + c, err := NewConfigFromFile("") + if err != nil { + t.Fatal(err) + } + got, err := c.tree.Marshal() + if err != nil { + t.Fatal(err) + } + if got != nil { + t.Fatalf("wanted nil byte slice, got %s", string(got)) + } +} + +func TestFailedToOpenConfigFile(t *testing.T) { + c, err := NewConfigFromFile("lol no not likely") + if err == nil { + t.Fatalf("expected an error got %v", c) + } +} diff --git a/v3/lint/global_configurations.go b/v3/lint/global_configurations.go index 4c0a7b7f3..826734ed3 100644 --- a/v3/lint/global_configurations.go +++ b/v3/lint/global_configurations.go @@ -1,3 +1,17 @@ +/* + * ZLint Copyright 2021 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 lint // Global is what one would intuitive think of as being the global context of the configuration file. @@ -123,3 +137,19 @@ func (e EtsiEsiConfig) namespace() string { type GlobalConfiguration interface { namespace() string } + +// defaultGlobals are used by other locations in the codebase that may want to iterate over all currently know +// global configuration types. Most notably, Registry.DefaultConfiguration uses it because it wants to print +// out a TOML document that is the full default configuration for ZLint. +var defaultGlobals = []GlobalConfiguration{ + &Global{}, + &CABFBaselineRequirementsConfig{}, + &RFC5280Config{}, + &RFC5480Config{}, + &RFC5891Config{}, + &CABFBaselineRequirementsConfig{}, + &CABFEVGuidelinesConfig{}, + &MozillaRootStorePolicyConfig{}, + &AppleRootStorePolicyConfig{}, + &CommunityConfig{}, +} diff --git a/v3/lint/registration.go b/v3/lint/registration.go index d87cd66cc..8f0a45394 100644 --- a/v3/lint/registration.go +++ b/v3/lint/registration.go @@ -313,41 +313,40 @@ func (r *registryImpl) GetConfiguration() Configuration { // to stdout. In this way, operators can quickly see what lints are configurable and what their // fields are without having to dig through documentation or, even worse, code. func (r *registryImpl) DefaultConfiguration() ([]byte, error) { + return r.defaultConfiguration(defaultGlobals) +} + +// defaultConfiguration is abstracted out to a private function that takes in a slice of globals +// for the sake of making unit testing easier. +func (r *registryImpl) defaultConfiguration(globals []GlobalConfiguration) ([]byte, error) { configurables := map[string]interface{}{} for name, lint := range r.lintsByName { switch configurable := lint.Lint().(type) { case Configurable: configurables[name] = stripGlobalsFromExample(configurable.Configure()) default: - // At this point, most lints are not configurable. } } - higherScopedConfigs := []GlobalConfiguration{ - &CABFBaselineRequirementsConfig{}, - &RFC5280Config{}, - &RFC5480Config{}, - &RFC5891Config{}, - &CABFBaselineRequirementsConfig{}, - &CABFEVGuidelinesConfig{}, - &MozillaRootStorePolicyConfig{}, - &AppleRootStorePolicyConfig{}, - &CommunityConfig{}, - } - for _, config := range higherScopedConfigs { - configurables[config.namespace()] = config - } - // We're just using stripGlobalsFromExample here as a convenient way to - // recursively turn the `Global` struct type into a map. - // - // We have to do this because if we simply followed the pattern above and did... - // - // configurables["Global"] = &Global{} - // - // ...then we would end up with a [Global] section in the resulting configuration, - // which is not what we are looking for (we simply want it to be flattened out into - // the top most context of the configuration file). - for k, v := range stripGlobalsFromExample(&Global{}).(map[string]interface{}) { - configurables[k] = v + for _, config := range globals { + switch config.(type) { + case *Global: + // We're just using stripGlobalsFromExample here as a convenient way to + // recursively turn the `Global` struct type into a map. + // + // We have to do this because if we simply followed the pattern above and did... + // + // configurables["Global"] = &Global{} + // + // ...then we would end up with a [Global] section in the resulting configuration, + // which is not what we are looking for (we simply want it to be flattened out into + // the top most context of the configuration file). + for k, v := range stripGlobalsFromExample(config).(map[string]interface{}) { + configurables[k] = v + } + default: + configurables[config.namespace()] = config + } + } w := &bytes.Buffer{} err := toml.NewEncoder(w).Indentation("").CompactComments(true).Encode(configurables) From 913dfca022b581712047d17b70d339ac46811b5e Mon Sep 17 00:00:00 2001 From: christopher-henderson Date: Sat, 22 Jan 2022 11:27:54 -0800 Subject: [PATCH 10/18] linter complaints --- v3/lint/configuration.go | 4 ---- v3/lint/configuration_test.go | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/v3/lint/configuration.go b/v3/lint/configuration.go index 1b5347fdc..a471552a1 100644 --- a/v3/lint/configuration.go +++ b/v3/lint/configuration.go @@ -175,10 +175,6 @@ func (c Configuration) resolveHigherScopedReferences(i interface{}) error { // This skips fields that are either not addressable or are private data members. continue } - // The linter doesn't like that there is an if-statement inside a for-loop, - // which is frankly kind of a bogus and useless lint. - // - //nolint:nestif if _, ok := field.Interface().(GlobalConfiguration); ok { // It's one of our higher level configurations, so we need to pull out a different // subtree from our TOML document and inject it int othis struct. diff --git a/v3/lint/configuration_test.go b/v3/lint/configuration_test.go index ac3f2f66b..e306d067e 100644 --- a/v3/lint/configuration_test.go +++ b/v3/lint/configuration_test.go @@ -1074,6 +1074,7 @@ something_else = "fills our hallowed halls" } func TestStripGlobalsFromStructWithPrivates(t *testing.T) { + //nolint:staticheck type Test struct { A string B Global From f9248a4a5f6dc4d6e2d9cfb3ed36c762a102ec97 Mon Sep 17 00:00:00 2001 From: Christopher Henderson Date: Sun, 24 Apr 2022 12:21:00 -0700 Subject: [PATCH 11/18] Update README.md Co-authored-by: Zakir Durumeric --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 61633a428..fe95aabca 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,7 @@ Example ZLint CLI usage: zlint -excludeSources=ETSI_ESI mycert.pem echo "Receive a copy of the full (default) configuration for all configurable lints" - zlint -exampleConfing + zlint -exampleConfig echo "Lint mycert.pem using a custom configuration for any configurable lints" zlint -config configFile.toml mycert.pem From e729b19fb20f1fb12ce7ad6e0d78c429a1efd925 Mon Sep 17 00:00:00 2001 From: Christopher Henderson Date: Sun, 24 Apr 2022 12:27:52 -0700 Subject: [PATCH 12/18] Update v3/cmd/zlint/main.go Co-authored-by: Zakir Durumeric --- v3/cmd/zlint/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/v3/cmd/zlint/main.go b/v3/cmd/zlint/main.go index ac1f3f194..18a75eb74 100644 --- a/v3/cmd/zlint/main.go +++ b/v3/cmd/zlint/main.go @@ -102,7 +102,7 @@ func main() { if exampleConfig { b, err := registry.DefaultConfiguration() if err != nil { - fmt.Println(err) + log.Fatalf("a critical error occurred while generating a configuration file, %s", err) os.Exit(1) } fmt.Printf("%s\n", string(b)) From 6d0dc7008f15fbc3980e03000e880c8b71e21635 Mon Sep 17 00:00:00 2001 From: Christopher Henderson Date: Sun, 24 Apr 2022 12:28:25 -0700 Subject: [PATCH 13/18] Update v3/cmd/zlint/main.go --- v3/cmd/zlint/main.go | 1 - 1 file changed, 1 deletion(-) diff --git a/v3/cmd/zlint/main.go b/v3/cmd/zlint/main.go index 18a75eb74..29ced1383 100644 --- a/v3/cmd/zlint/main.go +++ b/v3/cmd/zlint/main.go @@ -103,7 +103,6 @@ func main() { b, err := registry.DefaultConfiguration() if err != nil { log.Fatalf("a critical error occurred while generating a configuration file, %s", err) - os.Exit(1) } fmt.Printf("%s\n", string(b)) return From c477d54007654145f4d43364e192cb0104fba847 Mon Sep 17 00:00:00 2001 From: christopher-henderson Date: Sun, 24 Apr 2022 12:33:44 -0700 Subject: [PATCH 14/18] changing out type switch --- v3/lint/base.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/v3/lint/base.go b/v3/lint/base.go index 75d7a3d34..c5afe8a31 100644 --- a/v3/lint/base.go +++ b/v3/lint/base.go @@ -105,8 +105,8 @@ func (l *Lint) Execute(cert *x509.Certificate, config Configuration) *LintResult } func (l *Lint) execute(lint LintInterface, cert *x509.Certificate, config Configuration) *LintResult { - switch configurable := lint.(type) { - case Configurable: + configurable, ok := lint.(Configurable) + if ok { err := config.Configure(configurable.Configure(), l.Name) if err != nil { details := fmt.Sprintf( From 0ef39fe2f2de8f6ef37a09e1a8e1aaee3b2a9494 Mon Sep 17 00:00:00 2001 From: christopher-henderson Date: Sun, 19 Jun 2022 11:51:46 -0700 Subject: [PATCH 15/18] adding tests to confirm usage as library --- README.md | 36 ++++++++++++ v3/lint/registration.go | 4 +- v3/zlint_test.go | 118 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index fe95aabca..86591d30c 100644 --- a/README.md +++ b/README.md @@ -155,6 +155,42 @@ if err != nil { zlintResultSet := zlint.LintCertificateEx(parsed, registry) ``` +To lint a certificate in the presence of a particular configuration file, you must first construct the configuration and then make a call to `SetConfiguration` in the `Registry` interface. + +A `Configuration` may be constructed using any of the following functions: + +* `lint.NewConfig(r io.Reader) (Configuration, error)` +* `lint.NewConfigFromFile(config string) (Configuration, error)` +* `lint.NewConfigFromString(config string) (Configuration, error)` + +The contents of the input to all three constructors must be a valid TOML document. + +```go +import ( + "github.com/zmap/zcrypto/x509" + "github.com/zmap/zlint/v3" +) + +var certDER []byte = ... +parsed, err := x509.ParseCertificate(certDER) +if err != nil { + // If x509.ParseCertificate fails, the certificate is too broken to lint. + // This should be treated as ZLint rejecting the certificate + log.Fatal("unable to parse certificate:", err) +} +configuration, err := lint.NewConfigFromString(` + [some_configurable_lint] + IsWebPki = true + NumIterations = 42 + + [some_configurable_lint.AnySubMapping] + something = "else" + anything = "at all" +`) +lint.GlobalRegistry().SetConfigutration() +zlintResultSet := zlint.LintCertificate(parsed) +``` + See [the `zlint` command][zlint cmd]'s source code for an example. [zlint cmd]: https://github.com/zmap/zlint/blob/master/v3/cmd/zlint/main.go diff --git a/v3/lint/registration.go b/v3/lint/registration.go index b703ebf7b..ffa8220f1 100644 --- a/v3/lint/registration.go +++ b/v3/lint/registration.go @@ -362,10 +362,12 @@ func (r *registryImpl) defaultConfiguration(globals []GlobalConfiguration) ([]by // lints. //nolint:revive func NewRegistry() *registryImpl { - return ®istryImpl{ + registry := ®istryImpl{ lintsByName: make(map[string]*Lint), lintsBySource: make(map[LintSource][]*Lint), } + registry.SetConfiguration(NewEmptyConfig()) + return registry } // globalRegistry is the Registry used by all loaded lints that call diff --git a/v3/zlint_test.go b/v3/zlint_test.go index f791ff894..2779c11c9 100644 --- a/v3/zlint_test.go +++ b/v3/zlint_test.go @@ -1,9 +1,15 @@ package zlint import ( + "fmt" + "reflect" "strings" "testing" + "time" + "github.com/zmap/zlint/v3/util" + + "github.com/zmap/zcrypto/x509" "github.com/zmap/zlint/v3/lint" ) @@ -28,3 +34,115 @@ func TestLintNames(t *testing.T) { } } } + +type configurableTestLint struct { + A string + B int + C map[string]string + wantA string + wantB int + wantC map[string]string +} + +func NewConfigurableTestLint() lint.LintInterface { + return &configurableTestLint{C: make(map[string]string, 0), wantC: make(map[string]string, 0)} +} + +func (l *configurableTestLint) Configure() interface{} { + return l +} + +func (l *configurableTestLint) CheckApplies(c *x509.Certificate) bool { + return true +} + +func (l *configurableTestLint) Execute(c *x509.Certificate) *lint.LintResult { + if l.A != l.wantA { + return &lint.LintResult{Status: lint.Error, Details: fmt.Sprintf("A got %v, want %v", l.A, l.wantA)} + } + if l.B != l.wantB { + return &lint.LintResult{Status: lint.Error, Details: fmt.Sprintf("B got %v, want %v", l.B, l.wantB)} + } + if !reflect.DeepEqual(l.C, l.wantC) { + return &lint.LintResult{Status: lint.Error, Details: fmt.Sprintf("C got %v, want %v", l.C, l.wantC)} + } + return &lint.LintResult{Status: lint.Pass} +} + +func TestWithDefaultConfiguration(t *testing.T) { + lint.RegisterLint(&lint.Lint{ + Name: "library_usage_test_default_config", + Description: "CA Certificates subject field MUST not be empty and MUST have a non-empty distinguished name", + Citation: "RFC 5280: 4.1.2.6", + Source: lint.RFC5280, + EffectiveDate: util.RFC2459Date, + Lint: NewConfigurableTestLint, + }) + registry, err := lint.GlobalRegistry().Filter(lint.FilterOptions{ + IncludeNames: []string{"library_usage_test_default_config"}, + }) + if err != nil { + t.Fatal(err) + } + got := LintCertificateEx(&x509.Certificate{ + NotAfter: time.Now().Add(time.Hour), + NotBefore: time.Now().Add(-time.Hour), + }, registry) + result, ok := got.Results["library_usage_test_default_config"] + if !ok { + t.Fatal("no results found, perhaps the lint never ran?") + } + if result.Status != lint.Pass { + t.Fatalf("expected lint to pass, got %v (%s)", result.Status, result.Details) + } +} + +func TestWithNonDefaultConfiguration(t *testing.T) { + lint.RegisterLint(&lint.Lint{ + Name: "library_usage_test_non_default_config", + Description: "CA Certificates subject field MUST not be empty and MUST have a non-empty distinguished name", + Citation: "RFC 5280: 4.1.2.6", + Source: lint.RFC5280, + EffectiveDate: util.RFC2459Date, + Lint: func() lint.LintInterface { + return &configurableTestLint{ + C: make(map[string]string, 0), + wantA: "the greatest song in the world", + wantB: 42, + wantC: map[string]string{ + "something": "else", + "anything": "at all", + }} + }, + }) + registry, err := lint.GlobalRegistry().Filter(lint.FilterOptions{ + IncludeNames: []string{"library_usage_test_non_default_config"}, + }) + if err != nil { + t.Fatal(err) + } + config, err := lint.NewConfigFromString(` +[library_usage_test_non_default_config] +A = "the greatest song in the world" +B = 42 + +[library_usage_test_non_default_config.C] +something = "else" +anything = "at all" +`) + if err != nil { + t.Fatal(err) + } + registry.SetConfiguration(config) + got := LintCertificateEx(&x509.Certificate{ + NotAfter: time.Now().Add(time.Hour), + NotBefore: time.Now().Add(-time.Hour), + }, registry) + result, ok := got.Results["library_usage_test_non_default_config"] + if !ok { + t.Fatal("no results found, perhaps the lint never ran?") + } + if result.Status != lint.Pass { + t.Fatalf("expected lint to pass, got %v (%s)", result.Status, result.Details) + } +} From 13ef1e35b20ee5efef299ba1591ab746e03ac38f Mon Sep 17 00:00:00 2001 From: christopher-henderson Date: Sun, 19 Jun 2022 12:09:45 -0700 Subject: [PATCH 16/18] addressing some PR comments --- README.md | 10 +++++----- v3/lint/configuration.go | 8 ++++---- v3/lint/registration.go | 6 +++--- v3/test/configuration_test_framework_test.go | 6 +++--- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 86591d30c..bdf7e4a32 100644 --- a/README.md +++ b/README.md @@ -96,12 +96,12 @@ Example ZLint CLI usage: echo "Lint mycert.pem with all of the lints except for ETSI ESI sourced lints" zlint -excludeSources=ETSI_ESI mycert.pem - - echo "Receive a copy of the full (default) configuration for all configurable lints" - zlint -exampleConfig - echo "Lint mycert.pem using a custom configuration for any configurable lints" - zlint -config configFile.toml mycert.pem + echo "Receive a copy of the full (default) configuration for all configurable lints" + zlint -exampleConfig + + echo "Lint mycert.pem using a custom configuration for any configurable lints" + zlint -config configFile.toml mycert.pem See `zlint -h` for all available command line options. diff --git a/v3/lint/configuration.go b/v3/lint/configuration.go index a471552a1..d60f3aec7 100644 --- a/v3/lint/configuration.go +++ b/v3/lint/configuration.go @@ -100,8 +100,8 @@ func NewConfigFromString(config string) (Configuration, error) { // any attempt to resolve a namespace in `deserializeConfigInto` fails and thus results // in all defaults for all lints being maintained. func NewEmptyConfig() Configuration { - ctx, _ := NewConfigFromString("") - return ctx + cfg, _ := NewConfigFromString("") + return cfg } // deserializeConfigInto deserializes the section labeled by the provided `namespace` @@ -129,8 +129,8 @@ func NewEmptyConfig() Configuration { // Then the invocation of this function should be... // // ``` -// lint := &SomeOtherLink{} -// deserializeConfigInto(link, "w_some_other_lint") +// lint := &SomeOtherLint{} +// deserializeConfigInto(lint, "w_some_other_lint") // ``` // // If there is no such namespace found in this configuration then provided the namespace specific data encoded diff --git a/v3/lint/registration.go b/v3/lint/registration.go index ffa8220f1..2d6d96939 100644 --- a/v3/lint/registration.go +++ b/v3/lint/registration.go @@ -301,8 +301,8 @@ func (r *registryImpl) WriteJSON(w io.Writer) { } } -func (r *registryImpl) SetConfiguration(ctx Configuration) { - r.configuration = ctx +func (r *registryImpl) SetConfiguration(cfg Configuration) { + r.configuration = cfg } func (r *registryImpl) GetConfiguration() Configuration { @@ -381,7 +381,7 @@ var globalRegistry = NewRegistry() // registration process. // // IMPORTANT: RegisterLint will panic if given a nil lint, or a lint with a nil -// Lint pointer, or if the lint's Initialize function errors, or if the lint` +// Lint pointer, or if the lint's Initialize function errors, or if the lint // name matches a previously registered lint's name. These conditions all // indicate a bug that should be addressed by a developer. func RegisterLint(l *Lint) { diff --git a/v3/test/configuration_test_framework_test.go b/v3/test/configuration_test_framework_test.go index ac60adb70..688a2f832 100644 --- a/v3/test/configuration_test_framework_test.go +++ b/v3/test/configuration_test_framework_test.go @@ -90,7 +90,7 @@ BeerHall = "liedershousen" ` out := TestLintWithConfig("e_ca_common_name_missing2", inputPath, config) if out.Details != "liedershousen" { - panic("noooooooooooo") + t.Fatalf("unexpected output details, got '%s' want %s", out.Details, "liedershousen") } if out.Status != expected { t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status) @@ -106,7 +106,7 @@ BeerHall = "liedershousenssss" ` out := TestLintWithConfig("e_ca_common_name_missing2", inputPath, config) if out.Details != "liedershousenssss" { - panic("noooooooooooo") + t.Fatalf("unexpected output details, got '%s' want %s", out.Details, "liedershousenssss") } if out.Status != expected { t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status) @@ -156,7 +156,7 @@ BeerHall = "liedershousenssss" t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status) } if out.Details != "liedershousenssss" { - panic("noooooooooooo") + t.Fatalf("unexpected output details, got '%s' want %s", out.Details, "liedershousenssss") } } From bcc07d237bd7dc8bb3b1bc7db7ec222bf494da6f Mon Sep 17 00:00:00 2001 From: christopher-henderson Date: Sun, 19 Jun 2022 12:49:59 -0700 Subject: [PATCH 17/18] fixing code sample in README --- README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index bdf7e4a32..b76d381fb 100644 --- a/README.md +++ b/README.md @@ -187,7 +187,10 @@ configuration, err := lint.NewConfigFromString(` something = "else" anything = "at all" `) -lint.GlobalRegistry().SetConfigutration() +if err != nil { + log.Fatal("unable to parse configuration:", err) +} +lint.GlobalRegistry().SetConfigutration(configuration) zlintResultSet := zlint.LintCertificate(parsed) ``` From 519eae6395d3a0f18a06ce9028f80053dffda7e8 Mon Sep 17 00:00:00 2001 From: christopher-henderson Date: Sun, 17 Jul 2022 15:04:43 -0700 Subject: [PATCH 18/18] addressing nits --- README.md | 2 +- v3/cmd/zlint/main.go | 6 +++--- v3/test/helpers.go | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index b76d381fb..7fbec7440 100644 --- a/README.md +++ b/README.md @@ -160,7 +160,7 @@ To lint a certificate in the presence of a particular configuration file, you mu A `Configuration` may be constructed using any of the following functions: * `lint.NewConfig(r io.Reader) (Configuration, error)` -* `lint.NewConfigFromFile(config string) (Configuration, error)` +* `lint.NewConfigFromFile(path string) (Configuration, error)` * `lint.NewConfigFromString(config string) (Configuration, error)` The contents of the input to all three constructors must be a valid TOML document. diff --git a/v3/cmd/zlint/main.go b/v3/cmd/zlint/main.go index e3c1ae82f..15ce2b287 100644 --- a/v3/cmd/zlint/main.go +++ b/v3/cmd/zlint/main.go @@ -105,7 +105,7 @@ func main() { if err != nil { log.Fatalf("a critical error occurred while generating a configuration file, %s", err) } - fmt.Printf("%s\n", string(b)) + fmt.Println(string(b)) return } @@ -216,11 +216,11 @@ func trimmedList(raw string) []string { // use. //nolint:cyclop func setLints() (lint.Registry, error) { - config, err := lint.NewConfigFromFile(config) + configuration, err := lint.NewConfigFromFile(config) if err != nil { return nil, err } - lint.GlobalRegistry().SetConfiguration(config) + lint.GlobalRegistry().SetConfiguration(configuration) // If there's no filter options set, use the global registry as-is if nameFilter == "" && includeNames == "" && excludeNames == "" && includeSources == "" && excludeSources == "" { return lint.GlobalRegistry(), nil diff --git a/v3/test/helpers.go b/v3/test/helpers.go index bc4672109..1f2428989 100644 --- a/v3/test/helpers.go +++ b/v3/test/helpers.go @@ -42,12 +42,12 @@ func TestLint(lintName string, testCertFilename string) *lint.LintResult { return TestLintWithConfig(lintName, testCertFilename, "") } -func TestLintWithConfig(lintName string, testCertFilename string, context string) *lint.LintResult { - ctx, err := lint.NewConfigFromString(context) +func TestLintWithConfig(lintName string, testCertFilename string, configuration string) *lint.LintResult { + config, err := lint.NewConfigFromString(configuration) if err != nil { panic(err) } - return TestLintCert(lintName, ReadTestCert(testCertFilename), ctx) + return TestLintCert(lintName, ReadTestCert(testCertFilename), config) } // TestLintCert executes a lint with the given name against an already parsed