[INTERNAL][CORE] Adds Socks5 proxy connection encryption #1069
Conversation
This patch will now encrypt non mediated Socks 5 connection because the SMACK API do not offer it. The encryption is currently done with RSA and RC4 although RC4 should be replaced. This patch includes some randomization that makes sniffing RC4 sessions harder.
|
Complexity increasing per file
==============================
- core/test/junit/saros/net/stream/SecureByteStreamTest.java 4
- core/src/saros/net/stream/SecureByteStream.java 4
See the complete overview on Codacy |
| final DataOutputStream encryptedOutputStream = new DataOutputStream(rc4CipherOutputStream); | ||
| random.nextBytes(garbageData); | ||
|
|
||
| encryptedOutputStream.writeInt(garbageDataSize); |
There was a problem hiding this comment.
Missing the local inputstream and the remote outputstream.
| @@ -401,7 +401,7 @@ private IByteStreamConnection acceptNewRequest(BytestreamRequest request) | |||
| localAddress, | |||
| new JID(peer), | |||
| connectionIdentifier, | |||
| new XMPPByteStreamAdapter(inSession), | |||
| SecureByteStream.wrap(new XMPPByteStreamAdapter(inSession), false), | |||
There was a problem hiding this comment.
Catch IO exception, close the Stream and then rethrow the exception.
|
As I am not well versed with the network stuff and he already had a look a it earlier on, I would like @stefaus to have a look at it. But, as he is currently a little preoccupy, this might take a while. |
|
There is no XEP for encryption regarding Socks5 or Jingle yet. So there is no support for this even in the newest Smack version. |
srossbach
added
Area: Core
|
|
||
| // send garbage to avoid RC4 attacks/sniffing | ||
|
|
||
| final Random random = new Random(); |
There was a problem hiding this comment.
Random is just a pseudo random generator and not suitable here.
There was a problem hiding this comment.
And the fix would be ?
There was a problem hiding this comment.
Why do you marked this as resolved?
A fix would be to use a cryptographically strong random number generator like SecureRandom
There was a problem hiding this comment.
You suggested to not use RC4 and so the whole logic can be discarded.
|
|
||
| final SecretKey rc4SecretKey = new SecretKeySpec(rc4KeyData, "RC4"); | ||
|
|
||
| final Cipher rc4CipherOut = Cipher.getInstance("RC4"); |
There was a problem hiding this comment.
RC4 is a broken Cipher and shouldn't be used, see https://tools.ietf.org/html/rfc7465
|
|
||
| // send garbage to avoid RC4 attacks/sniffing | ||
|
|
||
| final Random random = new Random(); |
There was a problem hiding this comment.
And the fix would be ?
There was a problem hiding this comment.
The man in the middle attack could also be done through a compromised XMPP server. I do not see any reason to transmit the keys through another (unsafe) channel.
It makes a difference.
- Compromising a XMPP-Server is way more difficult.
- Transmission to the XMPP-Server is encrypted and authenticated against a publickey signed by a "trusted" authority / manually validated.
- Even if the XMPP-Server is compromised, this byte stream is not passing the XMPP-Server. The Attacker has to control the Server and a network on the direct transmission path. This is not the basic public wifi scenario.
Depending on how everything works out, I still plan to do a more comprehensive security approach as part of my master thesis.
|
|
||
| // send garbage to avoid RC4 attacks/sniffing | ||
|
|
||
| final Random random = new Random(); |
There was a problem hiding this comment.
Why do you marked this as resolved?
A fix would be to use a cryptographically strong random number generator like SecureRandom
This patch will now encrypt non mediated Socks 5 connection because the
SMACK API do not offer it.
The encryption is currently done with RSA and RC4 although RC4 should be
replaced. This patch includes some randomization that makes sniffing RC4
sessions harder.