From a0020de921b418be9250e23ef9b1eb34c0ac8b02 Mon Sep 17 00:00:00 2001 From: Dale Seo <5466341+DaleSeo@users.noreply.github.com> Date: Thu, 23 Jul 2026 11:20:28 -0400 Subject: [PATCH] fix: use issuer for JWT client audience --- ROADMAP.md | 4 +- conformance/Cargo.toml | 3 +- conformance/expected-failures-extensions.yaml | 1 - conformance/src/bin/client.rs | 142 +++++------------- crates/rmcp/CHANGELOG.md | 4 + crates/rmcp/Cargo.toml | 2 +- crates/rmcp/src/transport/auth.rs | 126 +++++++++++++++- 7 files changed, 163 insertions(+), 119 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 04b0c859a..60c35b4b9 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -30,7 +30,7 @@ runs them in separate server and client steps with `conformance/expected-failures-extensions.yaml`. - SEP-2663 Tasks server: 9 expected failures; `tasks-status-notifications` is currently skipped by the upstream harness; tracked in #868 -- Client extensions: `auth/client-credentials-basic` passes; `auth/client-credentials-jwt` and `auth/enterprise-managed-authorization` are expected failures +- Client extensions: `auth/client-credentials-basic` and `auth/client-credentials-jwt` pass; `auth/enterprise-managed-authorization` is an expected failure ### Spec features without conformance scenarios @@ -110,7 +110,7 @@ These extension scenarios are tracked but do not count toward tier advancement: | Scenario | Tag | Status | |---|---|---| -| `auth/client-credentials-jwt` | extension | ❌ Failed — JWT `aud` claim verification error | +| `auth/client-credentials-jwt` | extension | ✅ Passed | | `auth/client-credentials-basic` | extension | ✅ Passed | | `auth/enterprise-managed-authorization` | extension | ❌ Failed — scenario is not implemented by the conformance client | | `tasks-*` | extension | ❌ 9 expected failures · ⏭️ 1 upstream-skipped scenario | diff --git a/conformance/Cargo.toml b/conformance/Cargo.toml index 83b7007cf..fc4be6d90 100644 --- a/conformance/Cargo.toml +++ b/conformance/Cargo.toml @@ -18,6 +18,7 @@ rmcp = { path = "../crates/rmcp", features = [ "client", "elicitation", "auth", + "auth-client-credentials-jwt", "request-state", "transport-streamable-http-server", "transport-streamable-http-client-reqwest", @@ -33,5 +34,3 @@ anyhow = "1" reqwest = { version = "0.13", features = ["json"] } urlencoding = "2" url = "2" -p256 = { version = "0.14", features = ["ecdsa"] } -base64 = "0.22" diff --git a/conformance/expected-failures-extensions.yaml b/conformance/expected-failures-extensions.yaml index 13b94b668..fdf9617df 100644 --- a/conformance/expected-failures-extensions.yaml +++ b/conformance/expected-failures-extensions.yaml @@ -20,5 +20,4 @@ server: [] client: # Informational OAuth extension scenarios. - - auth/client-credentials-jwt - auth/enterprise-managed-authorization diff --git a/conformance/src/bin/client.rs b/conformance/src/bin/client.rs index 8505f2403..9d1348600 100644 --- a/conformance/src/bin/client.rs +++ b/conformance/src/bin/client.rs @@ -4,7 +4,10 @@ use rmcp::{ service::RequestContext, transport::{ AuthClient, AuthorizationManager, StreamableHttpClientTransport, - auth::{AuthorizationCallback, AuthorizationRequest, InMemoryCredentialStore, OAuthState}, + auth::{ + AuthorizationCallback, AuthorizationRequest, ClientCredentialsConfig, + InMemoryCredentialStore, JwtSigningAlgorithm, OAuthState, + }, streamable_http_client::StreamableHttpClientTransportConfig, }, }; @@ -651,49 +654,43 @@ async fn run_client_credentials_jwt( ) -> anyhow::Result<()> { let client_id = ctx .client_id - .as_deref() - .unwrap_or("conformance-test-client"); - let _pem = ctx + .clone() + .unwrap_or_else(|| "conformance-test-client".to_string()); + let signing_key = ctx .private_key_pem - .as_deref() - .ok_or_else(|| anyhow::anyhow!("Missing private_key_pem"))?; - let _alg = ctx + .as_ref() + .ok_or_else(|| anyhow::anyhow!("Missing private_key_pem"))? + .as_bytes() + .to_vec(); + let signing_algorithm = match ctx .signing_algorithm .as_deref() - .ok_or_else(|| anyhow::anyhow!("Missing signing_algorithm"))?; - - // Discover metadata to get token endpoint - let mut manager = AuthorizationManager::new(server_url).await?; - let metadata = manager.discover_metadata().await?; - let token_endpoint = metadata.token_endpoint.clone(); - manager.set_metadata(metadata); - - // Build JWT assertion - // Parse the PEM private key - let key = openssl_free_ec_sign(_pem, client_id, &token_endpoint)?; - - let http = reqwest::Client::new(); - let form_body = format!( - "grant_type=client_credentials&client_assertion_type={}&client_assertion={}", - urlencoding::encode("urn:ietf:params:oauth:client-assertion-type:jwt-bearer"), - urlencoding::encode(&key), - ); - let resp = http - .post(&token_endpoint) - .header("content-type", "application/x-www-form-urlencoded") - .body(form_body) - .send() - .await?; - - let token_resp: serde_json::Value = resp.json().await?; - let access_token = token_resp["access_token"] - .as_str() - .ok_or_else(|| anyhow::anyhow!("No access_token: {}", token_resp))?; + .ok_or_else(|| anyhow::anyhow!("Missing signing_algorithm"))? + { + "RS256" => JwtSigningAlgorithm::RS256, + "RS384" => JwtSigningAlgorithm::RS384, + "RS512" => JwtSigningAlgorithm::RS512, + "ES256" => JwtSigningAlgorithm::ES256, + "ES384" => JwtSigningAlgorithm::ES384, + algorithm => anyhow::bail!("Unsupported signing_algorithm: {algorithm}"), + }; + let config = ClientCredentialsConfig::PrivateKeyJwt { + client_id, + signing_key, + signing_algorithm, + token_endpoint_audience: None, + scopes: vec![], + resource: Some(server_url.to_string()), + }; + let mut oauth_state = OAuthState::new(server_url, None).await?; + oauth_state.authenticate_client_credentials(config).await?; + let manager = oauth_state + .into_authorization_manager() + .ok_or_else(|| anyhow::anyhow!("Client credentials flow did not authorize"))?; let transport = StreamableHttpClientTransport::with_client( - reqwest::Client::default(), - StreamableHttpClientTransportConfig::with_uri(server_url) - .auth_header(access_token.to_string()), + AuthClient::new(reqwest::Client::default(), manager), + StreamableHttpClientTransportConfig::with_uri(server_url), ); let client = BasicClientHandler.serve(transport).await?; @@ -709,73 +706,6 @@ async fn run_client_credentials_jwt( Ok(()) } -/// Minimal ES256 JWT signing without heavy deps. -/// We use ring or pure-Rust approach. For simplicity, use the p256 + base64 crates -/// that are already transitive deps of oauth2. -fn openssl_free_ec_sign(pem: &str, client_id: &str, audience: &str) -> anyhow::Result { - use std::time::{SystemTime, UNIX_EPOCH}; - - // Decode PEM → DER - let pem_body = pem - .lines() - .filter(|l| !l.starts_with("-----")) - .collect::(); - let der = base64_decode(&pem_body)?; - - // Parse PKCS#8 DER to get the raw EC private key bytes - // PKCS#8 for EC P-256: the raw 32-byte key is at the end of the structure - let raw_key = extract_ec_private_key(&der)?; - - let now = SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs(); - let header = base64url_encode(br#"{"alg":"ES256","typ":"JWT"}"#); - let payload_json = serde_json::json!({ - "iss": client_id, - "sub": client_id, - "aud": audience, - "iat": now, - "exp": now + 300, - "jti": format!("jti-{}", now), - }); - let payload = base64url_encode(payload_json.to_string().as_bytes()); - let signing_input = format!("{}.{}", header, payload); - - // Sign with p256 - let secret_key = p256::ecdsa::SigningKey::from_slice(raw_key.as_slice()) - .map_err(|e| anyhow::anyhow!("Invalid EC key: {}", e))?; - use p256::ecdsa::signature::Signer; - let sig: p256::ecdsa::Signature = secret_key.sign(signing_input.as_bytes()); - let sig_bytes = sig.to_bytes(); - let sig_b64 = base64url_encode(&sig_bytes); - - Ok(format!("{}.{}", signing_input, sig_b64)) -} - -fn base64url_encode(data: &[u8]) -> String { - use base64::Engine; - base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(data) -} - -fn base64_decode(s: &str) -> anyhow::Result> { - use base64::Engine; - Ok(base64::engine::general_purpose::STANDARD.decode(s.trim())?) -} - -/// Extract the raw 32-byte EC private key from a PKCS#8 DER blob. -fn extract_ec_private_key(der: &[u8]) -> anyhow::Result> { - // PKCS#8 wraps an ECPrivateKey. We look for the octet string containing - // the 32-byte private key. A simple heuristic: find 0x04 0x20 (OCTET STRING, len 32) - // followed by exactly 32 bytes that form the key. - // More robust: parse ASN.1. But for conformance testing this suffices. - for i in 0..der.len().saturating_sub(33) { - if der[i] == 0x04 && der[i + 1] == 0x20 && i + 34 <= der.len() { - return Ok(der[i + 2..i + 34].to_vec()); - } - } - Err(anyhow::anyhow!( - "Could not extract 32-byte EC private key from PKCS#8 DER" - )) -} - /// Cross-app access flow (SEP-1046 extension). async fn run_cross_app_access_client( server_url: &str, diff --git a/crates/rmcp/CHANGELOG.md b/crates/rmcp/CHANGELOG.md index dbfb14623..34fe75857 100644 --- a/crates/rmcp/CHANGELOG.md +++ b/crates/rmcp/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING**: rename `StreamableHttpServerConfig::stateful_mode` to `legacy_session_mode` (and the builder `with_stateful_mode` to `with_legacy_session_mode`) to clarify that the option only affects legacy protocol versions (`< 2026-07-28`); per SEP-2567 the `2026-07-28` draft version is always served statelessly ([#999](https://github.com/modelcontextprotocol/rust-sdk/pull/999)) +### Fixed + +- use the authorization server issuer as the default `private_key_jwt` audience + ## [2.2.0](https://github.com/modelcontextprotocol/rust-sdk/compare/rmcp-v2.1.0...rmcp-v2.2.0) - 2026-07-08 ### Added diff --git a/crates/rmcp/Cargo.toml b/crates/rmcp/Cargo.toml index 737682785..0724c01f2 100644 --- a/crates/rmcp/Cargo.toml +++ b/crates/rmcp/Cargo.toml @@ -57,7 +57,7 @@ pastey = { version = "0.2.0", optional = true } # oauth2 support oauth2 = { version = "5.0", optional = true, default-features = false } # JWT signing for client credentials (private_key_jwt) -jsonwebtoken = { version = "10", optional = true } +jsonwebtoken = { version = "10", optional = true, features = ["aws_lc_rs"] } # for auto generate schema schemars = { version = "1.0", optional = true, features = ["chrono04"] } diff --git a/crates/rmcp/src/transport/auth.rs b/crates/rmcp/src/transport/auth.rs index 881b54d45..74bbbf5de 100644 --- a/crates/rmcp/src/transport/auth.rs +++ b/crates/rmcp/src/transport/auth.rs @@ -862,13 +862,35 @@ pub enum ClientCredentialsConfig { client_id: String, signing_key: Vec, signing_algorithm: JwtSigningAlgorithm, - /// Override the `aud` claim in the JWT assertion; defaults to token_endpoint + /// Overrides the authorization server issuer used for the JWT `aud` claim. token_endpoint_audience: Option, scopes: Vec, resource: Option, }, } +#[cfg(feature = "auth-client-credentials-jwt")] +fn client_authentication_audience<'a>( + metadata: &'a AuthorizationMetadata, + configured_audience: Option<&'a str>, +) -> Result<&'a str, AuthError> { + configured_audience + .or(metadata.issuer.as_deref()) + .ok_or_else(|| { + AuthError::ClientCredentialsError( + "Authorization server metadata is missing the issuer required for private_key_jwt" + .to_string(), + ) + }) +} + +#[cfg(feature = "auth-client-credentials-jwt")] +fn client_authentication_header(algorithm: JwtSigningAlgorithm) -> jsonwebtoken::Header { + let mut header = jsonwebtoken::Header::new(algorithm.to_jsonwebtoken_algorithm()); + header.typ = Some("client-authentication+jwt".to_string()); + header +} + impl ClientCredentialsConfig { fn client_id(&self) -> &str { match self { @@ -989,6 +1011,18 @@ fn is_https_url(value: &str) -> bool { .unwrap_or(false) } +#[cfg(feature = "auth-client-credentials-jwt")] +fn is_allowed_client_credentials_endpoint(resource: &Url, token_endpoint: &Url) -> bool { + token_endpoint.scheme() == "https" + || (token_endpoint.scheme() == "http" + && resource + .host_str() + .is_some_and(AuthorizationManager::is_loopback_metadata_host) + && token_endpoint + .host_str() + .is_some_and(AuthorizationManager::is_loopback_metadata_host)) +} + impl AuthorizationManager { fn is_http_url(url: &Url) -> bool { matches!(url.scheme(), "http" | "https") && url.host_str().is_some() @@ -2872,22 +2906,20 @@ impl AuthorizationManager { .as_ref() .ok_or(AuthError::NoAuthorizationSupport)?; - // Validate that the token endpoint uses HTTPS before transmitting sensitive credentials. let token_endpoint_url = url::Url::parse(&metadata.token_endpoint).map_err(|e| { AuthError::ClientCredentialsError(format!( "Invalid token endpoint URL in authorization metadata: {e}" )) })?; - if token_endpoint_url.scheme() != "https" { + if !is_allowed_client_credentials_endpoint(&self.base_url, &token_endpoint_url) { return Err(AuthError::ClientCredentialsError( "Insecure token endpoint URL: HTTPS is required for client credentials flow" .to_string(), )); } - let audience = token_endpoint_audience - .as_deref() - .unwrap_or(&metadata.token_endpoint); + let audience = + client_authentication_audience(metadata, token_endpoint_audience.as_deref())?; let assertion = Self::build_jwt_assertion(client_id, audience, signing_key, *signing_algorithm)?; @@ -2993,7 +3025,7 @@ impl AuthorizationManager { "jti": jti, }); - let header = jsonwebtoken::Header::new(algorithm.to_jsonwebtoken_algorithm()); + let header = client_authentication_header(algorithm); let encoding_key = jsonwebtoken::EncodingKey::from_rsa_pem(signing_key).or_else(|_| { jsonwebtoken::EncodingKey::from_ec_pem(signing_key).map_err(|e| { AuthError::JwtSigningError(format!("Failed to parse signing key: {}", e)) @@ -6476,6 +6508,86 @@ mod tests { // -- client credentials (SEP-1046) -- + #[cfg(feature = "auth-client-credentials-jwt")] + #[test] + fn client_authentication_audience_defaults_to_metadata_issuer() { + let metadata = AuthorizationMetadata { + authorization_endpoint: "https://auth.example.com/authorize".to_string(), + token_endpoint: "https://auth.example.com/token".to_string(), + issuer: Some("https://auth.example.com".to_string()), + ..Default::default() + }; + + let audience = super::client_authentication_audience(&metadata, None).unwrap(); + + assert_eq!(audience, "https://auth.example.com"); + } + + #[cfg(feature = "auth-client-credentials-jwt")] + #[test] + fn client_authentication_audience_honors_explicit_override() { + let metadata = AuthorizationMetadata { + authorization_endpoint: "https://auth.example.com/authorize".to_string(), + token_endpoint: "https://auth.example.com/token".to_string(), + issuer: Some("https://auth.example.com".to_string()), + ..Default::default() + }; + + let audience = super::client_authentication_audience( + &metadata, + Some("https://legacy.example.com/token"), + ) + .unwrap(); + + assert_eq!(audience, "https://legacy.example.com/token"); + } + + #[cfg(feature = "auth-client-credentials-jwt")] + #[test] + fn client_authentication_audience_rejects_missing_metadata_issuer() { + let metadata = AuthorizationMetadata { + authorization_endpoint: "https://auth.example.com/authorize".to_string(), + token_endpoint: "https://auth.example.com/token".to_string(), + ..Default::default() + }; + + let error = super::client_authentication_audience(&metadata, None).unwrap_err(); + + assert!(matches!(error, AuthError::ClientCredentialsError(_))); + } + + #[cfg(feature = "auth-client-credentials-jwt")] + #[test] + fn client_authentication_header_sets_explicit_type() { + let header = super::client_authentication_header(super::JwtSigningAlgorithm::ES256); + + assert_eq!(header.typ.as_deref(), Some("client-authentication+jwt")); + } + + #[cfg(feature = "auth-client-credentials-jwt")] + #[test] + fn client_credentials_endpoint_allows_http_between_loopback_hosts() { + let resource = Url::parse("http://localhost:8000/mcp").unwrap(); + let token_endpoint = Url::parse("http://127.0.0.1:9000/token").unwrap(); + + assert!(super::is_allowed_client_credentials_endpoint( + &resource, + &token_endpoint + )); + } + + #[cfg(feature = "auth-client-credentials-jwt")] + #[test] + fn client_credentials_endpoint_rejects_http_to_non_loopback_host() { + let resource = Url::parse("http://localhost:8000/mcp").unwrap(); + let token_endpoint = Url::parse("http://auth.example.com/token").unwrap(); + + assert!(!super::is_allowed_client_credentials_endpoint( + &resource, + &token_endpoint + )); + } + #[tokio::test] async fn configure_client_credentials_uses_request_body_auth_for_client_secret() { let mut mgr = manager_with_metadata(None).await;