Code Review lulzlabs Radio AirChat - By kritik

[mofosyne]

Kritik the code reviewer concluded that the current code as it stands security wise "is completely useless". Adding that "If the author(s) fix at least the hardcoded key and random number generation issue, then the tool could have a future."

tl;dr: Messy code. Not enough comments. Ignorance of perl software distribution standards. Inconsistent style and programming paradigms. Insecure.

Security tldr: Encrypts "randomly generated ephemeral key using RSA", but ignores and use instead the "hardcoded key for symmetric encryption".

SOURCE http://www.daemon.de/blog/2014/04/25/351/code-review-lulzlabs-radio-airchat/

[lulzlabs]

from original module Crypt::CBC (Crypt::CBC is widely installed by default on many *nices.)

---

my $cipher_object_provided = $options->{cipher} && ref $options->{cipher};

# "key" is a misnomer here, because it is actually usually a passphrase that is used
# to derive the true key
my $pass = $options->{key};

if ($cipher_object_provided) {
  carp "Both a key and a pre-initialized Crypt::* object were passed. The key will be ignored"
if defined $pass;
  $pass ||= '';
}
elsif (!defined $pass) {
  croak "Please provide an encryption/decryption passphrase or key using -key"
}

---

we pinged Dr.Stein about CBC, (also asking to include other digest algos rather than md5 for the passphrase based key derivation). so basically we are waiting for the module to get updated before re-adjusting the code. there's a temporary hardcoded key on airchat required to initialized the cipher and that will immediately overwritten by they key derived from the passphrase. the hardcoded passphrase on the code is mostly a placeholder (instead of leaving a blank) and it's easy to change it at the Settings page.

Note: There's no bool types in Perl. you can define or undef variables and use that. so we used strings for fun, so It's useful to acknowledge who is really paying attention or not to the code.

comment aside: check always how the randomness of your own system performs. that would be a critical part of the whole thing.

love you all guise <3333 please help with whatever idea you got, we are brainstorming. and up-to-date we are on the 'free communications' experimenting stage dont be so serious...

[lulzlabs]

also about that blog. they are wrong sometimes. for example:

"When looking at the code, it looks like it's using RSA public key crypto to exchange keys with peers (via radio?). However, it's not using it at all." "encrypts a randomly generated ephemeral key using RSA but then ignores it and uses the above hardcoded key for symmetric encryption." wrong.

---

$dahpassiez = Crypt::CBC->random_bytes('4096');
$dahpassiez = sha256_base64($dahpassiez,"");    
[...]
my $cpassie = $public_rsa->encrypt($dahpassiez);
[...]
      $cipher->{'passphrase'} = $dahpassiez;
      eval{$ctx = $cipher->encrypt($txpack)};
[...]   
my $readypack = $cpassie .'</Key>'. $useThisk . '</reply>' . $ctx ;

---

this is how it happens:

you decide to encrypt a message using the public key of remote target. a random passphrase is generated and used to encrypt the message. (overwritten whatever supposedly hardcoded key....lol) this also encrypts the address of origin (aka just a fuzzy few bytes from a hash of your public key) then this passphrase is encrypted using the RSA public key of your target. you send the pack.

in the other side: you receive the pack. you extract the encrypted passphrase, encrypted message and encrypted address. you decrypt the encrypted passphrase with your private RSA key. you decrypt the message and the sender address using the decrypted passphrase.

"Another problem is, that all key material is being stored on disk without permission setting or checking. So the script will use the current umask, which would be 0644 for most users, therefore leaving the keys open to fetch for other users of the system." Wrong. check again. rights are 0600. working on nices. and even if so, we are thinking a better approach at this.

"Despite being a tool for encrypted radio communication the script contains code for Twitter publishing (with hardcoded API keys but they can be changed though), it fetches RSS feeds from various websites (hardcoded URIs of course), e.g. from NY Times. But it supports using a TOR proxy after all."

it helps to get some basic news from internet if someone in the network can get some basic internet axx and decide to share it... it's on the README.... ...

"The script uses both whitespace and tabs for indentation. As a result the overall code looks messy" it was written in some some chaotic piratenpad... lelelele

It's good if ppl quickly check the code. it would be better if they really test it little bit at least also....

but yeah discussing which one should be the definitive random generator is really cool.

[PowerPress]

Has the code been updated to address these security concerns?

[TLINDEN]

Hi there,

I updated the blog entry about the hardcoded key issue. Crypt::CBC indeed doesn't use the key parameter if you modify the passphrase parameter afterwards. This might not be good style but it works (I tested it).

So, I hereby appologize for my invalid conclusions about security.

best, Tom