Skip to content

Commit f310fac

Browse files
sarroutbiclaude
andauthored
Implement minimal RFC compliance for Location header and URI parsing (#1125)
* Implement minimal RFC compliance for Location header and URI parsing This change implements strict compliance with only the specified RFC sections: - RFC 9110 Section 10.2.2: 201 Created responses must include Location header - RFC 3986 Sections 4.1-4.2: URI reference syntax and relative reference handling Co-Authored-By: Claude <[email protected]> Signed-off-by: Sergio Arroutbi <[email protected]>
1 parent 2091359 commit f310fac

File tree

9 files changed

+1620
-49
lines changed

9 files changed

+1620
-49
lines changed

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,5 +57,6 @@ tempfile = "3.4.0"
5757
thiserror = "2.0"
5858
tokio = {version = "1", features = ["rt", "sync", "macros"]}
5959
tss-esapi = {version = "7.6.0", features = ["generate-bindings"]}
60+
url = "2.5.4"
6061
uuid = {version = "1.3", features = ["v4"]}
6162
zip = {version = "0.6", default-features = false, features= ["deflate"]}

keylime-push-model-agent/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ serde_derive.workspace = true
2525
serde_json.workspace = true
2626
static_assertions.workspace = true
2727
tokio.workspace = true
28+
url.workspace = true
2829

2930
[dev-dependencies]
3031
actix-rt.workspace = true

keylime-push-model-agent/src/attestation.rs

Lines changed: 173 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1+
use crate::header_validation::HeaderValidator;
12
use crate::{context_info_handler, struct_filler, url_selector};
23
use anyhow::Result;
34
use keylime::resilient_client::ResilientClient;
4-
use log::{debug, info};
5+
use log::{debug, info, warn};
56
use reqwest::header::HeaderMap;
6-
use reqwest::header::LOCATION;
77
use reqwest::StatusCode;
88
use std::time::Duration;
99

@@ -92,16 +92,14 @@ impl AttestationClient {
9292
let req = filler.get_attestation_request();
9393
debug!("Request body: {:?}", serde_json::to_string(&req));
9494

95-
let response = self
96-
.client
97-
.get_json_request_from_struct(
98-
reqwest::Method::POST,
99-
config.url,
100-
&req,
101-
Some("application/vnd.api+json".to_string()),
102-
)?
103-
.send()
104-
.await?;
95+
let request_builder = self.client.get_json_request_from_struct(
96+
reqwest::Method::POST,
97+
config.url,
98+
&req,
99+
Some("application/vnd.api+json".to_string()),
100+
)?;
101+
102+
let response = request_builder.send().await?;
105103

106104
let sc = response.status();
107105
let headers = response.headers().clone();
@@ -128,23 +126,33 @@ impl AttestationClient {
128126
) -> Result<ResponseInformation> {
129127
debug!("PATCH Request body: {json_body}");
130128

131-
let response = self
132-
.client
133-
.get_json_request(
134-
reqwest::Method::PATCH,
135-
config.url,
136-
&json_body,
137-
Some("application/vnd.api+json".to_string()),
138-
)?
139-
.send()
140-
.await?;
129+
let request_builder = self.client.get_json_request(
130+
reqwest::Method::PATCH,
131+
config.url,
132+
&json_body,
133+
Some("application/vnd.api+json".to_string()),
134+
)?;
135+
136+
let response = request_builder.send().await?;
141137

142138
let sc = response.status();
143139
let headers = response.headers().clone();
144140
let response_body = response.text().await?;
145141

146142
info!("PATCH Response code:{sc}");
147143
info!("PATCH Response headers: {headers:?}");
144+
145+
// Only validate Location header for 201 Created responses per RFC 9110 Section 10.2.2
146+
if sc.as_u16() == 201 {
147+
if let Err(e) = HeaderValidator::validate_201_created_response(
148+
&headers,
149+
Some(config.url),
150+
) {
151+
warn!("201 Created response validation failed: {}", e);
152+
// Don't fail the request, just log the warning for now
153+
}
154+
}
155+
148156
if !response_body.is_empty() {
149157
info!("PATCH Response body: {response_body}");
150158
}
@@ -162,13 +170,20 @@ impl AttestationClient {
162170
config: &NegotiationConfig<'_>,
163171
) -> Result<ResponseInformation> {
164172
info!("--- Phase 2: Preparing and Sending Evidence ---");
165-
let location_header = neg_response
166-
.headers
167-
.get(LOCATION)
168-
.ok_or_else(|| {
169-
anyhow::anyhow!("Verifier response missing 'Location' header")
170-
})?
171-
.to_str()?;
173+
174+
// Use RFC-compliant Location header validation
175+
let location_header = match HeaderValidator::validate_location_header(
176+
&neg_response.headers,
177+
Some(config.verifier_url),
178+
) {
179+
Ok(location) => location,
180+
Err(e) => {
181+
return Err(anyhow::anyhow!(
182+
"Location header validation failed: {}",
183+
e
184+
));
185+
}
186+
};
172187

173188
let patch_url = url_selector::get_evidence_submission_request_url(
174189
&url_selector::UrlArgs {
@@ -362,6 +377,130 @@ mod tests {
362377
assert!(response_info.body.contains("evidence_requested"));
363378
}
364379

380+
#[actix_rt::test]
381+
async fn test_rfc_compliance_with_mockoon() {
382+
if std::env::var("MOCKOON").is_err() {
383+
return;
384+
}
385+
386+
let config = create_test_config(
387+
"http://localhost:3000/v3.0/agents/d432fbb3-d2f1-4a97-9ef7-75bd81c00000/attestations",
388+
"", "", "",
389+
);
390+
391+
let client = AttestationClient::new(&config).unwrap();
392+
let result = client.send_negotiation(&config).await;
393+
394+
assert!(
395+
result.is_ok(),
396+
"Request to mockoon failed: {:?}",
397+
result.err()
398+
);
399+
400+
let response_info = result.unwrap();
401+
402+
// Test RFC 9110 Section 10.2.2 compliance - 201 Created must have Location header
403+
assert_eq!(
404+
response_info.status_code,
405+
StatusCode::CREATED,
406+
"Expected 201 Created from Mockoon, but got {}",
407+
response_info.status_code
408+
);
409+
410+
// Validate Location header for 201 Created responses per RFC 9110 Section 10.2.2
411+
let validation_result = if response_info.status_code.as_u16() == 201 {
412+
HeaderValidator::validate_201_created_response(
413+
&response_info.headers,
414+
Some("http://localhost:3000"),
415+
)
416+
.map(|_| ())
417+
} else {
418+
Ok(())
419+
};
420+
421+
assert!(
422+
validation_result.is_ok(),
423+
"RFC compliance validation failed: {:?}",
424+
validation_result.err()
425+
);
426+
427+
// Specifically test Location header validation according to RFC 3986
428+
let location_validation = HeaderValidator::validate_location_header(
429+
&response_info.headers,
430+
Some("http://localhost:3000"),
431+
);
432+
433+
assert!(
434+
location_validation.is_ok(),
435+
"Location header validation failed: {:?}",
436+
location_validation.err()
437+
);
438+
439+
// Verify the Location header contains a valid URI
440+
let location = location_validation.unwrap();
441+
assert!(
442+
location.starts_with("http://")
443+
|| location.starts_with("https://")
444+
|| location.starts_with("/"),
445+
"Location header should be a valid URI reference: {}",
446+
location
447+
);
448+
449+
// Test evidence submission with RFC-compliant Location header
450+
let has_location = response_info
451+
.headers
452+
.contains_key(reqwest::header::LOCATION);
453+
let response_body = response_info.body.clone(); // Clone the body for later use
454+
455+
if has_location {
456+
let evidence_result = client
457+
.handle_evidence_submission(response_info, &config)
458+
.await;
459+
460+
// The evidence submission may fail due to mock data, but header validation should succeed
461+
// We're mainly testing that the RFC compliance validation doesn't prevent the request
462+
match evidence_result {
463+
Ok(evidence_response) => {
464+
info!(
465+
"Evidence submission succeeded with status: {}",
466+
evidence_response.status_code
467+
);
468+
469+
// Validate Location header for 201 Created responses per RFC 9110 Section 10.2.2
470+
let evidence_validation =
471+
if evidence_response.status_code.as_u16() == 201 {
472+
HeaderValidator::validate_201_created_response(
473+
&evidence_response.headers,
474+
Some("http://localhost:3000"),
475+
)
476+
.map(|_| ())
477+
} else {
478+
Ok(())
479+
};
480+
481+
if evidence_validation.is_err() {
482+
warn!("Evidence response header validation failed: {:?}", evidence_validation.err());
483+
// Don't fail the test, just log as this might be due to mock server limitations
484+
}
485+
}
486+
Err(e) => {
487+
// Evidence submission failure is acceptable for this test
488+
// We're primarily testing RFC compliance validation
489+
info!(
490+
"Evidence submission failed (expected with mock): {}",
491+
e
492+
);
493+
}
494+
}
495+
496+
// Use the cloned body for the final assertion
497+
assert!(response_body.contains("evidence_requested"));
498+
} else {
499+
// If no Location header, just check the body directly
500+
assert!(response_info.body.contains("evidence_requested"));
501+
}
502+
}
503+
365504
#[actix_rt::test]
366505
async fn test_handle_evidence_submission_no_location_header() {
367506
let config = create_test_config("http://localhost:3000", "", "", "");
@@ -379,10 +518,11 @@ mod tests {
379518
.await;
380519

381520
assert!(result.is_err());
382-
assert!(result
383-
.unwrap_err()
384-
.to_string()
385-
.contains("missing 'Location' header"));
521+
let error_msg = result.unwrap_err().to_string();
522+
assert!(
523+
error_msg.contains("Location header validation failed")
524+
|| error_msg.contains("missing 'Location' header")
525+
);
386526
}
387527

388528
#[actix_rt::test]

0 commit comments

Comments
 (0)