search

iboboc / android-notifier

Automatically exported from code.google.com/p/android-notifier
0 stars 0 forks source link

Encryption #24

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Some form of encryption for Wifi broadcast

Original issue reported on code.google.com by tyan...@gmail.com on 5 Jun 2010 at 6:05

GoogleCodeExporter commented 9 years ago 
Reasonable, and something I already considered.

THe hard part here is deciding how strong the encryption needs to be - it can 
vary anywhere from just cryptographic signatures using a shared passkey (which 
only avoids spoofed messages) up to full-blown 3DES or RSA with asymmetric keys 
which would also avoid interception of the messages for privacy reasons, but be 
much harder for the user to configure (and to implement).

What level of security are you trying to achieve?

Original comment by rdamazio@gmail.com on 23 Aug 2010 at 5:03

GoogleCodeExporter commented 9 years ago 
i think a passkey with AES sld be fine.. the purpose is to ensure privacy not 
authentication.

Original comment by tyan...@gmail.com on 23 Aug 2010 at 10:06

GoogleCodeExporter commented 9 years ago 
Sounds good.
For now I'll force it to use the same encryption key for all devices, if you 
have multiple.
In the mac version, I'll save the passkey to the default system keychain.

Original comment by rdamazio@gmail.com on 23 Aug 2010 at 2:38

GoogleCodeExporter commented 9 years ago 
Issue 62 has been merged into this issue.

Original comment by rdamazio@gmail.com on 9 Sep 2010 at 12:45

GoogleCodeExporter commented 9 years ago 
Olá Rodrigo!

I think this is a good encryption system:

"I'll be adding encryption support soon, so you'll be able to input a pass 
phrase in both the phone and the computer, and anyone who doesn't know the 
passphrase won't be able to read your notifications or send you fake ones".

Cheers!

Original comment by tsioangyo on 9 Sep 2010 at 2:13

GoogleCodeExporter commented 9 years ago 
Issue 64 has been merged into this issue.

Original comment by rdamazio@gmail.com on 9 Sep 2010 at 5:57

GoogleCodeExporter commented 9 years ago 
The lack of encryption is what's stopping me from actually using this app.
Since wifi and bluetooth are huge battery drainers the only practical option 
for me is sending the notifications over 3g (using a target IP).

On a side-note, would it be possible to allow multiple custom target IP 
addresses (by entering comma-separated IP-addresses) instead of just one?

Original comment by folker...@gmail.com on 10 Sep 2010 at 1:17

GoogleCodeExporter commented 9 years ago 
Yes, multiple IPs will be supported in the next version :)

Original comment by rdamazio@gmail.com on 10 Sep 2010 at 3:29

GoogleCodeExporter commented 9 years ago

Original comment by rdamazio@gmail.com on 19 Sep 2010 at 6:48

GoogleCodeExporter commented 9 years ago 
Done on the Android side in r261 and r262.
Now adding to the Mac native app.

Leandro, can you also look into adding this to the multi-platform client? (you 
can probably use the very same class Encryption that I created in r262, just 
need a UI for entering the pass phrase, and a way to safely store it - e.g. 
using appropriate keychains in each OS)

Original comment by rdamazio@gmail.com on 19 Sep 2010 at 9:13

GoogleCodeExporter commented 9 years ago 
Any reason why you chose Triple DES instead of AES encryption? AES is much much 
stronger.

Original comment by kkotowicz on 19 Sep 2010 at 9:50

GoogleCodeExporter commented 9 years ago 
I'm not sure (I don't know Java's implementation of Triple DES), but you're 
padding the given key with zeroes (arraycopy) to 24 bytes. 3DES algorithm uses 
three different 56-bit keys to encrypt & decrypt & encrypt again. 

I'm not sure at the actual Java implementation, but I think it just splits the 
given 24 bytes into three keys required by 3DES algorithm. By padding right 
with zeroes, you're most likely effectively setting 2nd and 3rd key to 0 for 
keys shorter than 8 bytes, and that surely weakens the cryptographic security. 
I'd recommend changing  the padKeyToLength() method to split the zeroes evenly 
among the keys.

Original comment by kkotowicz on 19 Sep 2010 at 11:01

GoogleCodeExporter commented 9 years ago 
I checked the SUN Java implementation and it looks as I'm right - if the 
DES-EDE key is filled with zeroes from 8th byte, DES-EDE is simply reduced to 
DES with the first 8 bytes as the encryption key. 

I included the modified padding function - what do you think?

http://code.google.com/p/android-notifier/source/browse/trunk/AndroidNotifier/sr
c/org/damazio/notifier/notification/Encryption.java?spec=svn262&r=262#34

Original comment by kkotowicz on 20 Sep 2010 at 12:26

GoogleCodeExporter commented 9 years ago 
About AES vs 3DES - there's an ongoing debate about possible attacks against 
AES, so it's still arguable which one is stronger:
http://eprint.iacr.org/2010/337.pdf

About the key padding implementation - you're right, adding zeroes is not a 
good way, but adding bytes in any other deterministic way (and it has to be 
deterministic for the other side to reproduce) also weakens the security. I 
could of course either force the user to  use 24 bytes or have him  use a 
generated key file, but I think that'd be suboptimal in terms of the 
usability/security compromise I'm looking for (if you exchange SMS with 
military secrets, you shouldn't install this app in the first place :) ). That 
said, I'll try to think of a better solution before releasing anything - e.g. a 
Tiger hash of the pass phrase.

The Java impl used in Android is Bouncy Castle btw - 
http://www.bouncycastle.org/ for the source.

Changed payload padding in r263, added mac support in r264 and r266.

Original comment by rdamazio@gmail.com on 20 Sep 2010 at 7:30

GoogleCodeExporter commented 9 years ago 
AES vs 3DES - I still think AES is stronger (of course there are various known 
attacks on both), but I think we agree that 3DES (properly implemented) is 
secure enough. I was just curious why you chose it if you possibly had AES 
available (I don't if AES is 
available on the platform though). But that's offtopic. 3DES is ok.

I still have some notes regarding current trunk (r268). There are still some 
issues:

- padding the key with zeroes is very dangerous in 3DES (it's Java 
implementation). As I said, it effectively reduces it to DES and I've checked 
it. Padding with anything else is better. Basically the key consists of 3*8 
bytes subkeys, and it's crucial that k1 != k2 != k3.

- hash from a user-supplied key given would be substantially better than 
padding deterministically, as you said.

- In Java's impl the least significant byte of DES/3DES key are just parity 
bits and they are ignored in encrypting/decrypting. So "\x01\xFE..." key 
produces the same results as "\x00\xFF...", so you're again losing the cipher 
strength. You should expand the key like in 
http://www.exampledepot.com/egs/javax.crypto/MakeDes.html (replace 8 bytes with 
24 and 56 bits with 168 in the code) - or use a hash ;)

- IV with zero values is a very bad idea, as it just gives away the first block 
of ciphertext in CBC mode - you're losing the protection of CBC mode (a xor 0 = 
a). http://en.wikipedia.org/wiki/Initialization_vector#Block_Ciphers. Ideally 
the IV random,unique and agreed beforehand, but I think we could be safe with 
yet another hash of the passkey. 

So, basically, use a hash for the key and for the IV.

Original comment by kkotowicz on 20 Sep 2010 at 10:05

GoogleCodeExporter commented 9 years ago 
Yup, I know.

Original comment by rdamazio@gmail.com on 20 Sep 2010 at 1:13

GoogleCodeExporter commented 9 years ago 
Please see if r269 makes you happy :)
This should be done on my side, marking as fixed.

Original comment by rdamazio@gmail.com on 20 Sep 2010 at 2:04

GoogleCodeExporter commented 9 years ago 
At first glance, looks OK to me :) We're probably good to go with this.

Original comment by kkotowicz on 20 Sep 2010 at 2:09

GoogleCodeExporter commented 9 years ago 
And thanks for considering my comments (and doing it so quickly) :)

Original comment by kkotowicz on 20 Sep 2010 at 2:11

GoogleCodeExporter commented 9 years ago 
It seems the password is stored in plain-text in preferences storage, I think 
only the password digest should be saved, otherwise I will not be able to use 
the password I use for everything :), you might want to take a look at this: 
http://www.jasypt.org/howtoencryptuserpasswords.html

Original comment by lehph...@gmail.com on 20 Sep 2010 at 3:22

GoogleCodeExporter commented 9 years ago 
Thank you for making such insightful comments - cryptography is not my 
specialty.

lephyro - there are a few considerations on top of this:
- If anyone obtains the password hash, they can still use it directly to both 
spoof messages and intercept them (you're still storing the key) - the link you 
pointed to doesn't apply as a way to store encryption keys, only to store 
passwords (the kind you type in and has to match) - in other words, I can't 
store the key in any irreversible way, as I need its original form to do the 
encryption
- If I store just the hash, the user can't edit his current password, just set 
a new one (which is ok, but probably involves making a customized 
DialogPreference for entering it).

The only solution I can think of that makes it slightly harder to use the 
stored key is to encrypt it with the device id - still, the app is open source 
and anyone that can run code that obtains the password can also run code to 
generate the same device id and decrypt it.

The other solution is to abuse the account mangement system to add credentials 
for this - this ensures some level of security (assuming a non-rooted phone), 
but any other app could still request to use the credentials.

Any other proposed solutions?
If not, I'm keeping it as is, assuming that in a well-behaved phone, only the 
app itself can access its preferences.

Original comment by rdamazio@gmail.com on 20 Sep 2010 at 6:28

GoogleCodeExporter commented 9 years ago 
We can't assume the phone is not rooted.

There is no need to use the plain text password to encrypt messages, we can use 
the hash of it as well, preferably hashed many times (>1000 as the linked 
article suggests). The Growl Network Transport Protocol, for instance, uses the 
hash of the key to encrypt messages.

I think the user will set the password once and forget, they will not mind 
entering it again if they need to edit it.

Original comment by lehph...@gmail.com on 20 Sep 2010 at 7:26

GoogleCodeExporter commented 9 years ago 
We're already using a hashed version of the pass phrase to encrypt it (see the 
code). Hashing many times doesn't give us any additional protection against 
someone stealing the key though - if we store what we use as the encryption 
key, no matter how many times it was hashed, anyone can reuse it.

Also, even on rooted phones, it's only possible to obtain the key by either 
explicitly requesting root permission to the user, or using adb (and in both 
cases the user is taking the risk by allowing/enabling it).

My point is just that unless we can make it very hard to obtain the key, it's 
not worth bothering trying to protect it with silly measures like encrypting 
with a reproducible key. The real solution would be to use RSA, but I would 
really, really not like to have to implement key exchange with the desktop as 
it would involve changing the protocol rather than just wrapping it.

Original comment by rdamazio@gmail.com on 20 Sep 2010 at 8:06

GoogleCodeExporter commented 9 years ago 
> anyone can reuse it
My point is protecting the password stored in preferences, message encryption 
is OK with the current implementation.

> user is taking the risk by allowing/enabling it
We can't trust the user to protect himself alone, let's give him a little help 
:)

Original comment by lehph...@gmail.com on 20 Sep 2010 at 8:23

GoogleCodeExporter commented 9 years ago 
Ok, I've added multiple hashing and keeping the hashed password in r296.

Original comment by rdamazio@gmail.com on 26 Sep 2010 at 4:09

GoogleCodeExporter commented 9 years ago 
Fixed a minor bug from r296 in r304.

Original comment by rdamazio@gmail.com on 27 Sep 2010 at 6:11

GoogleCodeExporter commented 9 years ago 
Committed changes to desktop client in r316.

Original comment by lehph...@gmail.com on 27 Sep 2010 at 7:08

GoogleCodeExporter commented 9 years ago 
When will support be added in the multi platform client or is there anoter bug 
report/feature request for that?

Original comment by brendan.lefoll on 30 Sep 2010 at 8:20

GoogleCodeExporter commented 9 years ago 
See comment 27 - it has already been added, just not released yet.

Original comment by rdamazio@gmail.com on 30 Sep 2010 at 12:20