Closed GoogleCodeExporter closed 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
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
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
Issue 62 has been merged into this issue.
Original comment by rdamazio@gmail.com
on 9 Sep 2010 at 12:45
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
Issue 64 has been merged into this issue.
Original comment by rdamazio@gmail.com
on 9 Sep 2010 at 5:57
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
Yes, multiple IPs will be supported in the next version :)
Original comment by rdamazio@gmail.com
on 10 Sep 2010 at 3:29
Original comment by rdamazio@gmail.com
on 19 Sep 2010 at 6:48
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
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
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
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
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
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
Yup, I know.
Original comment by rdamazio@gmail.com
on 20 Sep 2010 at 1:13
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
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
And thanks for considering my comments (and doing it so quickly) :)
Original comment by kkotowicz
on 20 Sep 2010 at 2:11
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
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
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
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
> 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
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
Fixed a minor bug from r296 in r304.
Original comment by rdamazio@gmail.com
on 27 Sep 2010 at 6:11
Committed changes to desktop client in r316.
Original comment by lehph...@gmail.com
on 27 Sep 2010 at 7:08
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
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
Original issue reported on code.google.com by
tyan...@gmail.com
on 5 Jun 2010 at 6:05