facebookarchive / RakNet

RakNet is a cross platform, open source, C++ networking engine for game programmers.
Other
3.31k stars 1.02k forks source link

Android secure connection crash. #89

Open kochol opened 8 years ago

kochol commented 8 years ago

Hi I use raknet secure connections in my game but it crash in android here is logcat log.

F/libc (18590): Fatal signal 7 (SIGBUS), code 1, fault addr 0x96d62c87 in tid 18665 (Thread-5992) W/libc (18590): Security Level: (1), Debug inforamtion is controlled by the DUMPABLE flag. I/DEBUG ( 446): * * * * * * * * * * * * * * * * I/DEBUG ( 446): Build fingerprint: 'htc/htc_asia_tw/htc_eyetuhl:5.0.2/LRX22G/653658.2:user/release-keys' I/DEBUG ( 446): Revision: '0' I/DEBUG ( 446): ABI: 'arm' I/DEBUG ( 446): pid: 18590, tid: 18665, name: Thread-5992 >>> com.plangtongs.skyheroes <<< I/DEBUG ( 446): signal 7 (SIGBUS), code 1 (BUSADRALN), fault addr 0x96d62c87 I/DEBUG ( 446): r0 95213588 r1 96d62c87 r2 00000008 r3 00000000 I/DEBUG ( 446): r4 00000000 r5 00000000 r6 995ef788 r7 95213588 I/DEBUG ( 446): r8 96d62c87 r9 952136c0 sl 96d62c7f fp 00000001 I/DEBUG ( 446): ip 95213580 sp 95213514 lr 9ac016b3 pc 9abff36e cpsr 800f0030 I/DEBUG ( 446): I/DEBUG ( 446): backtrace: I/DEBUG ( 446): #00 pc 0144336e /data/app/com.plangtongs.skyheroes-1/lib/arm/libUE4.so (cat::SecureEqual(void const, void const, int)+27) I/DEBUG ( 446): #01 pc 014456af /data/app/com.plangtongs.skyheroes-1/lib/arm/libUE4.so (cat::AuthenticatedEncryption::Decrypt(unsigned char, unsigned int&)+282) I/DEBUG ( 446): #02 pc 0143e461 /data/app/com.plangtongs.skyheroes-1/lib/arm/libUE4.so (RakNet::ReliabilityLayer::HandleSocketReceiveFromConnectedPlayer(char const, unsigned int, RakNet::SystemAddress&, DataStructures::ListRakNet::PluginInterface2*&, int, RakNet::RakNetSocket2, RakNet::RakNetRandom, unsigned long long, RakNet::BitStream&)+184) I/DEBUG ( 446): #03 pc 01434237 /data/app/com.plangtongs.skyheroes-1/lib/arm/libUE4.so (RakNet::ProcessNetworkPacket(RakNet::SystemAddress, char const, int, RakNet::RakPeer, RakNet::RakNetSocket2, unsigned long long, RakNet::BitStream&)+150) I/DEBUG ( 446): #04 pc 0143648f /data/app/com.plangtongs.skyheroes-1/lib/arm/libUE4.so (RakNet::RakPeer::RunUpdateCycle(RakNet::BitStream&)+98) I/DEBUG ( 446): #05 pc 0142edb3 /data/app/com.plangtongs.skyheroes-1/lib/arm/libUE4.so (RakNet::UpdateNetworkLoop(void_)+62) I/DEBUG ( 446): #06 pc 00012f93 /system/lib/libc.so (__pthread_start(void*)+30) I/DEBUG ( 446): #07 pc 00011057 /system/lib/libc.so (__start_thread+6)

larku commented 8 years ago

I've recently encountered the exact same issue.

I've replaced the offending method cat::SecureEqual with the implementation below - note this may not be as secure so use at your own risk..

Note: the secureEquals method is used to eliminate timing attacks ( https://www.google.com.au/webhp?sourceid=chrome-instant&ion=1&espv=2&ie=UTF-8#q=secure%20equals%20timing%20attacks ) as such please asses if this is appropriate for your purposes.

bool SecureEqual(const void *vA, const void *vB, int bytes)
{
      size_t i;
      const unsigned char *a = (const unsigned char *)vA;
      const unsigned char *b = (const unsigned char *)vB;
      unsigned char x = 0;

      for (i = 0; i < bytes; i++)
          x |= a[i] ^ b[i];

      return x == 0;
}

This will be added to my larku fork sometime over the next two/three weeks. This will become the default implementation for Android unless someone has a fix for the current implementation (I don't have time to analyse why the current implementation fails).

More info: As I understand this new implementation can be optimised by the compiler and possibly not be as secure (with respect to timing attacks). The idea is to always compare the entire set of bytes and not stop once a difference is found (can indicate based on time where in the bytes the comparison failed which leads to a way to discover a match).

Another method you could replace this with (which is likely more costly) is to do something simple like this pseudo code:

bool SecureEqual(const void *vA, const void *vB, int bytes)
{
      return sha256(vA) == sha256(vB); 
}

Since the bytes actually being compared will be completely different for every input value, timing attacks do not give any useful information (so I understand).

I'm not a security expert so please take my advice with this in mind.

Let me know if this fixes things for you also.

kochol commented 8 years ago

Thank you for your quick and complete answer.

I write only return true and it works. It seems this code is not important in client side am I right?

larku commented 8 years ago

Hi @kochol,

SecureEqual is called (indirectly) in many places, it's used in KeyAgreementInitiator and KeyAgreementResponder and AuthenticatedEncryption The AuthenticatedEncryption is used as part of the SecurityLayer which is the heart of the encryption layer.

I'd suggest that returning true here likely makes things very insecure (if not totally insecure).

You'd be far better off using this replacement for SecureEqual()

bool SecureEqual(const void *vA, const void *vB, int bytes)
{
      size_t i;
      const unsigned char *a = (const unsigned char *)vA;
      const unsigned char *b = (const unsigned char *)vB;
      unsigned char x = 0;

      for (i = 0; i < bytes; i++)
          x |= a[i] ^ b[i];

      return x == 0;
}

I believe that would be a far safer option. It's tested and working (in terms of returning correct results) and honours the contract of the original implementation (only returns true when equal).

kochol commented 8 years ago

Hi @larku

What about the code here. Is this code good? https://github.com/catid/libcat/blob/master/SecureEqual.cpp

larku commented 8 years ago

That seems to be identical to the original implementation bundled with RakNet (except for that final line before the return statement).

I'd assume this will fail exactly like the original implementation.

The implementation I suggested you use is reasonably common in crypto libraries so I'd assume it's likely an ok candidate (see here: https://cryptocoding.net/index.php/Coding_rules ).

Do you have a reason to avoid the suggested replacement?

kochol commented 8 years ago

@larku Thank you for your help. No I don't have any replacement but I enjoy chatting with you :D I used your code and it works.

larku commented 8 years ago

Hi @kochol,

Excellent :)