Reject discovered IdP endpoints with a non-HTTP scheme

warpgate-sso/src/error.rs

@@ -36,6 +36,10 @@ pub enum SsoError {
     GoogleDirectory(String),
     #[error("the OIDC provider doesn't support RP-initiated logout")]
     LogoutNotSupported,
+    #[error(
+        "the OIDC provider advertised a {endpoint} of `{url}`: only http and https endpoints are accepted"
+    )]
+    UnsupportedEndpointScheme { endpoint: String, url: String },
     #[error(transparent)]
     Other(Box<dyn Error + Send + Sync>),
 }

warpgate-sso/src/metadata.rs

@@ -2,13 +2,20 @@ use std::collections::HashMap;
 use std::sync::{LazyLock, Mutex};
 use std::time::{Duration, Instant};
 
+use openidconnect::url::Url;
 use openidconnect::{DiscoveryError, ProviderMetadataWithLogout, reqwest};
 
 use crate::SsoError;
 use crate::config::SsoInternalProviderConfig;
 
 const METADATA_CACHE_TTL: Duration = Duration::from_secs(300);
 
+/// Schemes an endpoint from a discovery document is allowed to use.
+///
+/// `https` is what the OIDC Discovery spec mandates; `http` is kept for
+/// providers reached over a trusted network (test rigs, in-cluster IdPs).
+const ALLOWED_ENDPOINT_SCHEMES: [&str; 2] = ["https", "http"];
+
 #[allow(clippy::type_complexity)]
 static METADATA_CACHE: LazyLock<Mutex<HashMap<String, (Instant, ProviderMetadataWithLogout)>>> =
     LazyLock::new(|| Mutex::new(HashMap::new()));
@@ -25,6 +32,67 @@ fn store_metadata(issuer: String, metadata: &ProviderMetadataWithLogout) {
     }
 }
 
+fn check_endpoint_scheme(endpoint: &str, url: &Url) -> Result<(), SsoError> {
+    if ALLOWED_ENDPOINT_SCHEMES.contains(&url.scheme()) {
+        return Ok(());
+    }
+    Err(SsoError::UnsupportedEndpointScheme {
+        endpoint: endpoint.to_owned(),
+        url: url.to_string(),
+    })
+}
+
+/// Reject a discovery document that advertises an endpoint we shouldn't be
+/// dereferencing or handing to a browser.
+///
+/// `openidconnect` parses endpoints as generic URLs and puts no constraint on
+/// their scheme, so a hostile or compromised provider can advertise something
+/// like `javascript:...` as its `authorization_endpoint` or
+/// `end_session_endpoint`. Both of those reach the browser as a URL to
+/// navigate to (`GET /sso/providers/:name/start` and `GET /sso/logout` return
+/// them to the frontend, which assigns them to `location.href`), so a
+/// `javascript:` URL there would execute on the gateway's own origin. The
+/// remaining endpoints are only ever fetched server-side, but there is no
+/// legitimate non-HTTP value for any of them either.
+///
+/// Checking here covers every consumer of discovery metadata, present and
+/// future, instead of relying on each call site to remember.
+fn validate_endpoint_schemes(metadata: &ProviderMetadataWithLogout) -> Result<(), SsoError> {
+    check_endpoint_scheme(
+        "authorization_endpoint",
+        metadata.authorization_endpoint().url(),
+    )?;
+    check_endpoint_scheme("jwks_uri", metadata.jwks_uri().url())?;
+
+    let optional = [
+        ("token_endpoint", metadata.token_endpoint().map(|x| x.url())),
+        (
+            "userinfo_endpoint",
+            metadata.userinfo_endpoint().map(|x| x.url()),
+        ),
+        (
+            "registration_endpoint",
+            metadata.registration_endpoint().map(|x| x.url()),
+        ),
+        (
+            "end_session_endpoint",
+            metadata
+                .additional_metadata()
+                .end_session_endpoint
+                .as_ref()
+                .map(|x| x.url()),
+        ),
+    ];
+
+    for (endpoint, url) in optional {
+        if let Some(url) = url {
+            check_endpoint_scheme(endpoint, url)?;
+        }
+    }
+
+    Ok(())
+}
+
 pub async fn discover_metadata(
     config: &SsoInternalProviderConfig,
     http_client: &reqwest::Client,
@@ -45,6 +113,137 @@ pub async fn discover_metadata(
             })
         })?;
 
+    // Validate before caching, so a hostile document is never served from the
+    // cache and never reaches a caller.
+    validate_endpoint_schemes(&metadata)?;
+
     store_metadata(cache_key, &metadata);
     Ok(metadata)
 }
+
+#[cfg(test)]
+mod tests {
+    use serde_json::{Value, json};
+
+    use super::{ProviderMetadataWithLogout, SsoError, validate_endpoint_schemes};
+
+    /// A minimal discovery document, with `extra` merged over the defaults.
+    fn metadata(extra: &Value) -> ProviderMetadataWithLogout {
+        let mut doc = json!({
+            "issuer": "https://idp.example.com",
+            "authorization_endpoint": "https://idp.example.com/authorize",
+            "jwks_uri": "https://idp.example.com/jwks",
+            "response_types_supported": ["code"],
+            "subject_types_supported": ["public"],
+            "id_token_signing_alg_values_supported": ["RS256"],
+        });
+
+        if let (Some(doc), Some(extra)) = (doc.as_object_mut(), extra.as_object()) {
+            for (key, value) in extra {
+                doc.insert(key.clone(), value.clone());
+            }
+        }
+
+        serde_json::from_value(doc).unwrap()
+    }
+
+    /// The endpoint `validate_endpoint_schemes` rejected, if it rejected one.
+    fn rejected_endpoint(extra: &Value) -> Option<String> {
+        match validate_endpoint_schemes(&metadata(extra)) {
+            Err(SsoError::UnsupportedEndpointScheme { endpoint, .. }) => Some(endpoint),
+            _ => None,
+        }
+    }
+
+    #[test]
+    fn plain_https_document_is_accepted() {
+        assert!(
+            validate_endpoint_schemes(&metadata(&json!({
+                "token_endpoint": "https://idp.example.com/token",
+                "userinfo_endpoint": "https://idp.example.com/userinfo",
+                "registration_endpoint": "https://idp.example.com/register",
+                "end_session_endpoint": "https://idp.example.com/logout",
+            })))
+            .is_ok()
+        );
+    }
+
+    #[test]
+    fn http_is_accepted_for_providers_on_a_trusted_network() {
+        assert!(
+            validate_endpoint_schemes(&metadata(&json!({
+                "authorization_endpoint": "http://keycloak.internal:8080/authorize",
+                "end_session_endpoint": "http://keycloak.internal:8080/logout",
+            })))
+            .is_ok()
+        );
+    }
+
+    #[test]
+    fn javascript_authorization_endpoint_is_rejected() {
+        // This one is handed to the browser by `GET /sso/providers/:name/start`.
+        assert_eq!(
+            rejected_endpoint(&json!({
+                "authorization_endpoint": "javascript:alert(document.cookie)",
+            }))
+            .as_deref(),
+            Some("authorization_endpoint")
+        );
+    }
+
+    #[test]
+    fn javascript_end_session_endpoint_is_rejected() {
+        // This one is handed to the browser by `GET /sso/logout`.
+        assert_eq!(
+            rejected_endpoint(&json!({
+                "end_session_endpoint": "javascript:alert(document.cookie)",
+            }))
+            .as_deref(),
+            Some("end_session_endpoint")
+        );
+    }
+
+    #[test]
+    fn data_endpoints_are_rejected() {
+        assert_eq!(
+            rejected_endpoint(&json!({
+                "authorization_endpoint": "data:text/html,<script>alert(1)</script>",
+            }))
+            .as_deref(),
+            Some("authorization_endpoint")
+        );
+        assert_eq!(
+            rejected_endpoint(&json!({
+                "end_session_endpoint": "data:text/html,<script>alert(1)</script>",
+            }))
+            .as_deref(),
+            Some("end_session_endpoint")
+        );
+    }
+
+    #[test]
+    fn server_side_endpoints_are_checked_too() {
+        for endpoint in [
+            "jwks_uri",
+            "token_endpoint",
+            "userinfo_endpoint",
+            "registration_endpoint",
+        ] {
+            assert_eq!(
+                rejected_endpoint(&json!({ endpoint: "file:///etc/passwd" })).as_deref(),
+                Some(endpoint)
+            );
+        }
+    }
+
+    #[test]
+    fn the_offending_url_is_reported() {
+        let err = validate_endpoint_schemes(&metadata(&json!({
+            "end_session_endpoint": "javascript:alert(1)",
+        })))
+        .err()
+        .map(|e| e.to_string())
+        .unwrap_or_default();
+        assert!(err.contains("javascript:alert(1)"), "{err}");
+    }
+}

warpgate-web/src/common/AuthBar.svelte

@@ -8,6 +8,7 @@
         DropdownToggle,
     } from '@sveltestrap/sveltestrap'
 
+    import { navigateToExternalUrl } from 'common/helpers'
     import { api } from 'gateway/lib/api'
     import { reloadServerInfo, serverInfo } from 'gateway/lib/store'
     import Fa from 'svelte-fa'
@@ -20,7 +21,7 @@
 
     async function singleLogout() {
         const response = await api.initiateSsoLogout()
-        location.href = response.url
+        navigateToExternalUrl(response.url)
     }
 </script>
 

warpgate-web/src/common/helpers.ts

@@ -9,6 +9,28 @@ export function routeQueryParams(): URLSearchParams {
     return new URLSearchParams(router.querystring ?? '')
 }
 
+/**
+ * Navigate to a URL that originates outside Warpgate - currently the
+ * `authorization_endpoint` / `end_session_endpoint` an OIDC provider
+ * advertises in its discovery document.
+ *
+ * Assigning e.g. a `javascript:` URL to `location.href` runs it on our own
+ * origin, so only http(s) is allowed through. The backend rejects such
+ * endpoints at discovery time; this is the second line of defence.
+ */
+export function navigateToExternalUrl(url: string): void {
+    let parsed: URL
+    try {
+        parsed = new URL(url, location.href)
+    } catch {
+        throw new Error(`Refusing to navigate to a malformed URL: ${url}`)
+    }
+    if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') {
+        throw new Error(`Refusing to navigate to a non-HTTP URL: ${url}`)
+    }
+    location.href = url
+}
+
 export function downloadBlob(content: string, filename: string): void {
     const blob = new Blob([content], { type: 'text/plain' })
     const url = URL.createObjectURL(blob)

warpgate-web/src/gateway/Login.svelte

@@ -7,7 +7,7 @@
     import { faArrowRight } from '@fortawesome/free-solid-svg-icons'
     import { Alert, Button, FormGroup } from '@sveltestrap/sveltestrap'
     import { stringifyError } from 'common/errors'
-    import { routeQueryParams } from 'common/helpers'
+    import { navigateToExternalUrl, routeQueryParams } from 'common/helpers'
     import Loadable from 'common/Loadable.svelte'
 
     import {
@@ -165,7 +165,7 @@
         busy = true
         try {
             const p = await api.startSso({ name: provider.name, next: nextURL })
-            location.href = p.url
+            navigateToExternalUrl(p.url)
         } catch (err) {
             error = await stringifyError(err)
             busy = false