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

RF: hsts #53

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

RF: hsts #53

wants to merge 4 commits into from

Conversation

syywu
Copy link
Contributor

@syywu syywu commented Jun 15, 2024

No description provided.

Copy link

codecov bot commented Jun 15, 2024

Codecov Report

Attention: Patch coverage is 81.39535% with 8 lines in your changes missing coverage. Please review.

Flag Coverage Δ
unittests 61.06% <81.39%> (+1.31%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
server/server.go 84.21% <100.00%> (ø)
checks/checks.go 0.00% <0.00%> (ø)
handlers/hsts.go 54.54% <50.00%> (+12.60%) ⬆️
checks/hsts.go 84.61% <84.61%> (ø)

checks/hsts.go Outdated Show resolved Hide resolved
checks/hsts.go Outdated
}
defer resp.Body.Close()

hstsHeader := resp.Header.Get(StrictTransportSecurity)
Copy link
Collaborator

Choose a reason for hiding this comment

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

headers can be set in one valid string such as

Strict-Transport-Security: max-age=63072000; includeSubDomains; preload

or multiple such as

Strict-Transport-Security: max-age=63072000
Strict-Transport-Security: includeSubDomains; preload

both are valid but the use of Get only returns one in the case of scenario 2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

*http.Response.header is a map so would the second Strict-Transport-Security not replace the first value?

checks/hsts.go Show resolved Hide resolved
checks/hsts.go Outdated Show resolved Hide resolved
return &HSTSResponse{Message: "Site does not serve any HSTS headers."}, nil
}

if !strings.Contains(hstsHeader, "max-age") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although max-age is not optional, I guess we shouldn't assume it'll have max-age in the headers?

}

var maxAgeString string
for _, h := range strings.Split(hstsHeader, " ") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The OG way to extract max-age was through the use of regex and indexing into the slice to get the max age value. Is this a better approach than indexing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants