Code Review lulzlabs Radio AirChat

Yesterday someone announced AirChat on the cypherpunks mailinglist. He didn't provide any comment as to what it is or why he posted it. However I took a look anyway. I found some really odd piece of software and decided to publish a review about it. Please note, that I only looked at the code. I didn't install it and I didn't run it (I've got no radio device anyway). So, this review is not about usage, user interface or something, it's only about the code and the security implications from what I found there.

What is AirChat radio?

AirChat is a perl script and according to their README, it can be used to communicate encrypted via radio signals. No internet or mobile required, so they say.

Summary

The code in the script is a mess. One monolithic script which contains keys, configuration, html code, css code, almost no comments. It ignores all perl standards of perl software distribution (build, packaging, unit tests, documentation). The script has obviously been written by different authors (or the main author copy/pasted code from somewhere else) in different styles and paradigmas. It looks much like the old perl scripts of the late 1990s. AirChat is one of the worst security softwares I've ever seen. I would not encourage anyone using it.

Source Organization

Essentially, there's none. The software consists of one file (airchat.pl), which contains everything, like modules, html code, configuration, daemon code, ui code, hardware interfacing code and so forth.

AirChat completely ignores all perl standards about how to distribute perl software. There's no MakeMaker Makefile.PL, there are no unit tests and practically no documentation. The README file consists in large parts of prosa text, where the author(s) tell about their motives to create the software. Though it contains information how to install the software, there is no single hint about usage beside telling the user to connect with the browser to localhost to some port.

Coding Style

The script uses both whitespace and tabs for indentation. As a result the overall code looks messy, sometimes it's difficult to read it. Here's an example:

(the long underscored lines come from emacs syntax highlighting, these are whitespaces)

The script is also styled in different ways. I'd assume it have been written by different authors or the author used code from somewhere else.

The AirChat script uses some very - ahem - weird variable names. Here are some examples:

my $cock;
$settings->{'settings'}{'penis'}{'penis'} = 'also cocks' ;
my $penispenis = " ";
%dahfuckingkeys = %{$getem};
my @sortedshit = split("##END##",$pack);
$dahpassiez = Crypt::CBC->random_bytes('4096');

Obviously the author is fixated on male sexual organs. Another example of this are the modem_* functions. For example the function modem_setting() returns the literal string "penis" as do several others for no reason. Function names follow this pattern as well. Some examples: sendthefuckout(), resenditpl0x() or gogogodispatch().

Beside variables there are also gems like this:

 if ($mustEncrypt eq "yeahbabygetthisoneintoyourassFCCandNSA") {

Comments in the code are rare. When there is a comment, it comments the obvious in most cases or is just senseless. An example:

	## youz taking drugz again 

Because of missing comments and no code documentation in general it is difficult to follow the program flow in order to figure out, what the script does.

Perl

The perl code in airchat.pl is inefficient, hard to read, inconsistent and old styled. One good thing can be noted though: the author has enabled strict and warnings, and uses my (or our) keywords to avoid undefined behavior. In most cases he also checks for validity. However there's not much error handling in the script. That is, an action will be done if a variable X is set, but there's no code for the case when it isn't. That's not always the case, but can be found all over the script. Here's one example of poor error checking (and reporting):

	eval{$ctx = $cipher->encrypt($txpack)};
	eval{$useThisk = $cipher->encrypt($useThisk)};
	if ($@) {
	  print "error encrypting";
} else {
    [..]

Only the eval() error of the second encrypt call is being checked. If the first one fails gets ignored. It's also not clear why he's using eval() at all.

airchat.pl contains lots of global variables and some of them are scattered throughout the script. I counted 76 global variables, however, some of them are part of the module AirChatServer (contained within the script).

It is obvious that the author is inexperienced when it comes to perl data structures. Here's a snippet of the function load_settings():

    $proxyhost = $settings->{'settings'}{'Tor and Proxy'}{'proxyhost'}  if defined  $settings->{'settings'}{'Tor and Proxy'}{'proxyhost'}  ;
    $proxyport = $settings->{'settings'}{'Tor and Proxy'}{'proxyport'}  if defined  $settings->{'settings'}{'Tor and Proxy'}{'proxyport'}  ;
    $proxyuser = $settings->{'settings'}{'Tor and Proxy'}{'proxyuser'}  if defined  $settings->{'settings'}{'Tor and Proxy'}{'proxyuser'}  ;
    $proxypass = $settings->{'settings'}{'Tor and Proxy'}{'proxypass'}  if defined  $settings->{'settings'}{'Tor and Proxy'}{'proxypass'}  ;

That's a lot of repeated typing. In general, the author uses a global hash %settings to store configuration which is to be saved to a configuration file, but doesn't use that hash. Instead he assigns every single value in that hash to a single global variable. He does the reverse when saving it to the configuration file.

Despite the notion of Windows in the documentation, the script isn't portable, since it's using hardocded unix filesystem separators:

 open(F, '<', "$foolder/.AirChatsettings") or die "cannot open file settings";

Instead he should use the File::Spec perl module which does stuff like this in a portable way. There's also no check if the configuration directory, which is the script location, is writable (save_settings()). The same applies for saved messages (whatever kind of message it tries to save) and saved public keys.

The author is not very familiar with regular expressions as well. Let's take a look at this example:

if ($kxcode[1] =~ m/:CTALK2:[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/ ) {

He's essentially checking for a hex value, but he doesn't know of perl regex quantifiers, such as: [0-9a-f]{6}. Variations of the above regex can be found throughout the whole script. There's another issue which the above line shows us: hardcoded values. And there are lots of them. Things like "OHAITHERE" or "HOTELINDIA", sizes, protocol codes (I'll come to that later) and stuff like this. Here's an example of a hardcoded size value:

my $kidx = substr($kidx,0,6);

Here the author extracts the first 6 chars of a hexstring and uses it as a key id (see below). Key id extraction code like this is scattered overall in the script, everwhere using the hardcoded 6.

Speaking of hardcoded stuff. airchat.pl contains a webserver as well (it's being used to interface to the user, I think) and a couple of builtin CGI functions. Those functions use print() statements to put HTML out. Lots of them. The author seems to be not very familiar with HTML as well, eg:

 print qq {&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;<a href="/addKey" ><code>add a new key</code></a>};

There's even an embedded font file (base64 encoded). Maintaining that stuff must be very hard. Since the HTML and style is hardcoded in the script it can't be customized (if a user wanted to do that) or translated.

There are a lot more issues. I just covered a couple of them.

Security

The airchat.pl script is completely unsecure despite the repeated "fuck the NSA" references here and there in the script and the README. It's (trying to) use RSA and AES and Camilla Ciphers along with SHA2 for hashing. 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. The reason is, that every encryption key for symmetric encryption, which is being used to encrypt messages, uses hardcoded keys. Here are the keys I found:

•  'b0ssjP3n1s+d2y0l0+IATTsuxiOB1vz7 - being used in asym_encrypt(), asym_decrypt(), sendinghits(), gettingdecodedmsg()
•  'x3UstrV@Hl;Mm#G9#_q,suckXZ$O^;55jlT*' - used as passphrase with Crypt::CBC.
• there are twitter API keys in the script!

Basically the script encrypts a randomly generated ephemeral key using RSA but then ignores it and uses the above hardcoded key for symmetric encryption. SeeUpdate below!

Random bytes are always generated this way:

 $dahpassiez = Crypt::CBC->random_bytes('4096');

He should have been using Crypt::Random or Bytes::Random::Secure instead. From the Crypt::CBC docs:

$data = random_bytes($numbytes)

Return $numbytes worth of random data. On systems that support the "/dev/urandom" device file, this data will be read from the device. Otherwise, it will be generated by repeated calls to the Perl rand() function.

Beside those two major issues it generates key id's in a bad fashion:

 my $rsa = Crypt::OpenSSL::RSA->generate_key(2048);
 my $keyidx = $rsa->get_public_key_string() if defined $rsa;
 $keyidx = sha512_hex($keyidx,"");
 $keyidx = substr($keyidx,0,6);

Basically (as already mentioned earlier) he's using the first 6 characters of the hex encoded hash of the hex encoded RSA public key string. What could possibly go wrong?

Another issue with the above code is that it would generate a keyid from undef if RSA key generation fails for some reason. And it would store it anyway.

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.

There seems to be no key expire mechanism as well. Once generated a key will be used forever. At least in theory, since the RSA keys are not used anyway as mentioned above.

Other Oddities

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.

Conclusion

By just using a mobile phone the user can communicate more secure via radios signals. Since every encryption is being done with the same hardcoded key, it is completely useless. If the author(s) fix at least the hardcoded key and random number generation issue, then the tool could have a future. See Update below!

Update 2016-10-04:

#!/usr/bin/perl
use Crypt::CBC;
use MIME::Base64;
my $k1 = "eins";
my $k2 = "zwei";
my $cl = "hallo";
my $c1 = Crypt::CBC->new({ key => $k1 });
$c1->{passphrase} = $k2;
my $en = $c1->encrypt($cl);
my $c2 = Crypt::CBC->new({ key => $k2 });
printf "clear: %s decr: %s",
  encode_base64($cl),
  encode_base64($c2->decrypt($en));

 % ./t.pl 
clear: aGFsbG8=
 decr: aGFsbG8=

Therefore my conclusion that the tool is insecure because of hardcoded keys was wrong. It might still be insecure because of other reasons though, but this is not part of this update. However, it is confusing for the casual reader to see those hardcoded (but, as I now found, unused) keys in the code. In fact I only made the review in the first place because of this.

#source

↷ 25.04.2014      ⤒