Skip to content

Commit 5e1c029

Browse files
authored
fix(scan): grade hsts on the final response scheme after redirects (#310)
hsts grading was gated on the scheme of the originally requested url, but the client follows redirects, so an http target that redirects to https skipped the hsts check entirely and dropped a high-severity finding. decide the scheme from the final response request url instead, falling back to the requested url only when no response request is set.
1 parent e2da1bc commit 5e1c029

2 files changed

Lines changed: 67 additions & 1 deletion

File tree

internal/scan/securityheaders.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func SecurityHeaders(url string, timeout time.Duration, logdir string) (Security
7878
// header-only scan: drain on close so the conn is returned to the pool.
7979
defer httpx.DrainClose(resp)
8080

81-
results := gradeSecurityHeaders(resp.Header, strings.HasPrefix(url, "https://"))
81+
results := gradeSecurityHeaders(resp.Header, responseIsHTTPS(resp, url))
8282

8383
for _, r := range results {
8484
line := r.Header + " " + r.Note
@@ -96,6 +96,18 @@ func SecurityHeaders(url string, timeout time.Duration, logdir string) (Security
9696
return results, nil
9797
}
9898

99+
// responseIsHTTPS reports whether the response was actually served over
100+
// https. the client follows redirects, so a request that started as
101+
// http:// can end up served over https:// (or vice versa); the final
102+
// scheme lives on resp.Request.URL, not the originally requested url.
103+
// falls back to the requested url's scheme if that's unavailable.
104+
func responseIsHTTPS(resp *http.Response, requestedURL string) bool {
105+
if resp != nil && resp.Request != nil && resp.Request.URL != nil {
106+
return resp.Request.URL.Scheme == "https"
107+
}
108+
return strings.HasPrefix(requestedURL, "https://")
109+
}
110+
99111
func gradeSecurityHeaders(header http.Header, https bool) SecurityHeaderResults {
100112
var results SecurityHeaderResults
101113

internal/scan/securityheaders_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ package scan
1515
import (
1616
"net/http"
1717
"net/http/httptest"
18+
"net/url"
1819
"testing"
1920
"time"
2021
)
@@ -151,6 +152,59 @@ func TestGradeSecurityHeaders_Disclosure(t *testing.T) {
151152
}
152153
}
153154

155+
func TestResponseIsHTTPS_UsesFinalRequestScheme(t *testing.T) {
156+
httpURL, err := url.Parse("http://example.com")
157+
if err != nil {
158+
t.Fatal(err)
159+
}
160+
httpsURL, err := url.Parse("https://example.com")
161+
if err != nil {
162+
t.Fatal(err)
163+
}
164+
165+
tests := []struct {
166+
name string
167+
resp *http.Response
168+
requestedURL string
169+
want bool
170+
}{
171+
{
172+
// the requested url was http, but the client followed a redirect
173+
// to https; hsts must be graded against the actual scheme.
174+
name: "redirected from http to https",
175+
resp: &http.Response{Request: &http.Request{URL: httpsURL}},
176+
requestedURL: "http://example.com",
177+
want: true,
178+
},
179+
{
180+
name: "redirected from https to http",
181+
resp: &http.Response{Request: &http.Request{URL: httpURL}},
182+
requestedURL: "https://example.com",
183+
want: false,
184+
},
185+
{
186+
name: "no redirect, stays http",
187+
resp: &http.Response{Request: &http.Request{URL: httpURL}},
188+
requestedURL: "http://example.com",
189+
want: false,
190+
},
191+
{
192+
name: "no request on response falls back to requested url",
193+
resp: &http.Response{},
194+
requestedURL: "https://example.com",
195+
want: true,
196+
},
197+
}
198+
199+
for _, tt := range tests {
200+
t.Run(tt.name, func(t *testing.T) {
201+
if got := responseIsHTTPS(tt.resp, tt.requestedURL); got != tt.want {
202+
t.Errorf("responseIsHTTPS() = %v, want %v", got, tt.want)
203+
}
204+
})
205+
}
206+
}
207+
154208
func TestSecurityHeaders_LiveResponse(t *testing.T) {
155209
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
156210
w.Header().Set("X-Frame-Options", "SAMEORIGIN")

0 commit comments

Comments
 (0)