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.

Sign up for GitHub

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

Lint07 #22

Open
wants to merge 2 commits into
base: lint06
Choose a base branch
from
Open

Lint07 #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c990070
Apple Lint 07
tnarg999 Sep 16, 2020
06661c2
Apple Lint 19 (sorry forgot to make a separate PR)
tnarg999 Sep 16, 2020
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
85 changes: 45 additions & 40 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,35 +2,28 @@

## Adding a New Lint

First write the function body of the lint in the `linter/lintfuncs.go` file, which should be of the form `func(resp *ocsp.Response, leafCert *x509.Certificate, issuerCert *x509.Certificate) (LintStatus, string)`. `LintStatus` is an enum that takes the value `Passed`, `Failed`, or `Error`, which should indicate whether the lint passed, failed, or errored while running. The string returned should provide additional information on the status.
First write the function body of the lint in the `linter/lintfuncs.go` file, which should be of the form `func(resp *ocsp.Response, leafCert *x509.Certificate, issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string)`. `LintStatus` is an enum that takes the value `Passed`, `Failed`, or `Error`, which should indicate whether the lint passed, failed, or errored while running. The string returned should provide additional information on the status.

Example:

```go
// LintProducedAtDate checks that an OCSP Response ProducedAt date is no more than ProducedAtLimit in the past
// Source: Apple Lints 03 & 05
func LintProducedAtDate(resp *ocsp.Response, leafCert *x509.Certificate, issuerCert *x509.Certificate) (LintStatus, string) {
// default assume certificate being checked is a subscriber certificate
certType := "subscriber certificate"
producedAtLimit := ProducedAtLimitSubscriber
if leafCert != nil && leafCert.IsCA {
certType = "subordinate CA certificate"
producedAtLimit = ProducedAtLimitCA
// CheckStatus checks that the status of the OCSP response matches what the user expects it to be
// Source: Apple Lint 07
func CheckStatus(resp *ocsp.Response, leafCert *x509.Certificate, issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string) {
if lintOpts.ExpectedStatus == None {
return Passed, fmt.Sprintf("User did not specify an expected status (fyi OCSP response status was %s)", StatusIntMap[resp.Status])
}

expectedStatus := ocsp.Good
if lintOpts.ExpectedStatus == Revoked {
expectedStatus = ocsp.Revoked
}

limit, err := time.ParseDuration(producedAtLimit)

if err != nil {
return Error, fmt.Sprintf("Could not parse time duration %s", producedAtLimit)
}

if time.Since(resp.ProducedAt) > limit {
return Failed, fmt.Sprintf("OCSP Response producedAt date %s for %s is more than %s in the past",
resp.ProducedAt, certType, DurationToString[producedAtLimit])
if resp.Status != expectedStatus {
return Failed, fmt.Sprintf("Expected status %s, OCSP response status was %s", lintOpts.ExpectedStatus, StatusIntMap[resp.Status])
}

return Passed, fmt.Sprintf("OCSP Response producedAt date %s for %s is within %s of the past",
resp.ProducedAt, certType, DurationToString[producedAtLimit])
return Passed, fmt.Sprintf("OCSP Response status matched expected status of %s", lintOpts.ExpectedStatus)
}
```
Comment on lines 7 to 28
Copy link
Collaborator

Choose a reason for hiding this comment

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

Like I think I mentioned in another PR, every time the code changes you'll have to. remember to come back and update this too. It might be better to just point to a file and a function in the file rather than copy-pasting the whole code.


Expand All @@ -39,29 +32,41 @@ Next please write unit tests for your new linting function in `linter/lintfuncs_
Example:

```go
// TestLintProducedAtDate tests LintProducedAtDate, which checks that an
// OCSP Response ProducedAt date is no more than ProducedAtLimit in the past
// Source: Apple Lint 03
func TestLintProducedAtDate(t *testing.T) {
tools := ocsptools.Tools{}
ocspResp, err := tools.ReadOCSPResp(RespBadDates)
// TestCheckStatus tests CheckStatus, which checks whether or not the OCSP response
// status matches what the user expected
// Source: Apple Lint 07
func TestCheckStatus(t *testing.T) {
mockLintOpts := &LintOpts{
LeafCertType: Subscriber,
LeafCertNonIssued: false,
ExpectedStatus: None,
}

ocspResp, err := ocsptools.Tools{}.ReadOCSPResp(RespBadDates)
if err != nil {
panic(err)
panic(fmt.Sprintf("Could not read OCSP Response file %s: %s", RespBadDates, err))
}

t.Run("Old ProducedAt date", func(t *testing.T) {
status, info := LintProducedAtDate(ocspResp, nil, nil)
if status != Failed {
t.Errorf("Should have had error, instead got: %s", info)
t.Run("No expected status", func(t *testing.T) {
status, info := CheckStatus(ocspResp, nil, nil, mockLintOpts)
if status != Passed {
t.Errorf("Lint should have passed, instead got status %s: %s", status, info)
}
})

ocspResp.ProducedAt = time.Now()
mockLintOpts.ExpectedStatus = Revoked
t.Run("Expected status revoked for good response", func(t *testing.T) {
status, info := CheckStatus(ocspResp, nil, nil, mockLintOpts)
if status != Failed {
t.Errorf("Lint should have failed, instead got status %s: %s", status, info)
}
})

t.Run("Happy path", func(t *testing.T) {
status, info := LintProducedAtDate(ocspResp, nil, nil)
mockLintOpts.ExpectedStatus = Good
t.Run("Expected status good for good response", func(t *testing.T) {
status, info := CheckStatus(ocspResp, nil, nil, mockLintOpts)
if status != Passed {
t.Errorf("Should not have gotten error, instead got error: %s", info)
t.Errorf("Lint should have passed, instead got status %s: %s", status, info)
}
})
}
Expand All @@ -71,10 +76,10 @@ Finally in `linter/linter.go`, add the address of a new `LintStruct` to the glob

Example:
```go
&LintStruct{
"Check response producedAt date",
"Apple Lints 03 & 05",
LintProducedAtDate,
{
"Check response status",
"Apple Lint 07",
CheckStatus,
}
```

4 changes: 2 additions & 2 deletions linter/lintconsts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ const (

type OCSPStatus string
const (
Good OCSPStatus = "Good"
Revoked OCSPStatus = "Revoked"
Good OCSPStatus = "good"
Revoked OCSPStatus = "revoked"
None OCSPStatus = ""
)

Expand Down
25 changes: 15 additions & 10 deletions linter/linter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,20 @@ import (

// LintStruct defines the struct of a lint
type LintStruct struct {
Info string // description of the lint
Source string // source of the lint
Exec func(resp *ocsp.Response, issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string) // the linting function itself
Info string // description of the lint
Source string // source of the lint
// the linting function
Exec func(resp *ocsp.Response, leafCert *x509.Certificate,
issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string)
}

// Lints is the global array of lints that are to be tested (TODO: change to a map)
var Lints = []*LintStruct{
{
"Check response status",
"Apple Lint 07",
CheckStatus,
},
{
"Check response signature",
"Apple Lints 10 & 12",
Expand All @@ -30,12 +37,12 @@ var Lints = []*LintStruct{
},
{
"Check response producedAt date",
"Apple Lints 03 & 05",
"Apple Lints 03, 05, & 19",
LintProducedAtDate,
},
{
"Check response thisUpdate date",
"Apple Lints 03 & 05",
"Apple Lints 03, 05, & 19",
LintThisUpdateDate,
},
{
Expand All @@ -52,7 +59,7 @@ var Lints = []*LintStruct{

// LinterInterface is an interface containing the functions that are exported from this file
type LinterInterface interface {
LintOCSPResp(*ocsp.Response, *x509.Certificate, *LintOpts, bool)
LintOCSPResp(*ocsp.Response, *x509.Certificate, *x509.Certificate, *LintOpts, bool)
}

// Linter is a struct of type LinterInterface
Expand Down Expand Up @@ -83,12 +90,10 @@ func printResults(results []*LintResult, verbose bool) {
}

// LintOCSPResp takes in a parsed OCSP response and prints its status, and then lints it
func (l Linter) LintOCSPResp(resp *ocsp.Response, issuerCert *x509.Certificate, lintOpts *LintOpts, verbose bool) {
fmt.Printf("OCSP Response status: %s \n\n", StatusIntMap[resp.Status])

func (l Linter) LintOCSPResp(resp *ocsp.Response, leafCert *x509.Certificate, issuerCert *x509.Certificate, lintOpts *LintOpts, verbose bool) {
var results []*LintResult
for _, lint := range Lints {
status, info := lint.Exec(resp, issuerCert, lintOpts)
status, info := lint.Exec(resp, leafCert, issuerCert, lintOpts)
results = append(results, &LintResult{
Lint: lint,
Status: status,
Expand Down
44 changes: 36 additions & 8 deletions linter/lintfuncs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,28 @@ var DurationToString = map[string]string{
NextUpdateLimitSubscriber: "10 days",
}

// CheckStatus checks that the status of the OCSP response matches what the user expects it to be
// Source: Apple Lint 07
func CheckStatus(resp *ocsp.Response, leafCert *x509.Certificate, issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string) {
if lintOpts.ExpectedStatus == None {
return Passed, fmt.Sprintf("User did not specify an expected status (fyi OCSP response status was %s)", StatusIntMap[resp.Status])
}

expectedStatus := ocsp.Good
if lintOpts.ExpectedStatus == Revoked {
expectedStatus = ocsp.Revoked
}

if resp.Status != expectedStatus {
return Failed, fmt.Sprintf("Expected status %s, OCSP response status was %s", lintOpts.ExpectedStatus, StatusIntMap[resp.Status])
}

return Passed, fmt.Sprintf("OCSP Response status matched expected status of %s", lintOpts.ExpectedStatus)
}

// CheckSignature checks in the ocsp response is signed with an algorithm that uses SHA1
// Source: Apple Lints 10 & 12
func CheckSignature(resp *ocsp.Response, issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string) {
func CheckSignature(resp *ocsp.Response, leafCert *x509.Certificate, issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string) {
if resp.Signature == nil || len(resp.Signature) == 0 {
return Failed, "OCSP Response is not signed"
}
Expand All @@ -47,7 +66,7 @@ func CheckSignature(resp *ocsp.Response, issuerCert *x509.Certificate, lintOpts
// CheckResponder checks that the OCSP Responder is either the issuing CA or a delegated responder
// issued by the issuing CA either by comparing public key hashes or names
// Source: Apple Lint 13
func CheckResponder(resp *ocsp.Response, issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string) {
func CheckResponder(resp *ocsp.Response, leafCert *x509.Certificate, issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string) {
if issuerCert == nil {
return Unknown, "Issuer certificate not provided, can't check responder"
}
Expand Down Expand Up @@ -95,8 +114,8 @@ func CheckResponder(resp *ocsp.Response, issuerCert *x509.Certificate, lintOpts
}

// LintProducedAtDate checks that an OCSP Response ProducedAt date is no more than ProducedAtLimit in the past
// Source: Apple Lints 03 & 05
func LintProducedAtDate(resp *ocsp.Response, issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string) {
// Source: Apple Lints 03, 05, & 19
func LintProducedAtDate(resp *ocsp.Response, leafCert *x509.Certificate, issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string) {
// default assume certificate being checked is a subscriber certificate
producedAtLimit := ProducedAtLimitSubscriber
if lintOpts.LeafCertType == CA {
Expand All @@ -119,8 +138,17 @@ func LintProducedAtDate(resp *ocsp.Response, issuerCert *x509.Certificate, lintO
}

// LintThisUpdateDate checks that an OCSP Response ThisUpdate date is no more than ThisUpdateLimit in the past
// Source: Apple Lints 03 & 05
func LintThisUpdateDate(resp *ocsp.Response, issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string) {
// Source: Apple Lints 03, 05, & 19
func LintThisUpdateDate(resp *ocsp.Response, leafCert *x509.Certificate, issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string) {
if resp.ThisUpdate.After(time.Now()) {
return Failed, fmt.Sprintf("OCSP response thisUpdate date %s is in the future", resp.ThisUpdate)
}

if leafCert != nil && resp.ThisUpdate.Before(leafCert.NotBefore) {
return Failed, fmt.Sprintf("OCSP response thisUpdate date %s is before certificate issuance date %s",
resp.ThisUpdate, leafCert.NotBefore)
}

// default assume certificate being checked is a subscriber certificate
thisUpdateLimit := ThisUpdateLimitSubscriber
if lintOpts.LeafCertType == CA {
Expand All @@ -145,7 +173,7 @@ func LintThisUpdateDate(resp *ocsp.Response, issuerCert *x509.Certificate, lintO

// LintNextUpdateDate checks that an OCSP Response NextUpdate date is no more than NextUpdateLimitSubscriber in the past
// Source: Apple Lint 04
func LintNextUpdateDate(resp *ocsp.Response, issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string) {
func LintNextUpdateDate(resp *ocsp.Response, leafCert *x509.Certificate, issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string) {
if lintOpts.LeafCertType == CA {
return Passed, "OCSP Response nextUpdate lint not applicable to CA certificates"
}
Expand All @@ -168,7 +196,7 @@ func LintNextUpdateDate(resp *ocsp.Response, issuerCert *x509.Certificate, lintO

// LintStatusForNonIssuedCert checks that an OCSP response for a non-issued certificate does not have status Good
// Source: Apple Lint 06
func LintStatusForNonIssuedCert(resp *ocsp.Response, issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string) {
func LintStatusForNonIssuedCert(resp *ocsp.Response, leafCert *x509.Certificate, issuerCert *x509.Certificate, lintOpts *LintOpts) (LintStatus, string) {
if !lintOpts.LeafCertNonIssued {
return Passed, "OCSP Response is for issued certificate"
}
Expand Down
Loading