diff options
author | 2012-09-11 11:48:50 -0400 | |
---|---|---|
committer | 2012-09-11 11:48:50 -0400 | |
commit | 084dc188c700c1ff7c0ce5db86715b2ece5bf6f2 (patch) | |
tree | 1c670ad7342895eee7b289cd7790a27da19bb290 | |
parent | 57bee4b43076be2dfe60c47088c8b1f095278248 (diff) |
additional security sanity checks on pairing messages
-rw-r--r-- | Assistant/Threads/PairListener.hs | 24 | ||||
-rw-r--r-- | doc/design/assistant/pairing.mdwn | 31 |
2 files changed, 46 insertions, 9 deletions
diff --git a/Assistant/Threads/PairListener.hs b/Assistant/Threads/PairListener.hs index 09fd9513d..93eef65ba 100644 --- a/Assistant/Threads/PairListener.hs +++ b/Assistant/Threads/PairListener.hs @@ -22,6 +22,7 @@ import Utility.Tense import Network.Multicast import Network.Socket import qualified Data.Text as T +import Data.Char thisThread :: ThreadName thisThread = "PairListener" @@ -36,16 +37,18 @@ pairListenerThread st dstatus scanremotes urlrenderer = thread $ withSocketsDo $ go sock cache = getmsg sock [] >>= \msg -> case readish msg of Nothing -> go sock cache Just m -> do + sane <- checkSane msg (pip, verified) <- verificationCheck m =<< (pairingInProgress <$> getDaemonStatus dstatus) - case pairMsgStage m of - PairReq -> do + case (sane, pairMsgStage m) of + (False, _) -> go sock cache + (_, PairReq) -> do pairReqReceived verified dstatus urlrenderer m go sock $ invalidateCache m cache - PairAck -> do + (_, PairAck) -> do pairAckReceived verified pip st dstatus scanremotes m cache >>= go sock - PairDone -> do + (_, PairDone) -> do pairDoneReceived verified pip st dstatus scanremotes m go sock cache @@ -53,7 +56,8 @@ pairListenerThread st dstatus scanremotes urlrenderer = thread $ withSocketsDo $ - check its UUID against the UUID we have stored. If - they're the same, someone is sending bogus messages, - which could be an attempt to brute force the shared - - secret. -} + - secret. + -} verificationCheck m (Just pip) = do let verified = verifiedPairMsg m pip let sameuuid = pairUUID (inProgressPairData pip) == pairUUID (pairMsgData $ m) @@ -65,6 +69,16 @@ pairListenerThread st dstatus scanremotes urlrenderer = thread $ withSocketsDo $ return (Nothing, False) else return (Just pip, verified && sameuuid) verificationCheck _ Nothing = return (Nothing, False) + + {- Various sanity checks on the content of the message. -} + checkSane msg + {- Control characters could be used in a + - console poisoning attack. -} + | any isControl msg || any (`elem` "\r\n") msg = do + runThreadState st $ + warning "illegal control characters in pairing message; ignoring" + return False + | otherwise = return True {- PairReqs invalidate the cache of recently finished pairings. - This is so that, if a new pairing is started with the diff --git a/doc/design/assistant/pairing.mdwn b/doc/design/assistant/pairing.mdwn index 388010ca8..fb4f4dd8f 100644 --- a/doc/design/assistant/pairing.mdwn +++ b/doc/design/assistant/pairing.mdwn @@ -3,12 +3,23 @@ have some way of pairing devices. ## security +Pairing uses its own network protocol, built on top of multicast UDP. + It's important that pairing securely verifies that the right host is being paired with. This is accomplied by having a shared secret be entered on -both the hosts that will be paired. They can then construct messages that -the other host can verify using the shared secret, and so know that, -for example, the ssh public key it received belongs to the right host -and has not been altered by a man in the middle. +both the hosts that will be paired. Hopefully that secret is communicated +securely out of band. + +(In practice, the security of that communication will vary. To guard against +interception, each pairing session pairs exactly two hosts and then forgets +the shared secret. So an attacker who tries to reuse an intercepted secret +will not succeed in pairing. This does not guard against an intercepted +secret that is used before the legitimate parties finish pairing.) + +Each host can construct messages that the other host can verify using the +shared secret, and so know that, for example, the ssh public key it +received belongs to the right host and has not been altered by a man in the +middle. The verification works like this: Take a HMAC SHA1 checksum of the message, using the shared secret as the HMAC key. Include this checksum after the @@ -44,6 +55,18 @@ by replaying these: So replay attacks don't seem to be a problem. +So far I've considered security from a third-party attacker, but either the +first or second parties in pairing could also be attackers. Presumably they +trust each other with access to their files as mediated by +[[git-annex-shell]]. However, one could try to get shell access to the +other's computer by sending malicious data in a pairing message. So the +pairing code always checks every data field's content, for example the ssh +public key is rejected if it looks at all unusual. Any control characters +in the pairing message cause it to be rejected, to guard against console +poisoning attacks. Furthermore, git-annex is careful not to expose data to +the shell, and the webapp uses Yesod's type safety to ensure all user input +is escaped before going to the browser. + ## TODO * pairing over IPV6 only networks does not work. Haskell's |