Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Configurable Lints #648

Merged
merged 30 commits into from
Jul 23, 2022
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
0ba0e8f
Support for configurable lints
christopher-henderson Jul 9, 2021
253cf9e
removing vestigial test interface
christopher-henderson Nov 28, 2021
fa256fb
removing test files and making naming more consistent
christopher-henderson Nov 28, 2021
791d39f
Merge branch 'master' into context
christopher-henderson Dec 11, 2021
363a88b
drastically simplified deserialization logic
christopher-henderson Dec 20, 2021
9e8467c
Merge branch 'context' of github.com:zmap/zlint into context
christopher-henderson Dec 20, 2021
80c7ce5
Merge branch 'master' into context
christopher-henderson Dec 20, 2021
2515536
Merge branch 'context' of github.com:zmap/zlint into context
christopher-henderson Dec 20, 2021
6dd03c6
that's a bad lint
christopher-henderson Dec 20, 2021
bfa8e73
fixing typos, adding more tests, adding more utils for tricky code
christopher-henderson Jan 8, 2022
3f5549c
linter complained about name
christopher-henderson Jan 8, 2022
a193feb
Merge branch 'master' into context
christopher-henderson Jan 9, 2022
a15789b
fixing goland's overzealous comment refactoring
christopher-henderson Jan 9, 2022
fc8ac18
Merge branch 'context' of github.com:zmap/zlint into context
christopher-henderson Jan 9, 2022
b1cf605
made it to 100% code coverage for the core logic
christopher-henderson Jan 22, 2022
913dfca
linter complaints
christopher-henderson Jan 22, 2022
2e82e15
Merge branch 'master' into context
christopher-henderson Jan 22, 2022
965c245
Merge branch 'master' into context
christopher-henderson Mar 13, 2022
10bbe2d
Merge branch 'master' into context
zakird Apr 15, 2022
f9248a4
Update README.md
christopher-henderson Apr 24, 2022
e729b19
Update v3/cmd/zlint/main.go
christopher-henderson Apr 24, 2022
6d0dc70
Update v3/cmd/zlint/main.go
christopher-henderson Apr 24, 2022
7ccf302
Merge branch 'master' into context
christopher-henderson Apr 24, 2022
c477d54
changing out type switch
christopher-henderson Apr 24, 2022
202da61
Merge branch 'master' into context
christopher-henderson Jun 19, 2022
0ef39fe
adding tests to confirm usage as library
christopher-henderson Jun 19, 2022
13ef1e3
addressing some PR comments
christopher-henderson Jun 19, 2022
bcc07d2
fixing code sample in README
christopher-henderson Jun 19, 2022
519eae6
addressing nits
christopher-henderson Jul 17, 2022
b7e79f1
Merge branch 'master' into context
christopher-henderson Jul 17, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 98 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------------

Expand Down Expand Up @@ -167,6 +245,26 @@ 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 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
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.Status != expected {
t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status)
}
}
```

Updating the TLD Map
--------------------
Expand Down
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 -exampleConfig

echo "Lint mycert.pem using a custom configuration for any configurable lints"
zlint -config configFile.toml mycert.pem
christopher-henderson marked this conversation as resolved.
Show resolved Hide resolved

See `zlint -h` for all available command line options.

christopher-henderson marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
19 changes: 19 additions & 0 deletions v3/cmd/zlint/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ var ( // flags
includeSources string
excludeSources string
printVersion bool
config string
Copy link
Member Author

@christopher-henderson christopher-henderson Jan 8, 2022

Choose a reason for hiding this comment

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

The changes in main are to:

  1. Add the new flags that either point to a configuration file are prints an example default one to the terminal.
  2. If printing the default example is selected, then that is done and the problem exists. Any other flag is ignored. This behavior is consistent with printing a list of all available lints as well as printing ZLint's version.
  3. Loading the (optional) configuration file before selecting what lints to run.

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
Expand All @@ -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() {
Expand Down Expand Up @@ -95,6 +99,16 @@ func main() {
return
}

if exampleConfig {
b, err := registry.DefaultConfiguration()
if err != nil {
log.Fatalf("a critical error occurred while generating a configuration file, %s", err)
os.Exit(1)
christopher-henderson marked this conversation as resolved.
Show resolved Hide resolved
}
fmt.Printf("%s\n", string(b))
christopher-henderson marked this conversation as resolved.
Show resolved Hide resolved
return
}

if listLintSources {
sources := registry.Sources()
sort.Sort(sources)
Expand Down Expand Up @@ -200,6 +214,11 @@ func trimmedList(raw string) []string {
// includeNames, excludeNames, includeSources, and excludeSources flag values in
// use.
func setLints() (lint.Registry, error) {
config, err := lint.NewConfigFromFile(config)
christopher-henderson marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}
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
Expand Down
1 change: 1 addition & 0 deletions v3/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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-20220402174210-599ec18ecbac
Expand Down
2 changes: 2 additions & 0 deletions v3/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
29 changes: 27 additions & 2 deletions v3/lint/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package lint
*/

import (
"fmt"
"time"

"github.com/zmap/zcrypto/x509"
Expand All @@ -34,6 +35,11 @@ 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{}
}

// A Lint struct represents a single lint, e.g.
// "e_basic_constraints_not_critical". It contains an implementation of LintInterface.
type Lint struct {
Expand Down Expand Up @@ -87,14 +93,33 @@ 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()
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()
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:
christopher-henderson marked this conversation as resolved.
Show resolved Hide resolved
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}
}
}
if !lint.CheckApplies(cert) {
return &LintResult{Status: NA}
} else if !l.CheckEffective(cert) {
Expand Down
Loading