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

✨ Improved Security Policy Check #2195

Merged
merged 30 commits into from
Nov 4, 2022
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
91f5041
:sparkles: Improved Security Policy Check (#2137)
shissam Aug 23, 2022
0c59ded
Repaired Security Policy to correctly use linked content length for e…
shissam Aug 24, 2022
7827ea9
gofmt'ed changes
shissam Aug 24, 2022
5a518e7
Repaired the case in the evaluation which was too sensitive to conten…
shissam Aug 24, 2022
cad900c
added unit test cases for the new content-based Security Policy checks
shissam Aug 24, 2022
9f9a8e6
reverted the direct (mistaken) change to checks.md and updated the ch…
shissam Aug 27, 2022
3bb3e09
:sparkles: Improved Security Policy Check (#2137) (revisted based on …
shissam Aug 27, 2022
98e9ca1
revised the score value based on observation of one *or more* url(s) …
shissam Aug 27, 2022
80b0e17
revised the score value based on observation of one *or more* url(s) …
shissam Aug 27, 2022
811b537
revised the score value based on observation of one *or more* url(s) …
shissam Aug 27, 2022
5998703
Addressed PR comments; added telemetry for policy hits in security po…
shissam Oct 19, 2022
cb71397
Resolved merge conflict with checks.yaml
shissam Oct 20, 2022
7e45c19
updated raw results to emit all the raw information for the new secur…
shissam Oct 22, 2022
c007797
Resolved merge conflicts and lint errors with json_raw_results.go
shissam Oct 23, 2022
ec1fcff
Addressed review comments to reorganize security policy data struct t…
shissam Oct 26, 2022
6ef1f14
Added logic to the security policy to process multiple security polic…
shissam Oct 27, 2022
f2accd4
added comments regarding the capacity to support multiple policy file…
shissam Oct 27, 2022
088bfb2
Addressed review comments to remove the dependency on the path in the…
shissam Oct 28, 2022
d53ce75
restored reporting full security policy path and filename for policie…
shissam Oct 28, 2022
0dd3c1d
Resolved conflicts in checks.yaml for documentation
shissam Oct 29, 2022
aae4808
resolved merge conflicts
shissam Nov 1, 2022
61fed0b
Merge branch 'main' into main
shissam Nov 1, 2022
bd720d7
✨ CLI for scorecard-attestor (#2309)
raghavkaul Nov 1, 2022
60ba3c8
Merge branch 'main' of https://github.com/shissam/scorecard into main
shissam Nov 1, 2022
776d4d6
removed whitespace before stanza for Run attestor e2e
shissam Nov 1, 2022
e0739b4
merge security-policy test branch with upstream:main
shissam Nov 2, 2022
48719cb
Merge branch 'main' of https://github.com/ossf/scorecard into main
shissam Nov 4, 2022
c46a581
resolved code review and doc review comments
shissam Nov 4, 2022
658d6d6
Merge branch 'main' of https://github.com/ossf/scorecard into main
shissam Nov 4, 2022
2668737
repaired the link for the maintainer's guide for supporting the coord…
shissam Nov 4, 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
33 changes: 31 additions & 2 deletions checker/raw_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,40 @@ type VulnerabilitiesData struct {
Vulnerabilities []clients.Vulnerability
}

type SecurityPolicyInformationType string

const (
// forms of security policy hints being evaluated.
SecurityPolicyInformationTypeEmail SecurityPolicyInformationType = "emailAddress"
SecurityPolicyInformationTypeLink SecurityPolicyInformationType = "httpLink"
SecurityPolicyInformationTypeText SecurityPolicyInformationType = "vulnDisclosureText"
)

type SecurityPolicyValueType struct {
Match string // Snippet of match
LineNumber uint // Line number in policy file of match
Offset uint // Offset in the line of the match
}

type SecurityPolicyInformation struct {
InformationType SecurityPolicyInformationType
InformationValue SecurityPolicyValueType
}

type SecurityPolicyFile struct {
// security policy information found in repo or org
shissam marked this conversation as resolved.
Show resolved Hide resolved
Information []SecurityPolicyInformation
// file that contains the security policy information
// only looking for one file
File File
// total size in bytes of the security policy file contents
SecurityContentLength uint
}

// SecurityPolicyData contains the raw results
// for the Security-Policy check.
type SecurityPolicyData struct {
// Files contains a list of files.
Files []File
PolicyFiles []SecurityPolicyFile
}

// BinaryArtifactData contains the raw results
Expand Down
123 changes: 116 additions & 7 deletions checks/evaluation/security_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,106 @@ import (
sce "github.com/ossf/scorecard/v4/errors"
)

func scoreSecurityCriteria(f checker.File,
contentLen uint,
info []checker.SecurityPolicyInformation,
dl checker.DetailLogger,
) int {
var urls, emails, discvuls, linkedContentLen, score int

emails = countSecInfo(info, checker.SecurityPolicyInformationTypeEmail, true)
urls = countSecInfo(info, checker.SecurityPolicyInformationTypeLink, true)
discvuls = countSecInfo(info, checker.SecurityPolicyInformationTypeText, false)

for _, i := range findSecInfo(info, checker.SecurityPolicyInformationTypeEmail, true) {
linkedContentLen += len(i.InformationValue.Match)
}
for _, i := range findSecInfo(info, checker.SecurityPolicyInformationTypeLink, true) {
linkedContentLen += len(i.InformationValue.Match)
}

msg := checker.LogMessage{
Path: f.Path,
Type: f.Type,
Text: "",
}

// #1: more than one unique (email/http) linked content found: score += 6
shissam marked this conversation as resolved.
Show resolved Hide resolved
// rationale: if more than one link, even stronger for the community
if (urls + emails) > 0 {
score += 6
msg.Text = "Found linked content in security policy"
dl.Info(&msg)
} else {
msg.Text = "no email or URL found in security policy"
dl.Warn(&msg)
}

// #2: more bytes than the sum of the length of all the linked content found: score += 3
// rationale: there appears to be information and context around those links
// no credit if there is just a link to a site or an email address (those given above)
// the test here is that each piece of linked content will likely contain a space
// before and after the content (hence the two multiplier)
if contentLen > 1 && (contentLen > uint(linkedContentLen+((urls+emails)*2))) {
score += 3
msg.Text = "Found text in security policy"
dl.Info(&msg)
} else {
msg.Text = "No text (beyond any linked content) found in security policy"
dl.Warn(&msg)
}

// #3: found whole number(s) and or match(es) to "Disclos" and or "Vuln": score += 1
// rationale: works towards the intent of the security policy file
// regarding whom to contact about vuls and disclosures and timing
// e.g., we'll disclose, report a vulnerabily, 30 days, etc.
// looking for at least 2 hits
if discvuls > 1 {
spencerschrock marked this conversation as resolved.
Show resolved Hide resolved
score += 1
msg.Text = "Found disclosure, vulnerability, and/or timelines in security policy"
dl.Info(&msg)
} else {
msg.Text = "One or no descriptive hints of disclosure, vulnerability, and/or timelines in security policy"
dl.Warn(&msg)
}

return score
}

func countSecInfo(secInfo []checker.SecurityPolicyInformation,
infoType checker.SecurityPolicyInformationType,
unique bool,
) int {
keys := make(map[string]bool)
count := 0
for _, entry := range secInfo {
if _, value := keys[entry.InformationValue.Match]; !value && entry.InformationType == infoType {
shissam marked this conversation as resolved.
Show resolved Hide resolved
keys[entry.InformationValue.Match] = true
count += 1
} else if !unique && entry.InformationType == infoType {
count += 1
}
}
return count
}

func findSecInfo(secInfo []checker.SecurityPolicyInformation,
infoType checker.SecurityPolicyInformationType,
unique bool,
) []checker.SecurityPolicyInformation {
keys := make(map[string]bool)
var secList []checker.SecurityPolicyInformation
for _, entry := range secInfo {
if _, value := keys[entry.InformationValue.Match]; !value && entry.InformationType == infoType {
shissam marked this conversation as resolved.
Show resolved Hide resolved
keys[entry.InformationValue.Match] = true
secList = append(secList, entry)
} else if !unique && entry.InformationType == infoType {
secList = append(secList, entry)
}
}
return secList
}

// SecurityPolicy applies the score policy for the Security-Policy check.
func SecurityPolicy(name string, dl checker.DetailLogger, r *checker.SecurityPolicyData) checker.CheckResult {
if r == nil {
Expand All @@ -27,23 +127,32 @@ func SecurityPolicy(name string, dl checker.DetailLogger, r *checker.SecurityPol
}

// Apply the policy evaluation.
if r.Files == nil || len(r.Files) == 0 {
// If the file is null or has zero lengths, directly return as not detected.
if len(r.PolicyFiles) == 0 {
// If the file is unset, directly return as not detected.
return checker.CreateMinScoreResult(name, "security policy file not detected")
}

for _, f := range r.Files {
// TODO: although this a loop, the raw checks will only return one security policy
// when more than one security policy file can be aggregated into a composite
// score, that logic can be comprehended here.
score := 0
for _, spd := range r.PolicyFiles {
score = scoreSecurityCriteria(spd.File,
spd.SecurityContentLength,
spd.Information, dl)

msg := checker.LogMessage{
Path: f.Path,
Type: f.Type,
Offset: f.Offset,
Path: spd.File.Path,
Type: spd.File.Type,
}
if msg.Type == checker.FileTypeURL {
msg.Text = "security policy detected in org repo"
} else {
msg.Text = "security policy detected in current repo"
}

dl.Info(&msg)
}
return checker.CreateMaxScoreResult(name, "security policy file detected")

return checker.CreateResultWithScore(name, "security policy file detected", score)
}
20 changes: 12 additions & 8 deletions checks/evaluation/security_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,32 +59,36 @@ func TestSecurityPolicy(t *testing.T) {
args: args{
name: "test_security_policy_3",
r: &checker.SecurityPolicyData{
Files: []checker.File{
{
PolicyFiles: []checker.SecurityPolicyFile{{
File: checker.File{
Path: "/etc/security/pam_env.conf",
Type: checker.FileTypeURL,
},
SecurityContentLength: 0,
Information: make([]checker.SecurityPolicyInformation, 0),
},
},
}},
},
want: checker.CheckResult{
Score: 10,
Score: 0,
},
},
{
name: "test_security_policy_4",
args: args{
name: "test_security_policy_4",
r: &checker.SecurityPolicyData{
Files: []checker.File{
{
PolicyFiles: []checker.SecurityPolicyFile{{
File: checker.File{
Path: "/etc/security/pam_env.conf",
},
SecurityContentLength: 0,
Information: make([]checker.SecurityPolicyInformation, 0),
},
},
}},
},
want: checker.CheckResult{
Score: 10,
Score: 0,
},
},
}
Expand Down
Loading