switch to invokes instead of emits

This commit is contained in:
Joseph Montanaro 2022-12-13 16:50:44 -08:00
parent 48269855e5
commit 9055fa41aa
11 changed files with 91 additions and 40 deletions

View File

@ -6,7 +6,8 @@ The following is a list of security features that I hope to add eventually, in a
* Disallow all Tauri APIs except for `invoke` and `emit`. The sole job of the frontend should be to collect user interaction. Everything else should be mediated through the backend. * Disallow all Tauri APIs except for `invoke` and `emit`. The sole job of the frontend should be to collect user interaction. Everything else should be mediated through the backend.
* Maximally-restrictive CSP - not sure if Tauri does this by default. Also not sure whether it will interfere with IPC to set a zero-access CSP. * Maximally-restrictive CSP - not sure if Tauri does this by default. Also not sure whether it will interfere with IPC to set a zero-access CSP.
* Allow user to specify a role to assume, so that role can be given narrower permissions. Allow falling back to the root credentials in the event that broader permissions are required. (Unsure about this one, is there a good way to make it low-friction?) * Allow user to specify a role to assume, so that role can be given narrower permissions. Allow falling back to the root credentials in the event that broader permissions are required. (Unsure about this one, is there a good way to make it low-friction?)
* To defend against the possibility that an attacker could replace, say, the `aws` executable with a malicious one that snarfs your credentials and then passes the command on to the real one, maybe track the path (and maybe even the hash) of the executable, and raise a warning if this is the first time we've seen that one? Using the hash would be safer, but would also introduce a lot of false positives, since every time the application gets updated it would trigger. On the other hand, users should presumably know when they've updated things, so maybe it would be ok. On the _other_ other hand, if somebody doesn't use `aws` very often then it might be weeks or months in between updating it and actually using the updated executable, in which case they probably won't remember that this is the first time they've used it since updating. * To defend against the possibility that an attacker could replace, say, the `aws` executable with a malicious one that snarfs your credentials and then passes the command on to the real one, maybe track the path (and maybe even the hash) of the executable, and raise a warning if this is the first time we've seen that one? Using the hash would be safer, but would also introduce a lot of false positives, since every time the application gets updated it would trigger. On the other hand, users should presumably know when they've updated things, so maybe it would be ok. On the _other_ other hand, if somebody doesn't use `aws` very often then it might be weeks or months in between updating it and actually using the updated executable, in which case they probably won't remember that this is the first time they've used it since updating.
Another possible approach is to _watch_ the files in question, and alert the user whenever any of them changes. Presumably the user will know whether this change is expected or not.
* Downgrade privileges after launching. In particular, if possible, disallow any kind of outgoing network access (obviously we have to bind the listening socket, but maybe we can filter that down to _just_ the ability to bind that particular address/port) and filesystem access outside of state db. I think this is doable on Linux, although it may involve high levels of `seccomp` grossness. No idea whether it's possible on Windows. Probably possible on MacOS although it may require lengths to which I am currently unwilling to go (e.g. pay for a certificate from Apple or something.) * Downgrade privileges after launching. In particular, if possible, disallow any kind of outgoing network access (obviously we have to bind the listening socket, but maybe we can filter that down to _just_ the ability to bind that particular address/port) and filesystem access outside of state db. I think this is doable on Linux, although it may involve high levels of `seccomp` grossness. No idea whether it's possible on Windows. Probably possible on MacOS although it may require lengths to which I am currently unwilling to go (e.g. pay for a certificate from Apple or something.)
* "Panic button" - if a potential attack is detected (e.g. the user denies a request but Creddy discovers the request has already succeeded somehow), offer a one-click option to lock out the current IAM user. (Sadly, you can't revoke session tokens, so this is the only way to limit a potential compromise). Not sure how feasible this is, session credentials may be limited with regard to what kind of IAM operations they can carry out.) * "Panic button" - if a potential attack is detected (e.g. the user denies a request but Creddy discovers the request has already succeeded somehow), offer a one-click option to lock out the current IAM user. (Sadly, you can't revoke session tokens, so this is the only way to limit a potential compromise). Not sure how feasible this is, session credentials may be limited with regard to what kind of IAM operations they can carry out.)
* Some kind of Yubikey or other HST integration. (Optional, since not everyone will have a HST.) This comes in two flavors: * Some kind of Yubikey or other HST integration. (Optional, since not everyone will have a HST.) This comes in two flavors:
@ -26,7 +27,7 @@ Regardless, we live in a world where it's difficult to run untrusted code withou
## Particular attacks ## Particular attacks
There are lots of ways that I can imagine someone might try to circumvent Creddy's protection. Most of them require that the attacker be targeting Creddy in particular, rather than just "AWS credentials generally". In addition, most of them are "noisy" - that is, there's a good chance that the attack will alert the user to the fact that they are being attacked. This is generally something attackers try to avoid. There are lots of ways that I can imagine someone might try to circumvent Creddy's protection. Most of them require that the attacker be targeting Creddy in particular, rather than just "AWS credentials generally". In addition, most of them are "noisy" - that is, there's a good chance that the attack will alert the user to the fact that they are being attacked. This is generally something attackers try to avoid, since an easily-detected attack is likely to be shut down before it can spread very far.
### Tricking Creddy into allowing a request that it shouldn't ### Tricking Creddy into allowing a request that it shouldn't
@ -34,4 +35,14 @@ If an attacker is able to compromise Creddy's frontend, e.g. via a JS library th
### Tricking the user into allowing a request they didn't intend to ### Tricking the user into allowing a request they didn't intend to
If an attacker can edit the user's .bashrc or similar, they could theoretically insert a function or pre-command hook that wraps, say, the `aws` command, and If an attacker can edit the user's .bashrc or similar, they could theoretically insert a function or pre-command hook that wraps, say, the `aws` command, and dump the credentials before continuing on with the user's command. This would most likely alert the user because either a) the attacker is hijacking the original `aws` command and thus it doesn't do what the user told it to, or b) the user's original `aws` command proceeds as normal after the malicious one, and the user is alerted by the second request where there should only have been one.
A similar but more-difficult-to-detect attack would be replacing the `aws` executable, or any other executable that is always expected to ask for AWS credentials, with a malicious wrapper that snarfs the credentials before passing them through to the original command. Creddy could defend against this to a certain extent by storing the hash of the executable, as discussed above.
### Pretending to be the user
Most desktop environments don't prevent applications from simulating user-input events such as mouse clicks and keypresses. An attacker could issue a credentials request, then immediately simulate whatever hotkey or mouse click Creddy normally interprets as "confirm this request". To mitigate this Creddy could implement a minimum time for which it _must_ be on screen before dismissal. The attacker could try to wait for the machine to be unattended before executing this attack, but this is chancy and could still result in detection. The request would still be logged in any case.
### Twiddling with Creddy's persistent state
The solutions to or mitigations for a lot of these attacks rely on Creddy being able to assume that its local database hasn't been tampered with. Unfortunately, given that our threat model is "other code running as the same user", this isn't a safe assumption.

View File

@ -2,6 +2,7 @@ use netstat2::{AddressFamilyFlags, ProtocolFlags, ProtocolSocketInfo};
use sysinfo::{System, SystemExt, Pid, ProcessExt}; use sysinfo::{System, SystemExt, Pid, ProcessExt};
use crate::errors::*; use crate::errors::*;
use crate::ipc::Client;
fn get_associated_pids(local_port: u16) -> Result<Vec<u32>, netstat2::error::Error> { fn get_associated_pids(local_port: u16) -> Result<Vec<u32>, netstat2::error::Error> {
@ -22,8 +23,6 @@ fn get_associated_pids(local_port: u16) -> Result<Vec<u32>, netstat2::error::Err
&& proto_info.local_addr == std::net::Ipv4Addr::LOCALHOST && proto_info.local_addr == std::net::Ipv4Addr::LOCALHOST
&& proto_info.remote_addr == std::net::Ipv4Addr::LOCALHOST && proto_info.remote_addr == std::net::Ipv4Addr::LOCALHOST
{ {
println!("PIDs associated with socket: {:?}", &sock_info.associated_pids);
println!("Scanned {i} sockets");
return Ok(sock_info.associated_pids) return Ok(sock_info.associated_pids)
} }
} }
@ -31,16 +30,22 @@ fn get_associated_pids(local_port: u16) -> Result<Vec<u32>, netstat2::error::Err
} }
pub fn get_client_info(local_port: u16) -> Result<(), ClientInfoError> { // Theoretically, on some systems, multiple processes can share a socket. We have to
// account for this even though 99% of the time there will be only one.
pub fn get_clients(local_port: u16) -> Result<Vec<Client>, ClientInfoError> {
let mut clients = Vec::new();
let mut sys = System::new(); let mut sys = System::new();
for p in get_associated_pids(local_port)? { for p in get_associated_pids(local_port)? {
let pid = Pid::from(p as i32); let pid = Pid::from(p as i32);
sys.refresh_process(pid); sys.refresh_process(pid);
let proc = sys.process(pid) let proc = sys.process(pid)
.ok_or(ClientInfoError::PidNotFound)?; .ok_or(ClientInfoError::PidNotFound)?;
let path = proc.exe().to_string_lossy();
println!("exe for requesting process: {path}");
}
Ok(()) let client = Client {
pid: p,
exe: proc.exe().to_string_lossy().into_owned(),
};
clients.push(client);
}
Ok(clients)
} }

View File

@ -57,6 +57,7 @@ pub enum RequestError {
MalformedHttpRequest, MalformedHttpRequest,
RequestTooLarge, RequestTooLarge,
NoCredentials(GetCredentialsError), NoCredentials(GetCredentialsError),
ClientInfo(ClientInfoError),
} }
impl From<tokio::io::Error> for RequestError { impl From<tokio::io::Error> for RequestError {
fn from(e: std::io::Error) -> RequestError { fn from(e: std::io::Error) -> RequestError {
@ -73,6 +74,11 @@ impl From<GetCredentialsError> for RequestError {
RequestError::NoCredentials(e) RequestError::NoCredentials(e)
} }
} }
impl From<ClientInfoError> for RequestError {
fn from(e: ClientInfoError) -> RequestError {
RequestError::ClientInfo(e)
}
}
impl Display for RequestError { impl Display for RequestError {
fn fmt(&self, f: &mut Formatter) -> Result<(), std::fmt::Error> { fn fmt(&self, f: &mut Formatter) -> Result<(), std::fmt::Error> {
@ -84,6 +90,8 @@ impl Display for RequestError {
RequestTooLarge => write!(f, "HTTP request too large"), RequestTooLarge => write!(f, "HTTP request too large"),
NoCredentials(GetCredentialsError::Locked) => write!(f, "Recieved go-ahead but app is locked"), NoCredentials(GetCredentialsError::Locked) => write!(f, "Recieved go-ahead but app is locked"),
NoCredentials(GetCredentialsError::Empty) => write!(f, "Received go-ahead but no credentials are known"), NoCredentials(GetCredentialsError::Empty) => write!(f, "Received go-ahead but no credentials are known"),
ClientInfo(ClientInfoError::PidNotFound) => write!(f, "Could not resolve PID of client process."),
ClientInfo(ClientInfoError::NetstatError(e)) => write!(f, "Error getting client socket details: {e}"),
} }
} }
} }

View File

@ -4,9 +4,17 @@ use tauri::State;
use crate::state::{AppState, Session}; use crate::state::{AppState, Session};
#[derive(Copy, Clone, Serialize, Deserialize)] #[derive(Clone, Serialize, Deserialize)]
pub struct Client {
pub pid: u32,
pub exe: String,
}
#[derive(Clone, Serialize, Deserialize)]
pub struct Request { pub struct Request {
pub id: u64, pub id: u64,
pub clients: Vec<Client>,
} }
@ -41,7 +49,7 @@ pub async fn unlock(passphrase: String, app_state: State<'_, AppState>) -> Resul
#[tauri::command] #[tauri::command]
pub fn session_status(app_state: State<'_, AppState>) -> String { pub fn get_session_status(app_state: State<'_, AppState>) -> String {
let session = app_state.session.read().unwrap(); let session = app_state.session.read().unwrap();
match *session { match *session {
Session::Locked(_) => "locked".into(), Session::Locked(_) => "locked".into(),

View File

@ -21,7 +21,11 @@ fn main() {
tauri::Builder::default() tauri::Builder::default()
.manage(initial_state) .manage(initial_state)
.invoke_handler(tauri::generate_handler![ipc::unlock, ipc::respond]) .invoke_handler(tauri::generate_handler![
ipc::unlock,
ipc::respond,
ipc::get_session_status,
])
.setup(|app| { .setup(|app| {
let addr = std::net::SocketAddrV4::from_str("127.0.0.1:12345").unwrap(); let addr = std::net::SocketAddrV4::from_str("127.0.0.1:12345").unwrap();
tauri::async_runtime::spawn(server::serve(addr, app.handle())); tauri::async_runtime::spawn(server::serve(addr, app.handle()));

View File

@ -6,6 +6,7 @@ use tokio::sync::oneshot;
use tauri::{AppHandle, Manager}; use tauri::{AppHandle, Manager};
use crate::clientinfo;
use crate::errors::RequestError; use crate::errors::RequestError;
use crate::ipc::{Request, Approval}; use crate::ipc::{Request, Approval};
@ -46,12 +47,15 @@ async fn handle(mut stream: TcpStream, app_handle: AppHandle) -> Result<(), Requ
let app_state = app_handle.state::<crate::state::AppState>(); let app_state = app_handle.state::<crate::state::AppState>();
let request_id = app_state.register_request(chan_send); let request_id = app_state.register_request(chan_send);
if let std::net::SocketAddr::V4(addr) = stream.peer_addr()? { let peer_addr = match stream.peer_addr()? {
crate::clientinfo::get_client_info(addr.port()); std::net::SocketAddr::V4(addr) => addr,
} _ => unreachable!(), // we only listen on IPv4
};
let clients = clientinfo::get_clients(peer_addr.port())?;
// Do we want to panic if this fails? Does that mean the frontend is dead? // Do we want to panic if this fails? Does that mean the frontend is dead?
app_handle.emit_all("credentials-request", Request {id: request_id}).unwrap(); let req = Request {id: request_id, clients};
app_handle.emit_all("credentials-request", req).unwrap();
let mut buf = [0; 8192]; // it's what tokio's BufReader uses let mut buf = [0; 8192]; // it's what tokio's BufReader uses
let mut n = 0; let mut n = 0;

View File

@ -7,11 +7,15 @@
const dispatch = createEventDispatcher(); const dispatch = createEventDispatcher();
async function approve() { async function approve() {
if (appState.credentialStatus === 'unlocked') { let status = await invoke('get-session-status');
if (status === 'unlocked') {
dispatch('navigate', {target: 'ShowApproved'}); dispatch('navigate', {target: 'ShowApproved'});
} }
else if (status === 'locked') {
dispatch('navigate', {target: 'Unlock'})
}
else { else {
dispatch('navigate', {target: 'Unlock'}); dispatch('navigate', {target: 'EnterCredentials'});
} }
} }

View File

@ -8,10 +8,9 @@
onMount(async () => { onMount(async () => {
// will block until a request comes in // will block until a request comes in
let req = await appState.pendingRequests.get(); let req = await appState.pendingRequests.get();
console.log(req);
appState.currentRequest = req; appState.currentRequest = req;
console.log('Got credentials request from queue.'); console.log('Got credentials request from queue:');
console.log(appState); console.log(req);
dispatch('navigate', {target: 'Approve'}); dispatch('navigate', {target: 'Approve'});
}); });
</script> </script>

View File

@ -1,13 +1,18 @@
<script> <script>
import { createEventDispatcher } from 'svelte'; import { onMount, createEventDispatcher } from 'svelte';
import { emit } from '@tauri-apps/api/event'; import { emit } from '@tauri-apps/api/event';
import { invoke } from '@tauri-apps/api/tauri';
export let appState; export let appState;
var p = emit('request-response', {response: 'approved', requestId: 1}); onMount(async () => {
console.log('event emitted'); let response = {
console.log(p); id: appState.currentRequest.id,
appState.currentRequest = null; approval: 'Approved',
}
await invoke('respond', {response});
appState.currentRequest = null;
});
const dispatch = createEventDispatcher(); const dispatch = createEventDispatcher();
window.setTimeout(() => dispatch('navigate', {target: 'Home'}), 3000); window.setTimeout(() => dispatch('navigate', {target: 'Home'}), 3000);

View File

@ -1,11 +1,18 @@
<script> <script>
import { createEventDispatcher } from 'svelte'; import { onMount, createEventDispatcher } from 'svelte';
import { emit } from '@tauri-apps/api/event'; import { emit } from '@tauri-apps/api/event';
import { invoke } from '@tauri-apps/api/tauri';
export let appState; export let appState;
emit('request-response', {response: 'denied', requestId: 1}); onMount(async () => {
appState.currentRequest = null; let response = {
id: appState.currentRequest.id,
approval: 'Denied',
}
await invoke('respond', {response});
appState.currentRequest = null;
});
const dispatch = createEventDispatcher(); const dispatch = createEventDispatcher();
window.setTimeout(() => dispatch('navigate', {target: 'Home'}), 3000); window.setTimeout(() => dispatch('navigate', {target: 'Home'}), 3000);

View File

@ -9,16 +9,12 @@
let passphrase = ''; let passphrase = '';
async function unlock() { async function unlock() {
console.log('invoking unlock command.') console.log('invoking unlock command.')
let res = await invoke('unlock', {passphrase});
if (res) { try {
appState.credentialStatus = 'unlocked'; await invoke('unlock', {passphrase});
console.log('Unlock successful!');
if (appState.currentRequest) {
dispatch('navigate', {target: 'ShowApproved'});
}
} }
else { catch (e) {
// indicate decryption failed console.log('Unlock error:', e);
} }
} }
</script> </script>