diff --git a/DESCRIPTION b/DESCRIPTION index 35579c1..1509b07 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Type: Package Package: serverHeaders Title: Server Header Summary -Version: 0.1.16 +Version: 0.1.17 Authors@R: person("Jumping", "Rivers", , "info@jumpingrivers.com", role = c("aut", "cre")) Description: Checks and summarises the server headers for a given URL. @@ -16,7 +16,8 @@ Imports: purrr, rlang, rvest, - stringr + stringr, + xml2 Suggests: testthat (>= 3.0.0) Config/testthat/edition: 3 diff --git a/NEWS.md b/NEWS.md index b1c8aa2..11730ce 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,9 @@ +# serverHeaders 0.1.17 + +## Bug fixes and minor improvements + +* The Subresource Integrity check now ignores inline and same-origin scripts, and needs `integrity` and `crossorigin` on every cross-origin script. One hashed script no longer passes the whole page. + # serverHeaders 0.1.16 ## Bug fixes and minor improvements diff --git a/R/check_server_headers.R b/R/check_server_headers.R index 8f0fc99..dbf0536 100644 --- a/R/check_server_headers.R +++ b/R/check_server_headers.R @@ -59,7 +59,7 @@ is_redirected = function(res) { any(grepl("^3", status_codes)) || any(http_versions == "HTTP/2") } -# Retrieves HTML attributes of any script tags to check +# Retrieves HTML attributes of cross-origin script tags to check # subresource integrity get_js_resources = function(res) { # JSON, images etc have no script tags to parse @@ -68,7 +68,35 @@ get_js_resources = function(res) { } # Avoid encoding message. Handled above content = suppressMessages(httr::content(res)) - content |> + scripts = content |> rvest::html_elements("script") |> rvest::html_attrs() + get_cross_origin_scripts(scripts, res$url) +} + +# Inline and same-origin scripts can't be swapped by a third party, +# so only cross-origin scripts need subresource integrity +get_cross_origin_scripts = function(scripts, page_url) { + page_origin = get_origin(page_url) + is_cross_origin = vapply( + scripts, + function(attrs) { + src = attrs["src"] + if (is.na(src) || !nzchar(src)) { + return(FALSE) + } + get_origin(xml2::url_absolute(src, page_url)) != page_origin + }, + logical(1) + ) + scripts[is_cross_origin] +} + +get_origin = function(url) { + parts = httr::parse_url(url) + port = parts$port + if (is.null(port)) { + port = unname(c(http = "80", https = "443")[parts$scheme]) + } + paste0(parts$scheme, "://", tolower(parts$hostname), ":", port) } diff --git a/R/server_header_summaries.R b/R/server_header_summaries.R index 08dbaf0..33fc07c 100644 --- a/R/server_header_summaries.R +++ b/R/server_header_summaries.R @@ -270,25 +270,33 @@ header_summary.scheme = function(value, ...) { `header_summary.subresource-integrity` = function(value, ...) { header = class(value) + # value holds the attributes of each cross-origin script if (length(value) == 0) { status = "OK" - message = "Subresource Integrity is not needed since site contains no script tags" + message = "Subresource Integrity is not needed since the page loads no cross-origin scripts" } else { - integrity = purrr::map(value, "integrity") |> - purrr::list_c() + # crossorigin = "" means anonymous, so any value will do + is_protected = vapply( + value, + function(attrs) { + integrity = attrs["integrity"] + !is.na(integrity) && nzchar(integrity) && !is.na(attrs["crossorigin"]) + }, + logical(1) + ) - crossorigin = purrr::map(value, function(x) { - crossorigin_attr = x["crossorigin"] - !is.na(crossorigin_attr) && crossorigin_attr == "anonymous" - }) |> - purrr::list_c() - - if (all(crossorigin) && all(!is.null(integrity))) { + if (all(is_protected)) { status = "OK" message = "Subresource Integrity is implemented" } else { status = "WARN" - message = "Subresource Integrity not implemented" + message = paste( + "Subresource Integrity missing on", + sum(!is_protected), + "of", + length(is_protected), + "cross-origin scripts" + ) } } diff --git a/tests/testthat/test-check_server_headers.R b/tests/testthat/test-check_server_headers.R index 1488f47..7d8a855 100644 --- a/tests/testthat/test-check_server_headers.R +++ b/tests/testthat/test-check_server_headers.R @@ -30,7 +30,29 @@ test_that("Script tags are only looked for in HTML", { html = fake_response( "text/html; charset=utf-8", - '
' + '' ) - expect_equal(get_js_resources(html)[[1]][["src"]], "a.js") + expect_equal(get_js_resources(html)[[1]][["src"]], "https://cdn.example.net/a.js") +}) + +test_that("Only cross-origin scripts need subresource integrity", { + scripts = list( + character(0), + c(type = "application/ld+json"), + c(src = "/js/local.js"), + c(src = "https://WWW.example.com:443/js/same.js"), + c(src = "//cdn.example.net/lib.js", integrity = "sha384-x", crossorigin = ""), + c(src = "https://example.com/apex.js"), + c(src = "http://www.example.com/insecure.js") + ) + cross = get_cross_origin_scripts(scripts, "https://www.example.com/page") + expect_equal( + vapply(cross, \(x) x[["src"]], character(1)), + c( + "//cdn.example.net/lib.js", + "https://example.com/apex.js", + "http://www.example.com/insecure.js" + ) + ) + expect_equal(get_cross_origin_scripts(list(), "https://www.example.com"), list()) }) diff --git a/tests/testthat/test-header_summary.R b/tests/testthat/test-header_summary.R index 2e0313b..3bf1102 100644 --- a/tests/testthat/test-header_summary.R +++ b/tests/testthat/test-header_summary.R @@ -97,17 +97,25 @@ test_that("Redirection is reported", { expect_equal(summarise_header(FALSE, "redirection")$status, "WARN") }) -test_that("Subresource Integrity is checked on script tags", { +test_that("Every cross-origin script needs integrity and crossorigin", { sri = function(value) summarise_header(value, "subresource-integrity") - expect_equal(sri(list())$status, "OK") + expect_match(sri(list())$message, "no cross-origin scripts") hashed = c( src = "https://cdn.example.com/a.js", integrity = "sha384-x", crossorigin = "anonymous" ) - expect_equal(sri(list(hashed))$status, "OK") + empty_crossorigin = c( + src = "https://cdn.example.com/b.js", + integrity = "sha384-y", + crossorigin = "" + ) + expect_equal(sri(list(hashed, empty_crossorigin))$status, "OK") - no_crossorigin = c(src = "https://cdn.example.com/b.js", integrity = "sha384-y") - expect_equal(sri(list(hashed, no_crossorigin))$status, "WARN") + no_integrity = c(src = "https://cdn.example.com/c.js", crossorigin = "anonymous") + no_crossorigin = c(src = "https://cdn.example.com/d.js", integrity = "sha384-z") + out = sri(list(hashed, no_integrity, no_crossorigin)) + expect_equal(out$status, "WARN") + expect_equal(out$message, "Subresource Integrity missing on 2 of 3 cross-origin scripts") })