Skip to content

Commit 1582956

Browse files
guspowerlovasoa
andauthored
Extend CSP Configuration to handle user-supplied values that can contain {NONCE} (#911)
* Extend CSP Configuration to handle user-supplied values that can contain {NONCE} * fix RequestContext to use CSP value from AppConfig * update documentation to describe usage * generate nonce per request; config is a string (again); added playwright test to verify subsequent requests return a different nonce. * fix js lint `let` -> `const` * format * remove some useless string copies we are still re-parsing the csp template on every request * implement a proper csp template struct * parse the content-security-policy just once * fix merge issue. * fix docs * fix docs * clippy --------- Co-authored-by: lovasoa <contact@ophir.dev>
1 parent adcbaa6 commit 1582956

File tree

7 files changed

+131
-35
lines changed

7 files changed

+131
-35
lines changed

configuration.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ Here are the available configuration options and their default values:
3636
| `https_certificate_cache_dir` | ./sqlpage/https | A writeable directory where to cache the certificates, so that SQLPage can serve https traffic immediately when it restarts. |
3737
| `https_acme_directory_url` | https://acme-v02.api.letsencrypt.org/directory | The URL of the ACME directory to use when requesting a certificate. |
3838
| `environment` | development | The environment in which SQLPage is running. Can be either `development` or `production`. In `production` mode, SQLPage will hide error messages and stack traces from the user, and will cache sql files in memory to avoid reloading them from disk. |
39-
| `content_security_policy` | `script-src 'self' 'nonce-XXX` | The [Content Security Policy](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP) to set in the HTTP headers. If you get CSP errors in the browser console, you can set this to the empty string to disable CSP. |
39+
| `content_security_policy` | `script-src 'self' 'nonce-{NONCE}'` | The [Content Security Policy](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP) to set in the HTTP headers. If you get CSP errors in the browser console, you can set this to the empty string to disable CSP. If you want a custom CSP that contains a nonce, include the `'nonce-{NONCE}'` directive in your configuration string and it will be populated with a random value per request. |
4040
| `system_root_ca_certificates` | false | Whether to use the system root CA certificates to validate SSL certificates when making http requests with `sqlpage.fetch`. If set to false, SQLPage will use its own set of root CA certificates. If the `SSL_CERT_FILE` or `SSL_CERT_DIR` environment variables are set, they will be used instead of the system root CA certificates. |
4141
| `max_recursion_depth` | 10 | Maximum depth of recursion allowed in the `run_sql` function. Maximum value is 255. |
4242
| `markdown_allow_dangerous_html` | false | Whether to allow raw HTML in markdown content. Only enable this if the markdown content is fully trusted (not user generated). |

src/app_config.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::webserver::content_security_policy::ContentSecurityPolicyTemplate;
12
use crate::webserver::routing::RoutingConfig;
23
use anyhow::Context;
34
use clap::Parser;
@@ -264,8 +265,11 @@ pub struct AppConfig {
264265
pub compress_responses: bool,
265266

266267
/// Content-Security-Policy header to send to the client.
267-
/// If not set, a default policy allowing scripts from the same origin is used and from jsdelivr.net
268-
pub content_security_policy: Option<String>,
268+
/// If not set, a default policy allowing
269+
/// - scripts from the same origin,
270+
/// - script elements with the `nonce="{{@csp_nonce}}"` attribute,
271+
#[serde(default)]
272+
pub content_security_policy: ContentSecurityPolicyTemplate,
269273

270274
/// Whether `sqlpage.fetch` should load trusted certificates from the operating system's certificate store
271275
/// By default, it loads Mozilla's root certificates that are embedded in the `SQLPage` binary, or the ones pointed to by the

src/render.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,9 @@ impl HeaderContext {
9292
) -> Self {
9393
let mut response = HttpResponseBuilder::new(StatusCode::OK);
9494
response.content_type("text/html; charset=utf-8");
95-
if app_state.config.content_security_policy.is_none() {
96-
response.insert_header(&request_context.content_security_policy);
97-
}
95+
request_context
96+
.content_security_policy
97+
.apply_to_response(&mut response);
9898
Self {
9999
app_state,
100100
request_context,
+103-20
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,123 @@
1-
use std::fmt::Display;
2-
1+
use actix_web::http::header::{
2+
HeaderName, HeaderValue, TryIntoHeaderPair, CONTENT_SECURITY_POLICY,
3+
};
4+
use actix_web::HttpResponseBuilder;
35
use awc::http::header::InvalidHeaderValue;
46
use rand::random;
7+
use serde::Deserialize;
8+
use std::fmt::{Display, Formatter};
9+
use std::sync::Arc;
10+
11+
pub const DEFAULT_CONTENT_SECURITY_POLICY: &str = "script-src 'self' 'nonce-{NONCE}'";
512

6-
#[derive(Debug, Clone, Copy)]
13+
#[derive(Debug, Clone)]
714
pub struct ContentSecurityPolicy {
815
pub nonce: u64,
16+
template: ContentSecurityPolicyTemplate,
17+
}
18+
19+
/// A template for the Content Security Policy header.
20+
/// The template is a string that contains the nonce placeholder.
21+
/// The nonce placeholder is replaced with the nonce value when the Content Security Policy is applied to a response.
22+
/// This struct is cheap to clone.
23+
#[derive(Debug, Clone, PartialEq, Eq)]
24+
pub struct ContentSecurityPolicyTemplate {
25+
pub before_nonce: Arc<str>,
26+
pub after_nonce: Option<Arc<str>>,
927
}
1028

11-
impl Default for ContentSecurityPolicy {
29+
impl Default for ContentSecurityPolicyTemplate {
1230
fn default() -> Self {
13-
Self { nonce: random() }
31+
Self::from(DEFAULT_CONTENT_SECURITY_POLICY)
1432
}
1533
}
1634

17-
impl Display for ContentSecurityPolicy {
18-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
19-
write!(f, "script-src 'self' 'nonce-{}'", self.nonce)
35+
impl From<&str> for ContentSecurityPolicyTemplate {
36+
fn from(s: &str) -> Self {
37+
if let Some((before, after)) = s.split_once("{NONCE}") {
38+
Self {
39+
before_nonce: Arc::from(before),
40+
after_nonce: Some(Arc::from(after)),
41+
}
42+
} else {
43+
Self {
44+
before_nonce: Arc::from(s),
45+
after_nonce: None,
46+
}
47+
}
48+
}
49+
}
50+
51+
impl<'de> Deserialize<'de> for ContentSecurityPolicyTemplate {
52+
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
53+
where
54+
D: serde::Deserializer<'de>,
55+
{
56+
let s: &str = Deserialize::deserialize(deserializer)?;
57+
Ok(Self::from(s))
58+
}
59+
}
60+
61+
impl ContentSecurityPolicy {
62+
#[must_use]
63+
pub fn new(template: ContentSecurityPolicyTemplate) -> Self {
64+
Self {
65+
nonce: random(),
66+
template,
67+
}
68+
}
69+
70+
pub fn apply_to_response(&self, response: &mut HttpResponseBuilder) {
71+
if self.is_enabled() {
72+
response.insert_header(self);
73+
}
74+
}
75+
76+
fn is_enabled(&self) -> bool {
77+
!self.template.before_nonce.is_empty() || self.template.after_nonce.is_some()
2078
}
2179
}
2280

23-
impl actix_web::http::header::TryIntoHeaderPair for &ContentSecurityPolicy {
81+
impl Display for ContentSecurityPolicy {
82+
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
83+
let before = self.template.before_nonce.as_ref();
84+
if let Some(after) = &self.template.after_nonce {
85+
let nonce = self.nonce;
86+
write!(f, "{before}{nonce}{after}")
87+
} else {
88+
write!(f, "{before}")
89+
}
90+
}
91+
}
92+
impl TryIntoHeaderPair for &ContentSecurityPolicy {
2493
type Error = InvalidHeaderValue;
2594

26-
fn try_into_pair(
27-
self,
28-
) -> Result<
29-
(
30-
actix_web::http::header::HeaderName,
31-
actix_web::http::header::HeaderValue,
32-
),
33-
Self::Error,
34-
> {
95+
fn try_into_pair(self) -> Result<(HeaderName, HeaderValue), Self::Error> {
3596
Ok((
36-
actix_web::http::header::CONTENT_SECURITY_POLICY,
37-
actix_web::http::header::HeaderValue::from_str(&self.to_string())?,
97+
CONTENT_SECURITY_POLICY,
98+
HeaderValue::from_maybe_shared(self.to_string())?,
3899
))
39100
}
40101
}
102+
103+
#[cfg(test)]
104+
mod tests {
105+
use super::*;
106+
107+
#[test]
108+
fn test_content_security_policy_display() {
109+
let template = ContentSecurityPolicyTemplate::from(
110+
"script-src 'self' 'nonce-{NONCE}' 'unsafe-inline'",
111+
);
112+
let csp = ContentSecurityPolicy::new(template.clone());
113+
let csp_str = csp.to_string();
114+
assert!(csp_str.starts_with("script-src 'self' 'nonce-"));
115+
assert!(csp_str.ends_with("' 'unsafe-inline'"));
116+
let second_csp = ContentSecurityPolicy::new(template);
117+
let second_csp_str = second_csp.to_string();
118+
assert_ne!(
119+
csp_str, second_csp_str,
120+
"We should not generate the same nonce twice"
121+
);
122+
}
123+
}

src/webserver/http.rs

+6-8
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,9 @@ async fn render_sql(
177177
actix_web::rt::spawn(async move {
178178
let request_context = RequestContext {
179179
is_embedded: req_param.get_variables.contains_key("_sqlpage_embed"),
180-
content_security_policy: ContentSecurityPolicy::default(),
180+
content_security_policy: ContentSecurityPolicy::new(
181+
app_state.config.content_security_policy.clone(),
182+
),
181183
};
182184
let mut conn = None;
183185
let database_entries_stream =
@@ -470,7 +472,7 @@ pub fn create_app(
470472
.default_service(fn_service(default_prefix_redirect))
471473
.wrap(OidcMiddleware::new(&app_state))
472474
.wrap(Logger::default())
473-
.wrap(default_headers(&app_state))
475+
.wrap(default_headers())
474476
.wrap(middleware::Condition::new(
475477
app_state.config.compress_responses,
476478
middleware::Compress::default(),
@@ -508,13 +510,9 @@ pub fn payload_config(app_state: &web::Data<AppState>) -> PayloadConfig {
508510
PayloadConfig::default().limit(app_state.config.max_uploaded_file_size * 2)
509511
}
510512

511-
fn default_headers(app_state: &web::Data<AppState>) -> middleware::DefaultHeaders {
513+
fn default_headers() -> middleware::DefaultHeaders {
512514
let server_header = format!("{} v{}", env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION"));
513-
let mut headers = middleware::DefaultHeaders::new().add(("Server", server_header));
514-
if let Some(csp) = &app_state.config.content_security_policy {
515-
headers = headers.add(("Content-Security-Policy", csp.as_str()));
516-
}
517-
headers
515+
middleware::DefaultHeaders::new().add(("Server", server_header))
518516
}
519517

520518
pub async fn run_server(config: &AppConfig, state: AppState) -> anyhow::Result<()> {

src/webserver/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
//! - [`static_content`]: Static asset handling (JS, CSS, icons)
3030
//!
3131
32-
mod content_security_policy;
32+
pub mod content_security_policy;
3333
pub mod database;
3434
pub mod error_with_status;
3535
pub mod http;

tests/end-to-end/official-site.spec.ts

+11
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,17 @@ test("no console errors on card page", async ({ page }) => {
158158
await checkNoConsoleErrors(page, "card");
159159
});
160160

161+
test("CSP issues unique nonces per request", async ({ page }) => {
162+
const csp1 = await (await page.goto(BASE)).headerValue(
163+
"content-security-policy",
164+
);
165+
const csp2 = await (await page.reload()).headerValue(
166+
"content-security-policy",
167+
);
168+
169+
expect(csp1, `check if ${csp1} != ${csp2}`).not.toEqual(csp2);
170+
});
171+
161172
test("form component documentation", async ({ page }) => {
162173
await page.goto(`${BASE}/component.sql?component=form`);
163174

0 commit comments

Comments
 (0)