fix: fetch_root_key refuses to play along if called on the mainnet #526
Conversation
| @@ -330,6 +330,11 @@ impl Agent { | |||
| } | |||
| let status = self.status().await?; | |||
| let root_key = match status.root_key { | |||
| Some(key) if key[..] == IC_ROOT_KEY[..] => { | |||
There was a problem hiding this comment.
This wouldn't apply in case of an actual man-in-the-middle-attack.
There was a problem hiding this comment.
Correct. What this is meant to protect against is accidentally fetching the root key on mainnet, which opens the door to the man-in-the-middle attack.
This applies in the case where you are erroneously fetching the root key on mainnet. Prior to this change you won't notice, because everything works: you fetch the root key from mainnet, replace your root key with it (the same key), and go about your day. You shouldn't have, but no harm done, right? It "works" locally, it "works" on mainnet, it "works" on any testnet. It "works" everywhere; there's just one problem...
Then one day, you connect with the agent, only there's been a DNS hack and you connect somewhere else: a man-in-the-middle. You fetch the root key, like you always do. It's not the the IC_ROOT_KEY this time, but you don't notice, and all the signatures check out. You're fallen for the man-in-the-middle attack.
This change is intended to make you notice that you are fetching the root key from mainnet (opening yourself up to this attack) and stop doing that.
Does that make sense?
There was a problem hiding this comment.
It does make sense to nudge people not to fetch the root key from mainnet. But if we wanted to make sure that people don't fetch the root key from mainnet, we'd need to take the URL into account.
There was a problem hiding this comment.
There are other places that look at the URL and later skip the call or flag this as an error. But I wouldn't consider them as making sure, because there are a lot of ways to express the same URL.
This method however does seem like a way of making sure that you aren't blithely calling fetch_root_key against , because if you do, the only time it doesn't notice is if there is a MITM attack going on.
There was a problem hiding this comment.
Sounds good. Thank you for your explanations!
Co-authored-by: Linwei Shang <[email protected]>
Description
It's a mistake to call
Agent.fetch_root_key()on the mainnet. This makes it also an error, and also zeroes out the root key so that the agent would fail to verify any signatures even if the caller ignored the error.How Has This Been Tested?
Added a test
Checklist: