diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b6a8ad2..5f7721b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ - Renamed config structs to better describe their purpose, and remove "Inner" from a struct that's being exposed via the API. ([!402](https://gitlab.com/veilid/veilid/-/merge_requests/402)) - `VeilidConfig` -> `VeilidStartupOptions` - `VeilidConfigInner` -> `VeilidConfig` + - Gate insecure capabilities behind the new `footgun` feature flag ([#394](https://gitlab.com/veilid/veilid/-/issues/394)), which is disabled by default. ([!400](https://gitlab.com/veilid/veilid/-/merge_requests/400)) + - Calling `app_call` or `app_message` with a `NodeId` target will throw an error. Use a `PrivateRoute` target instead. + - Creating an `Unsafe` routing context will throw and error. Use `Safe` routing context instead. + - Any AppCall or AppMessage sent from a direct NodeId will not be received. - veilid-core: - **Security** Signed bootstrap v1 added which closes #293: https://gitlab.com/veilid/veilid/-/issues/293 diff --git a/veilid-core/Cargo.toml b/veilid-core/Cargo.toml index ab11bd1d..6b565722 100644 --- a/veilid-core/Cargo.toml +++ b/veilid-core/Cargo.toml @@ -62,6 +62,10 @@ virtual-network-server = ["veilid-tools/virtual-network-server"] # GeoIP geolocation = ["maxminddb", "reqwest"] +# Features that go against "Natural Security" concepts +# https://gitlab.com/veilid/veilid/-/issues/420 +footgun = [] + ### DEPENDENCIES [dependencies] diff --git a/veilid-core/src/core_context.rs b/veilid-core/src/core_context.rs index 1c27f667..fc34039e 100644 --- a/veilid-core/src/core_context.rs +++ b/veilid-core/src/core_context.rs @@ -73,6 +73,11 @@ impl VeilidCoreContext { veilid_log!(registry info "Veilid API starting up"); veilid_log!(registry info "Version: {}", veilid_version_string()); veilid_log!(registry info "Features: {:?}", veilid_features()); + #[cfg(feature = "footgun")] + { + veilid_log!(registry warn + "Footgun feature is enabled. This disables sender privacy protections and should be avoided in production."); + } // Register all components registry.register(ProtectedStore::new); diff --git a/veilid-core/src/rpc_processor/rpc_app_call.rs b/veilid-core/src/rpc_processor/rpc_app_call.rs index 3e63b968..f66cd2cf 100644 --- a/veilid-core/src/rpc_processor/rpc_app_call.rs +++ b/veilid-core/src/rpc_processor/rpc_app_call.rs @@ -118,6 +118,15 @@ impl RPCProcessor { .as_ref() .map(|nr| nr.node_ids().get(crypto_kind).unwrap()); + #[cfg(not(feature = "footgun"))] + { + if sender.is_some() { + return Ok(NetworkResult::invalid_message( + "Direct NodeId senders are not allowed for AppCall when footgun is disabled", + )); + } + } + // Register a waiter for this app call let handle = self.waiting_app_call_table.add_op_waiter(op_id, ()); diff --git a/veilid-core/src/rpc_processor/rpc_app_message.rs b/veilid-core/src/rpc_processor/rpc_app_message.rs index 05985366..bbd99f27 100644 --- a/veilid-core/src/rpc_processor/rpc_app_message.rs +++ b/veilid-core/src/rpc_processor/rpc_app_message.rs @@ -81,6 +81,15 @@ impl RPCProcessor { .as_ref() .map(|nr| nr.node_ids().get(crypto_kind).unwrap()); + #[cfg(not(feature = "footgun"))] + { + if sender.is_some() { + return Ok(NetworkResult::invalid_message( + "Direct NodeId senders are not allowed for AppMessage when footgun is disabled", + )); + } + } + // Pass the message up through the update callback let message = app_message.destructure(); (self.update_callback())(VeilidUpdate::AppMessage(Box::new(VeilidAppMessage::new( diff --git a/veilid-core/src/tests/common/test_dht.rs b/veilid-core/src/tests/common/test_dht.rs index 26346579..602322b2 100644 --- a/veilid-core/src/tests/common/test_dht.rs +++ b/veilid-core/src/tests/common/test_dht.rs @@ -11,55 +11,35 @@ lazy_static! { } pub async fn test_get_dht_value_unopened(api: VeilidAPI) { - let rc = api - .routing_context() - .unwrap() - .with_safety(SafetySelection::Unsafe(Sequencing::EnsureOrdered)) - .unwrap(); + let rc = api.routing_context().unwrap(); let result = rc.get_dht_value(*BOGUS_KEY, 0, false).await; assert_err!(result); } pub async fn test_open_dht_record_nonexistent_no_writer(api: VeilidAPI) { - let rc = api - .routing_context() - .unwrap() - .with_safety(SafetySelection::Unsafe(Sequencing::EnsureOrdered)) - .unwrap(); + let rc = api.routing_context().unwrap(); let result = rc.get_dht_value(*BOGUS_KEY, 0, false).await; assert_err!(result); } pub async fn test_close_dht_record_nonexistent(api: VeilidAPI) { - let rc = api - .routing_context() - .unwrap() - .with_safety(SafetySelection::Unsafe(Sequencing::EnsureOrdered)) - .unwrap(); + let rc = api.routing_context().unwrap(); let result = rc.close_dht_record(*BOGUS_KEY).await; assert_err!(result); } pub async fn test_delete_dht_record_nonexistent(api: VeilidAPI) { - let rc = api - .routing_context() - .unwrap() - .with_safety(SafetySelection::Unsafe(Sequencing::EnsureOrdered)) - .unwrap(); + let rc = api.routing_context().unwrap(); let result = rc.delete_dht_record(*BOGUS_KEY).await; assert_err!(result); } pub async fn test_create_delete_dht_record_simple(api: VeilidAPI) { - let rc = api - .routing_context() - .unwrap() - .with_safety(SafetySelection::Unsafe(Sequencing::EnsureOrdered)) - .unwrap(); + let rc = api.routing_context().unwrap(); let rec = rc .create_dht_record(DHTSchema::dflt(1).unwrap(), None, Some(CRYPTO_KIND_VLD0)) @@ -72,11 +52,7 @@ pub async fn test_create_delete_dht_record_simple(api: VeilidAPI) { } pub async fn test_create_dht_record_with_owner(api: VeilidAPI) { - let rc = api - .routing_context() - .unwrap() - .with_safety(SafetySelection::Unsafe(Sequencing::EnsureOrdered)) - .unwrap(); + let rc = api.routing_context().unwrap(); let crypto = api.crypto().unwrap(); let cs = crypto.get(CRYPTO_KIND_VLD0).unwrap(); @@ -99,11 +75,7 @@ pub async fn test_create_dht_record_with_owner(api: VeilidAPI) { } pub async fn test_get_dht_record_key(api: VeilidAPI) { - let rc = api - .routing_context() - .unwrap() - .with_safety(SafetySelection::Unsafe(Sequencing::EnsureOrdered)) - .unwrap(); + let rc = api.routing_context().unwrap(); let crypto = api.crypto().unwrap(); let cs = crypto.get(CRYPTO_KIND_VLD0).unwrap(); @@ -130,11 +102,7 @@ pub async fn test_get_dht_record_key(api: VeilidAPI) { } pub async fn test_get_dht_value_nonexistent(api: VeilidAPI) { - let rc = api - .routing_context() - .unwrap() - .with_safety(SafetySelection::Unsafe(Sequencing::EnsureOrdered)) - .unwrap(); + let rc = api.routing_context().unwrap(); let rec = rc .create_dht_record(DHTSchema::dflt(1).unwrap(), None, Some(CRYPTO_KIND_VLD0)) @@ -149,11 +117,7 @@ pub async fn test_get_dht_value_nonexistent(api: VeilidAPI) { } pub async fn test_set_get_dht_value(api: VeilidAPI) { - let rc = api - .routing_context() - .unwrap() - .with_safety(SafetySelection::Unsafe(Sequencing::EnsureOrdered)) - .unwrap(); + let rc = api.routing_context().unwrap(); let rec = rc .create_dht_record(DHTSchema::dflt(2).unwrap(), None, Some(CRYPTO_KIND_VLD0)) @@ -201,11 +165,7 @@ pub async fn test_set_get_dht_value(api: VeilidAPI) { } pub async fn test_open_writer_dht_value(api: VeilidAPI) { - let rc = api - .routing_context() - .unwrap() - .with_safety(SafetySelection::Unsafe(Sequencing::EnsureOrdered)) - .unwrap(); + let rc = api.routing_context().unwrap(); let rec = rc .create_dht_record(DHTSchema::dflt(2).unwrap(), None, Some(CRYPTO_KIND_VLD0)) diff --git a/veilid-core/src/veilid_api/routing_context.rs b/veilid-core/src/veilid_api/routing_context.rs index b387283c..d1a836c9 100644 --- a/veilid-core/src/veilid_api/routing_context.rs +++ b/veilid-core/src/veilid_api/routing_context.rs @@ -101,6 +101,15 @@ impl RoutingContext { veilid_log!(self debug "RoutingContext::with_safety(self: {:?}, safety_selection: {:?})", self, safety_selection); + if let SafetySelection::Unsafe(_) = safety_selection { + #[cfg(not(feature = "footgun"))] + { + return Err(VeilidAPIError::generic( + "Unsafe routing mode is not allowed without the 'footgun' feature enabled", + )); + } + } + Ok(Self { api: self.api.clone(), unlocked_inner: Arc::new(RoutingContextUnlockedInner { safety_selection }), @@ -161,19 +170,12 @@ impl RoutingContext { .map_err(VeilidAPIError::invalid_target) } - //////////////////////////////////////////////////////////////// - // App-level Messaging - - /// App-level bidirectional call that expects a response to be returned. - /// - /// Veilid apps may use this for arbitrary message passing. - /// - /// * `target` - can be either a direct node id or a private route. - /// * `message` - an arbitrary message blob of up to 32768 bytes. - /// - /// Returns an answer blob of up to 32768 bytes. #[instrument(target = "veilid_api", level = "debug", fields(__VEILID_LOG_KEY = self.log_key()), ret, err)] - pub async fn app_call(&self, target: Target, message: Vec) -> VeilidAPIResult> { + async fn internal_app_call( + &self, + target: Target, + message: Vec, + ) -> VeilidAPIResult> { veilid_log!(self debug "RoutingContext::app_call(self: {:?}, target: {:?}, message: {:?})", self, target, message); @@ -200,14 +202,45 @@ impl RoutingContext { Ok(answer.answer) } - /// App-level unidirectional message that does not expect any value to be returned. + #[cfg(feature = "footgun")] + //////////////////////////////////////////////////////////////// + // App-level Messaging + + /// App-level bidirectional call that expects a response to be returned. /// /// Veilid apps may use this for arbitrary message passing. /// /// * `target` - can be either a direct node id or a private route. /// * `message` - an arbitrary message blob of up to 32768 bytes. + /// + /// Returns an answer blob of up to 32768 bytes. + pub async fn app_call(&self, target: Target, message: Vec) -> VeilidAPIResult> { + self.internal_app_call(target, message).await + } + + #[cfg(not(feature = "footgun"))] + //////////////////////////////////////////////////////////////// + // App-level Messaging + + /// App-level bidirectional call that expects a response to be returned. + /// + /// Veilid apps may use this for arbitrary message passing. + /// + /// * `target` - a private route. + /// * `message` - an arbitrary message blob of up to 32768 bytes. + /// + /// Returns an answer blob of up to 32768 bytes. + pub async fn app_call(&self, target: Target, message: Vec) -> VeilidAPIResult> { + match target { + Target::PrivateRoute(_) => self.internal_app_call(target, message).await, + Target::NodeId(_) => Err(VeilidAPIError::invalid_target( + "Only PrivateRoute targets are allowed without the footgun feature", + )), + } + } + #[instrument(target = "veilid_api", level = "debug", fields(__VEILID_LOG_KEY = self.log_key()), ret, err)] - pub async fn app_message(&self, target: Target, message: Vec) -> VeilidAPIResult<()> { + async fn internal_app_message(&self, target: Target, message: Vec) -> VeilidAPIResult<()> { veilid_log!(self debug "RoutingContext::app_message(self: {:?}, target: {:?}, message: {:?})", self, target, message); @@ -233,6 +266,33 @@ impl RoutingContext { Ok(()) } + #[cfg(feature = "footgun")] + /// App-level unidirectional message that does not expect any value to be returned. + /// + /// Veilid apps may use this for arbitrary message passing. + /// + /// * `target` - can be either a direct node id or a private route. + /// * `message` - an arbitrary message blob of up to 32768 bytes. + pub async fn app_message(&self, target: Target, message: Vec) -> VeilidAPIResult<()> { + self.internal_app_message(target, message).await + } + + #[cfg(not(feature = "footgun"))] + /// App-level unidirectional message that does not expect any value to be returned. + /// + /// Veilid apps may use this for arbitrary message passing. + /// + /// * `target` - a private route. + /// * `message` - an arbitrary message blob of up to 32768 bytes. + pub async fn app_message(&self, target: Target, message: Vec) -> VeilidAPIResult<()> { + match target { + Target::PrivateRoute(_) => self.internal_app_message(target, message).await, + Target::NodeId(_) => Err(VeilidAPIError::invalid_target( + "Only PrivateRoute targets are allowed without the footgun feature", + )), + } + } + /////////////////////////////////// // DHT Records diff --git a/veilid-flutter/example/integration_test/test_dht.dart b/veilid-flutter/example/integration_test/test_dht.dart index bdc31648..b613f209 100644 --- a/veilid-flutter/example/integration_test/test_dht.dart +++ b/veilid-flutter/example/integration_test/test_dht.dart @@ -319,7 +319,7 @@ Future testWatchDHTValues(Stream updateStream) async { // as the watch's target final rcSet = await Veilid.instance.routingContext(); - final rcWatch = await Veilid.instance.unsafeRoutingContext(); + final rcWatch = await Veilid.instance.safeRoutingContext(); try { // Make a DHT record var rec = await rcWatch.createDHTRecord(const DHTSchema.dflt(oCnt: 10)); diff --git a/veilid-flutter/rust/Cargo.toml b/veilid-flutter/rust/Cargo.toml index e349d12f..bd88878b 100644 --- a/veilid-flutter/rust/Cargo.toml +++ b/veilid-flutter/rust/Cargo.toml @@ -32,6 +32,7 @@ rt-tokio = [ "opentelemetry/rt-tokio", ] debug-load = ["dep:ctor", "dep:libc-print", "dep:android_log-sys", "dep:oslog"] +footgun = ["veilid-core/footgun"] [dependencies] veilid-core = { path = "../../veilid-core", default-features = false } diff --git a/veilid-python/tests/test_dht.py b/veilid-python/tests/test_dht.py index 1555f7c6..817a466b 100644 --- a/veilid-python/tests/test_dht.py +++ b/veilid-python/tests/test_dht.py @@ -448,11 +448,11 @@ async def test_watch_many_dht_values(): # make routing contexts # unsafe version for debugging - rc0 = await (await api0.new_routing_context()).with_safety(SafetySelection.unsafe()) - rc1 = await (await api1.new_routing_context()).with_safety(SafetySelection.unsafe()) + # rc0 = await (await api0.new_routing_context()).with_safety(SafetySelection.unsafe()) + # rc1 = await (await api1.new_routing_context()).with_safety(SafetySelection.unsafe()) # safe default version - # rc0 = await api0.new_routing_context() - # rc1 = await api1.new_routing_context() + rc0 = await api0.new_routing_context() + rc1 = await api1.new_routing_context() async with rc0, rc1: diff --git a/veilid-python/tests/test_routing_context.py b/veilid-python/tests/test_routing_context.py index ae8a0a3e..9f3d6fac 100644 --- a/veilid-python/tests/test_routing_context.py +++ b/veilid-python/tests/test_routing_context.py @@ -34,9 +34,15 @@ async def test_routing_contexts(api_connection: veilid.VeilidAPI): ) await rc.release() - rc = await (await api_connection.new_routing_context()).with_safety( - veilid.SafetySelection.unsafe(veilid.Sequencing.PREFER_ORDERED) - ) + + # Test that creating an Unsafe routing context throws an error + # as mentioned in the CHANGELOG (footgun feature disabled by default) + rc = await api_connection.new_routing_context() + with pytest.raises(Exception): + rc = await rc.with_safety( + veilid.SafetySelection.unsafe(veilid.Sequencing.PREFER_ORDERED) + ) + await rc.release() diff --git a/veilid-server/Cargo.toml b/veilid-server/Cargo.toml index 377bc4ee..71f99f30 100644 --- a/veilid-server/Cargo.toml +++ b/veilid-server/Cargo.toml @@ -18,6 +18,7 @@ path = "src/main.rs" [features] default = ["rt-tokio", "veilid-core/default", "otlp-tonic"] default-async-std = ["rt-async-std", "veilid-core/default-async-std"] +footgun = ["veilid-core/footgun"] virtual-network = [ "veilid-core/virtual-network", diff --git a/veilid-wasm/Cargo.toml b/veilid-wasm/Cargo.toml index 1e2e8805..d4c3f505 100644 --- a/veilid-wasm/Cargo.toml +++ b/veilid-wasm/Cargo.toml @@ -18,6 +18,7 @@ path = "src/lib.rs" [features] default = ["veilid-core/default-wasm"] crypto-test = ["veilid-core/crypto-test"] +footgun = ["veilid-core/footgun"] [dependencies] veilid-core = { version = "0.4.4", path = "../veilid-core", default-features = false } diff --git a/veilid-wasm/tests/src/VeilidRoutingContext.test.ts b/veilid-wasm/tests/src/VeilidRoutingContext.test.ts index bd49e3fb..dd625226 100644 --- a/veilid-wasm/tests/src/VeilidRoutingContext.test.ts +++ b/veilid-wasm/tests/src/VeilidRoutingContext.test.ts @@ -23,7 +23,7 @@ describe('VeilidRoutingContext', () => { // } }, veilidCoreStartupConfig); await veilidClient.attach(); - await asyncCallWithTimeout(waitForPublicAttachment(), 10000); + await asyncCallWithTimeout(waitForPublicAttachment(), 30_000); //console.log("---Started Up---"); }); @@ -65,15 +65,21 @@ describe('VeilidRoutingContext', () => { VeilidRoutingContext.create().withSequencing('EnsureOrdered'); expect(routingContext instanceof VeilidRoutingContext).toBe(true); }); + + it('should error if unsafe is used', async () => { + expect(() => { + VeilidRoutingContext.create().withSafety({ + Unsafe: 'EnsureOrdered', + }); + }).toThrow(); + }); }); describe('operations', () => { let routingContext: VeilidRoutingContext; before('create routing context', () => { - routingContext = VeilidRoutingContext.create().withSafety({ - Unsafe: 'EnsureOrdered', - }); + routingContext = VeilidRoutingContext.create(); }); describe('createDhtRecord', () => { diff --git a/veilid-wasm/tests/src/veilidClient.test.ts b/veilid-wasm/tests/src/veilidClient.test.ts index 2ea76b70..454bfd23 100644 --- a/veilid-wasm/tests/src/veilidClient.test.ts +++ b/veilid-wasm/tests/src/veilidClient.test.ts @@ -75,7 +75,7 @@ describe('veilidClient', function () { }); it('should call debug command', async function () { - const response = await veilidClient.debug('txtrecord'); + const response = await veilidClient.debug('help'); expect(response).toBeDefined(); expect(response.length).toBeGreaterThan(0); });