diff --git a/dstack/kms/auth-eth/contracts/DstackApp.sol b/dstack/kms/auth-eth/contracts/DstackApp.sol index 6c27e9ccb..7d90f29ef 100644 --- a/dstack/kms/auth-eth/contracts/DstackApp.sol +++ b/dstack/kms/auth-eth/contracts/DstackApp.sol @@ -145,6 +145,7 @@ contract DstackApp is // Add a compose hash to allowed list function addComposeHash(bytes32 composeHash) external onlyOwner { + require(composeHash != bytes32(0), "invalid compose hash"); allowedComposeHashes[composeHash] = true; emit ComposeHashAdded(composeHash); _emitPolicy("compose-hash", composeHash, true); @@ -173,6 +174,7 @@ contract DstackApp is // Add a device ID to allowed list function addDevice(bytes32 deviceId) external onlyOwner { + require(deviceId != bytes32(0), "invalid device ID"); allowedDeviceIds[deviceId] = true; emit DeviceAdded(deviceId); _emitPolicy("device", deviceId, true); diff --git a/dstack/kms/auth-eth/test/DstackApp.t.sol b/dstack/kms/auth-eth/test/DstackApp.t.sol index 8ad8050e7..646bd827e 100644 --- a/dstack/kms/auth-eth/test/DstackApp.t.sol +++ b/dstack/kms/auth-eth/test/DstackApp.t.sol @@ -391,4 +391,13 @@ contract DstackAppTest is Test { app.addComposeHash(bytes32(uint256(1))); assertTrue(app.allowedComposeHashes(bytes32(uint256(1)))); } + + function test_AddZeroValuedPolicyEntriesIsRejected() public { + vm.startPrank(owner); + vm.expectRevert("invalid compose hash"); + app.addComposeHash(bytes32(0)); + vm.expectRevert("invalid device ID"); + app.addDevice(bytes32(0)); + vm.stopPrank(); + } } diff --git a/dstack/kms/src/main_service/upgrade_authority.rs b/dstack/kms/src/main_service/upgrade_authority.rs index 135f49de4..77306d870 100644 --- a/dstack/kms/src/main_service/upgrade_authority.rs +++ b/dstack/kms/src/main_service/upgrade_authority.rs @@ -49,6 +49,19 @@ pub(crate) fn ensure_app_id_len(app_id: &[u8]) -> Result<()> { Ok(()) } +/// Policy backends left-pad short hex values, so a short field would alias a +/// whitelisted full-width one. +fn ensure_identity_widths(boot_info: &BootInfo) -> Result<()> { + ensure_app_id_len(&boot_info.app_id)?; + if boot_info.compose_hash.len() != 32 { + bail!("compose_hash must be 32 bytes"); + } + if !matches!(boot_info.instance_id.len(), 0 | 20) { + bail!("instance_id must be 20 bytes or empty"); + } + Ok(()) +} + pub(crate) async fn local_kms_boot_info(verifier: &AttestationVerifier) -> Result { let response = app_attest(pad64([0u8; 32])) .await @@ -136,6 +149,7 @@ async fn send_request(req: reqwest::RequestBuilder, url: &s impl AuthApi { pub async fn is_app_allowed(&self, boot_info: &BootInfo, is_kms: bool) -> Result { + ensure_identity_widths(boot_info)?; match self { AuthApi::Dev { dev } => Ok(BootResponse { is_allowed: true, @@ -363,6 +377,43 @@ mod tests { } } + #[rocket::async_test] + async fn short_identities_are_refused_before_they_reach_the_backend() { + let (url, server) = serve(vec![ + r#"{"isAllowed":true,"gatewayAppId":"gateway","reason":"well-formed"}"#, + ]); + let auth = webhook(url); + + let mut aliasing_compose_hash = boot_info(1); + aliasing_compose_hash.compose_hash.truncate(31); + let err = auth + .is_app_allowed(&aliasing_compose_hash, false) + .await + .unwrap_err(); + assert!(err.to_string().contains("compose_hash must be 32 bytes")); + + let mut aliasing_instance_id = boot_info(1); + aliasing_instance_id.instance_id.truncate(19); + let err = auth + .is_app_allowed(&aliasing_instance_id, false) + .await + .unwrap_err(); + assert!(err + .to_string() + .contains("instance_id must be 20 bytes or empty")); + + // `--no-instance-id` deployments carry no instance id. + let mut no_instance_id = boot_info(1); + no_instance_id.instance_id.clear(); + assert!( + auth.is_app_allowed(&no_instance_id, false) + .await + .unwrap() + .is_allowed + ); + assert_eq!(server.join().unwrap().len(), 1); + } + #[rocket::async_test] async fn repeated_authorization_is_never_served_from_a_decision_cache() { let (url, server) = serve(vec![