From cddff787196bb47edaa1b88f549eec73470039ff Mon Sep 17 00:00:00 2001
From: Evgeny Poberezkin <2769109+epoberezkin@users.noreply.github.com>
Date: Sat, 24 Apr 2021 12:46:57 +0100
Subject: [PATCH] binary X509 encoding for RSA key send during transport
 handshake (#105)

---
 apps/smp-server/Main.hs            |  2 +-
 rfcs/2021-01-26-crypto.md          |  7 ++-
 src/Simplex/Messaging/Crypto.hs    | 37 ++++++++++------
 src/Simplex/Messaging/Transport.hs | 68 +++++++++++++++++++++++-------
 src/Simplex/Messaging/Util.hs      |  6 +--
 tests/SMPClient.hs                 |  4 +-
 6 files changed, 89 insertions(+), 35 deletions(-)

diff --git a/apps/smp-server/Main.hs b/apps/smp-server/Main.hs
index 47cecf056..ee7801166 100644
--- a/apps/smp-server/Main.hs
+++ b/apps/smp-server/Main.hs
@@ -71,4 +71,4 @@ readCreateKey = do
         errorExit e = putStrLn (e <> ": " <> path) >> exitFailure
 
 publicKeyHash :: C.PublicKey -> B.ByteString
-publicKeyHash = C.serializeKeyHash . C.getKeyHash . C.serializePubKey
+publicKeyHash = C.serializeKeyHash . C.getKeyHash . C.binaryEncodePubKey
diff --git a/rfcs/2021-01-26-crypto.md b/rfcs/2021-01-26-crypto.md
index c7f5f2c9d..578a86c7f 100644
--- a/rfcs/2021-01-26-crypto.md
+++ b/rfcs/2021-01-26-crypto.md
@@ -22,7 +22,7 @@ To establish the session keys and base IVs, the server should have an asymmetric
 
 The handshake sequence is the following:
 
-1. Once the connection is established, the server sends its public 2048 bit key to the client. TODO currently the key will be sent as a line terminated with CRLF, using ad-hoc key serialization we use.
+1. Once the connection is established, the server sends transport_header and its public RSA key encoded in X509 binary format to the client.
 2. The client compares the hash of the received key with the hash it already has (e.g. received as part of connection invitation or server in NEW command). If the hash does not match, the client must terminate the connection. TODO as the hash is optional in server syntax at the moment, hash comparison will be optional as well. Probably it should become required.
 3. If the hash is the same, the client should generate random symmetric AES keys and base IVs that will be used as session keys/IVs by the client and the server.
 4. The client then should encrypt these symmetric keys and base IVs with the public key that the server sent, and send to the server the result of the encryption: `rsa-encrypt(snd-aes-key, snd-base-iv, rcv-aes-key, rcv-base-iv)`. `snd-aes-key` and `snd-base-iv` will be used by the client to encrypt **sent** messages and by the server to decrypt them, `rcv-aes-key` and `rcv-base-iv` will be used by the client to decrypt **received** messages and by the server to encrypt them.
@@ -34,6 +34,11 @@ All the subsequent data both from the client and from the server should be sent
 Each transport block sent by the client and the server has this syntax:
 
 ```abnf
+transport_header = block_size protocol key_size
+block_size = 4*4(OCTET) ; 4-byte block size sent by the server, currently the client rejects if > 65536 bytes
+protocol = 2*2(OCTET) ; currently it is 0, that means binary RSA key
+key_size = 2*2(OCTET) ; the encoded key size in bytes (binary encoded in X509 standard)
+
 transport_block = aes_body_auth_tag aes_encrypted_body ; fixed at 8192 bits
 aes_encrypted_body = 1*OCTET
 aes_body_auth_tag = 16*16(OCTET)
diff --git a/src/Simplex/Messaging/Crypto.hs b/src/Simplex/Messaging/Crypto.hs
index 766c46632..ce92763e0 100644
--- a/src/Simplex/Messaging/Crypto.hs
+++ b/src/Simplex/Messaging/Crypto.hs
@@ -33,10 +33,12 @@ module Simplex.Messaging.Crypto
     decryptAES,
     serializePrivKey,
     serializePubKey,
+    binaryEncodePubKey,
     serializeKeyHash,
     getKeyHash,
     privKeyP,
     pubKeyP,
+    binaryPubKeyP,
     keyHashP,
     authTagSize,
     authTagToBS,
@@ -179,8 +181,8 @@ generateKeyPair size = loop
         then loop
         else pure (PublicKey k, mkPrivateKey pk)
 
-rsaPrivateSize :: PrivateKey k => k -> Int
-rsaPrivateSize = R.public_size . R.private_pub . rsaPrivateKey
+privateKeySize :: PrivateKey k => k -> Int
+privateKeySize = R.public_size . R.private_pub . rsaPrivateKey
 
 publicKey :: FullPrivateKey -> PublicKey
 publicKey = PublicKey . R.private_pub . rsaPrivateKey
@@ -258,7 +260,7 @@ encrypt k paddedSize msg = do
 
 decrypt :: PrivateKey k => k -> ByteString -> ExceptT CryptoError IO ByteString
 decrypt pk msg'' = do
-  let (encHeader, msg') = B.splitAt (rsaPrivateSize pk) msg''
+  let (encHeader, msg') = B.splitAt (privateKeySize pk) msg''
   header <- decryptOAEP pk encHeader
   Header {aesKey, ivBytes, authTag, msgSize} <- except $ parseHeader header
   msg <- decryptAES aesKey ivBytes msg' authTag
@@ -342,6 +344,9 @@ serializePrivKey pk = "rsa:" <> encodePrivKey pk
 pubKeyP :: Parser PublicKey
 pubKeyP = keyP decodePubKey <|> legacyPubKeyP
 
+binaryPubKeyP :: Parser PublicKey
+binaryPubKeyP = either fail pure . binaryDecodePubKey =<< A.takeByteString
+
 privKeyP :: PrivateKey k => Parser k
 privKeyP = keyP decodePrivKey <|> legacyPrivKeyP
 
@@ -382,28 +387,34 @@ safeRsaPrivateKey (size, n, d) =
     }
 
 encodePubKey :: PublicKey -> ByteString
-encodePubKey = encodeKey . PubKeyRSA . rsaPublicKey
+encodePubKey = encode . binaryEncodePubKey
+
+binaryEncodePubKey :: PublicKey -> ByteString
+binaryEncodePubKey = binaryEncodeKey . PubKeyRSA . rsaPublicKey
 
 encodePrivKey :: PrivateKey k => k -> ByteString
-encodePrivKey = encodeKey . PrivKeyRSA . rsaPrivateKey
+encodePrivKey = encode . binaryEncodeKey . PrivKeyRSA . rsaPrivateKey
 
-encodeKey :: ASN1Object a => a -> ByteString
-encodeKey k = encode . toStrict . encodeASN1 DER $ toASN1 k []
+binaryEncodeKey :: ASN1Object a => a -> ByteString
+binaryEncodeKey k = toStrict . encodeASN1 DER $ toASN1 k []
 
 decodePubKey :: ByteString -> Either String PublicKey
-decodePubKey s =
-  decodeKey s >>= \case
+decodePubKey = binaryDecodePubKey <=< decode
+
+binaryDecodePubKey :: ByteString -> Either String PublicKey
+binaryDecodePubKey =
+  binaryDecodeKey >=> \case
     (PubKeyRSA k, []) -> Right $ PublicKey k
     r -> keyError r
 
 decodePrivKey :: PrivateKey k => ByteString -> Either String k
-decodePrivKey s =
-  decodeKey s >>= \case
+decodePrivKey =
+  decode >=> binaryDecodeKey >=> \case
     (PrivKeyRSA pk, []) -> Right $ mkPrivateKey pk
     r -> keyError r
 
-decodeKey :: ASN1Object a => ByteString -> Either String (a, [ASN1])
-decodeKey s = fromASN1 =<< first show . decodeASN1 DER . fromStrict =<< decode s
+binaryDecodeKey :: ASN1Object a => ByteString -> Either String (a, [ASN1])
+binaryDecodeKey = fromASN1 <=< first show . decodeASN1 DER . fromStrict
 
 keyError :: (a, [ASN1]) -> Either String b
 keyError = \case
diff --git a/src/Simplex/Messaging/Transport.hs b/src/Simplex/Messaging/Transport.hs
index d0468886e..7f18cc8ff 100644
--- a/src/Simplex/Messaging/Transport.hs
+++ b/src/Simplex/Messaging/Transport.hs
@@ -32,7 +32,7 @@ import GHC.IO.Exception (IOErrorType (..))
 import GHC.IO.Handle.Internals (ioe_EOF)
 import Generic.Random (genericArbitraryU)
 import Network.Socket
-import Network.Transport.Internal (encodeWord32)
+import Network.Transport.Internal (decodeNum16, decodeNum32, encodeEnum16, encodeEnum32, encodeWord32)
 import qualified Simplex.Messaging.Crypto as C
 import Simplex.Messaging.Parsers (parse, parseAll, parseRead1)
 import Simplex.Messaging.Util (bshow, liftError)
@@ -172,6 +172,7 @@ data HandshakeError
   | DECRYPT
   | VERSION
   | RSA_KEY
+  | HEADER
   | AES_KEYS
   | BAD_HASH
   | MAJOR_VERSION
@@ -233,15 +234,18 @@ makeNextIV SessionKey {baseIV, counter} = atomically $ do
 -- The numbers in function names refer to the steps in the document
 serverHandshake :: Handle -> C.FullKeyPair -> ExceptT TransportError IO THandle
 serverHandshake h (k, pk) = do
-  liftIO sendPublicKey_1
+  liftIO sendHeaderAndPublicKey_1
   encryptedKeys <- receiveEncryptedKeys_4
   HandshakeKeys {sndKey, rcvKey} <- decryptParseKeys_5 encryptedKeys
-  th <- liftIO $ transportHandle h rcvKey sndKey -- keys are swapped here
+  th <- liftIO $ transportHandle h rcvKey sndKey transportBlockSize -- keys are swapped here
   sendWelcome_6 th
   pure th
   where
-    sendPublicKey_1 :: IO ()
-    sendPublicKey_1 = putLn h $ C.serializePubKey k
+    sendHeaderAndPublicKey_1 :: IO ()
+    sendHeaderAndPublicKey_1 = do
+      let sKey = C.binaryEncodePubKey k
+          header = TransportHeader {blockSize = transportBlockSize, keySize = B.length sKey}
+      B.hPut h $ binaryTransportHeader header <> sKey
     receiveEncryptedKeys_4 :: ExceptT TransportError IO ByteString
     receiveEncryptedKeys_4 =
       liftIO (B.hGet h $ C.publicKeySize k) >>= \case
@@ -258,20 +262,25 @@ serverHandshake h (k, pk) = do
 -- The numbers in function names refer to the steps in the document
 clientHandshake :: Handle -> Maybe C.KeyHash -> ExceptT TransportError IO THandle
 clientHandshake h keyHash = do
-  k <- getPublicKey_1_2
+  (k, blkSize) <- getHeaderAndPublicKey_1_2
   keys@HandshakeKeys {sndKey, rcvKey} <- liftIO generateKeys_3
   sendEncryptedKeys_4 k keys
-  th <- liftIO $ transportHandle h sndKey rcvKey
+  th <- liftIO $ transportHandle h sndKey rcvKey blkSize
   getWelcome_6 th >>= checkVersion
   pure th
   where
-    getPublicKey_1_2 :: ExceptT TransportError IO C.PublicKey
-    getPublicKey_1_2 = do
-      s <- liftIO $ getLn h
+    getHeaderAndPublicKey_1_2 :: ExceptT TransportError IO (C.PublicKey, Int)
+    getHeaderAndPublicKey_1_2 = do
+      header <- liftIO (B.hGet h transportHeaderSize)
+      TransportHeader {blockSize, keySize} <- liftEither $ parse transportHeaderP (TEHandshake HEADER) header
+      when (blockSize < transportBlockSize || blockSize > maxTransportBlockSize) $
+        throwError $ TEHandshake HEADER
+      s <- liftIO $ B.hGet h keySize
       maybe (pure ()) (validateKeyHash_2 s) keyHash
-      liftEither $ parseKey s
+      key <- liftEither $ parseKey s
+      pure (key, blockSize)
     parseKey :: ByteString -> Either TransportError C.PublicKey
-    parseKey = first (const $ TEHandshake RSA_KEY) . parseAll C.pubKeyP
+    parseKey = first (const $ TEHandshake RSA_KEY) . parseAll C.binaryPubKeyP
     validateKeyHash_2 :: ByteString -> C.KeyHash -> ExceptT TransportError IO ()
     validateKeyHash_2 k kHash
       | C.getKeyHash k == kHash = pure ()
@@ -296,6 +305,35 @@ clientHandshake h keyHash = do
       when (major smpVersion > major currentSMPVersion) . throwE $
         TEHandshake MAJOR_VERSION
 
+data TransportHeader = TransportHeader {blockSize :: Int, keySize :: Int}
+  deriving (Eq, Show)
+
+binaryRsaTransport :: Int
+binaryRsaTransport = 0
+
+transportBlockSize :: Int
+transportBlockSize = 8192
+
+maxTransportBlockSize :: Int
+maxTransportBlockSize = 65536
+
+transportHeaderSize :: Int
+transportHeaderSize = 8
+
+binaryTransportHeader :: TransportHeader -> ByteString
+binaryTransportHeader TransportHeader {blockSize, keySize} =
+  encodeEnum32 blockSize <> encodeEnum16 binaryRsaTransport <> encodeEnum16 keySize
+
+transportHeaderP :: Parser TransportHeader
+transportHeaderP = TransportHeader <$> int32 <* binaryRsaTransportP <*> int16
+  where
+    int32 = decodeNum32 <$> A.take 4
+    int16 = decodeNum16 <$> A.take 2
+    binaryRsaTransportP = binaryRsa <$> int16
+    binaryRsa :: Int -> Parser Int
+    binaryRsa 0 = pure 0
+    binaryRsa _ = fail "unknown transport mode"
+
 serializeHandshakeKeys :: HandshakeKeys -> ByteString
 serializeHandshakeKeys HandshakeKeys {sndKey, rcvKey} =
   serializeKey sndKey <> serializeKey rcvKey
@@ -315,8 +353,8 @@ handshakeKeysP = HandshakeKeys <$> keyP <*> keyP
 parseHandshakeKeys :: ByteString -> Either TransportError HandshakeKeys
 parseHandshakeKeys = parse handshakeKeysP $ TEHandshake AES_KEYS
 
-transportHandle :: Handle -> SessionKey -> SessionKey -> IO THandle
-transportHandle h sk rk = do
+transportHandle :: Handle -> SessionKey -> SessionKey -> Int -> IO THandle
+transportHandle h sk rk blockSize = do
   sndCounter <- newTVarIO 0
   rcvCounter <- newTVarIO 0
   pure
@@ -324,5 +362,5 @@ transportHandle h sk rk = do
       { handle = h,
         sndKey = sk {counter = sndCounter},
         rcvKey = rk {counter = rcvCounter},
-        blockSize = 8192
+        blockSize
       }
diff --git a/src/Simplex/Messaging/Util.hs b/src/Simplex/Messaging/Util.hs
index e8397015d..b05e7ff45 100644
--- a/src/Simplex/Messaging/Util.hs
+++ b/src/Simplex/Messaging/Util.hs
@@ -39,11 +39,11 @@ infixl 4 <$$>
 bshow :: Show a => a -> ByteString
 bshow = B.pack . show
 
-liftIOEither :: (MonadUnliftIO m, MonadError e m) => IO (Either e a) -> m a
+liftIOEither :: (MonadIO m, MonadError e m) => IO (Either e a) -> m a
 liftIOEither a = liftIO a >>= liftEither
 
-liftError :: (MonadUnliftIO m, MonadError e' m) => (e -> e') -> ExceptT e IO a -> m a
+liftError :: (MonadIO m, MonadError e' m) => (e -> e') -> ExceptT e IO a -> m a
 liftError f = liftEitherError f . runExceptT
 
-liftEitherError :: (MonadUnliftIO m, MonadError e' m) => (e -> e') -> IO (Either e a) -> m a
+liftEitherError :: (MonadIO m, MonadError e' m) => (e -> e') -> IO (Either e a) -> m a
 liftEitherError f a = liftIOEither (first f <$> a)
diff --git a/tests/SMPClient.hs b/tests/SMPClient.hs
index c6e6af28e..360771df8 100644
--- a/tests/SMPClient.hs
+++ b/tests/SMPClient.hs
@@ -31,10 +31,10 @@ testPort :: ServiceName
 testPort = "5000"
 
 teshKeyHashStr :: B.ByteString
-teshKeyHashStr = "p1xa/XuzchgqomEL6RX+Me+fX096w50V7nJPAA0wpDE="
+teshKeyHashStr = "KXNE1m2E1m0lm92WGKet9CL6+lO742Vy5G6nsrkvgs8="
 
 teshKeyHash :: Maybe C.KeyHash
-teshKeyHash = Just "p1xa/XuzchgqomEL6RX+Me+fX096w50V7nJPAA0wpDE="
+teshKeyHash = Just "KXNE1m2E1m0lm92WGKet9CL6+lO742Vy5G6nsrkvgs8="
 
 testSMPClient :: MonadUnliftIO m => (THandle -> m a) -> m a
 testSMPClient client =