From 5615e5a7fefea19d37f4402e47ff31af73efabb1 Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Thu, 15 Oct 2020 10:00:54 +0100 Subject: [PATCH 1/3] agent: Fix crasher if UpdateInterface request empty Check if the interface specified in the `UpdateInterface` API is set before using it to avoid crashing the agent. Fixes: #950. Signed-off-by: James O. D. Hunt --- src/agent/src/rpc.rs | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/agent/src/rpc.rs b/src/agent/src/rpc.rs index d7b95cd7b6..4b53c713d3 100644 --- a/src/agent/src/rpc.rs +++ b/src/agent/src/rpc.rs @@ -919,6 +919,13 @@ impl protocols::agent_ttrpc::AgentService for agentService { _ctx: &ttrpc::TtrpcContext, req: protocols::agent::UpdateInterfaceRequest, ) -> ttrpc::Result { + if req.interface.is_none() { + return Err(ttrpc::Error::RpcStatus(ttrpc::get_status( + ttrpc::Code::INVALID_ARGUMENT, + format!("empty update interface request"), + ))); + } + let interface = req.interface.clone(); let s = Arc::clone(&self.sandbox); let mut sandbox = s.lock().unwrap(); @@ -1755,7 +1762,27 @@ fn load_kernel_module(module: &protocols::agent::KernelModule) -> Result<()> { #[cfg(test)] mod tests { use super::*; + use crate::protocols::agent_ttrpc::AgentService; use oci::{Hook, Hooks}; + use std::sync::mpsc::{Receiver, Sender}; + use ttrpc::{MessageHeader, TtrpcContext}; + + fn mk_ttrpc_context() -> (TtrpcContext, Receiver<(MessageHeader, Vec)>) { + let mh = MessageHeader::default(); + + let (tx, rx): ( + Sender<(MessageHeader, Vec)>, + Receiver<(MessageHeader, Vec)>, + ) = channel(); + + let ctx = TtrpcContext { + fd: -1, + mh: mh, + res_tx: tx, + }; + + (ctx, rx) + } #[test] fn test_load_kernel_module() { @@ -1795,4 +1822,21 @@ mod tests { append_guest_hooks(&s, &mut oci); assert_eq!(s.hooks, oci.hooks); } + + #[test] + fn test_update_interface() { + let logger = slog::Logger::root(slog::Discard, o!()); + let sandbox = Sandbox::new(&logger).unwrap(); + + let agent_service = Box::new(agentService { + sandbox: Arc::new(Mutex::new(sandbox)), + }); + + let req = protocols::agent::UpdateInterfaceRequest::default(); + let (ctx, _) = mk_ttrpc_context(); + + let result = agent_service.update_interface(&ctx, req); + + assert!(result.is_err(), "expected update interface to fail"); + } } From 3d084c7d2387b01cd91010a1ca7db64802f4d157 Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Thu, 15 Oct 2020 10:03:13 +0100 Subject: [PATCH 2/3] agent: Fix crasher if UpdateRoutes request empty Check if the routes specified in the `UpdateRoutes` API is set before using it to avoid crashing the agent. Fixes: #949. Signed-off-by: James O. D. Hunt --- src/agent/src/rpc.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/agent/src/rpc.rs b/src/agent/src/rpc.rs index 4b53c713d3..b7fe53ce97 100644 --- a/src/agent/src/rpc.rs +++ b/src/agent/src/rpc.rs @@ -954,6 +954,13 @@ impl protocols::agent_ttrpc::AgentService for agentService { req: protocols::agent::UpdateRoutesRequest, ) -> ttrpc::Result { let mut routes = protocols::agent::Routes::new(); + if req.routes.is_none() { + return Err(ttrpc::Error::RpcStatus(ttrpc::get_status( + ttrpc::Code::INVALID_ARGUMENT, + format!("empty update routes request"), + ))); + } + let rs = req.routes.clone().unwrap().Routes.into_vec(); let s = Arc::clone(&self.sandbox); @@ -1839,4 +1846,21 @@ mod tests { assert!(result.is_err(), "expected update interface to fail"); } + + #[test] + fn test_update_routes() { + let logger = slog::Logger::root(slog::Discard, o!()); + let sandbox = Sandbox::new(&logger).unwrap(); + + let agent_service = Box::new(agentService { + sandbox: Arc::new(Mutex::new(sandbox)), + }); + + let req = protocols::agent::UpdateRoutesRequest::default(); + let (ctx, _) = mk_ttrpc_context(); + + let result = agent_service.update_routes(&ctx, req); + + assert!(result.is_err(), "expected update routes to fail"); + } } From 8495306641f8edd2af467a6de65e94662b409288 Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Thu, 15 Oct 2020 10:05:27 +0100 Subject: [PATCH 3/3] agent: Fix crasher if AddARPNeighbors request empty Check if the ARP neighbours specified in the `AddARPNeighbors` API is set before using it to avoid crashing the agent. Fixes: #955. Signed-off-by: James O. D. Hunt --- src/agent/src/rpc.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/agent/src/rpc.rs b/src/agent/src/rpc.rs index b7fe53ce97..6ef6a7cffe 100644 --- a/src/agent/src/rpc.rs +++ b/src/agent/src/rpc.rs @@ -1156,6 +1156,13 @@ impl protocols::agent_ttrpc::AgentService for agentService { _ctx: &ttrpc::TtrpcContext, req: protocols::agent::AddARPNeighborsRequest, ) -> ttrpc::Result { + if req.neighbors.is_none() { + return Err(ttrpc::Error::RpcStatus(ttrpc::get_status( + ttrpc::Code::INVALID_ARGUMENT, + format!("empty add arp neighbours request"), + ))); + } + let neighs = req.neighbors.clone().unwrap().ARPNeighbors.into_vec(); let s = Arc::clone(&self.sandbox); @@ -1863,4 +1870,21 @@ mod tests { assert!(result.is_err(), "expected update routes to fail"); } + + #[test] + fn test_add_arp_neighbors() { + let logger = slog::Logger::root(slog::Discard, o!()); + let sandbox = Sandbox::new(&logger).unwrap(); + + let agent_service = Box::new(agentService { + sandbox: Arc::new(Mutex::new(sandbox)), + }); + + let req = protocols::agent::AddARPNeighborsRequest::default(); + let (ctx, _) = mk_ttrpc_context(); + + let result = agent_service.add_arp_neighbors(&ctx, req); + + assert!(result.is_err(), "expected add arp neighbors to fail"); + } }