Skip to content

Commit

Permalink
Fixed random on sensible parts (generation auth_key, session_id and k…
Browse files Browse the repository at this point in the history
…eys for secret chats)
  • Loading branch information
vysheng committed Dec 24, 2013
1 parent d947869 commit b2ba81e
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 7 deletions.
4 changes: 4 additions & 0 deletions main.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ char *binlog_file_name;
int binlog_enabled;
extern int log_level;
int sync_from_start;
int allow_weak_random;

void set_default_username (const char *s) {
if (default_username) {
Expand Down Expand Up @@ -375,6 +376,9 @@ void args_parse (int argc, char **argv) {
case 'E':
disable_auto_accept = 1;
break;
case 'w':
allow_weak_random = 1;
break;
case 'h':
default:
usage ();
Expand Down
19 changes: 14 additions & 5 deletions mtproto-client.c
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ char new_nonce[256];
char server_nonce[256];
extern int binlog_enabled;
extern int disable_auto_accept;

extern int allow_weak_random;

int total_packets_sent;
long long total_data_sent;
Expand Down Expand Up @@ -98,6 +98,15 @@ double get_utime (int clock_id) {
return res;
}

void do_rand (void *s, int l) {
if (RAND_bytes (s, l) < 0) {
if (allow_weak_random) {
RAND_pseudo_bytes (s, l);
} else {
assert (0 && "End of random. If you want, you can start with -w");
}
}
}


#define STATS_BUFF_SIZE (64 << 10)
Expand Down Expand Up @@ -223,7 +232,7 @@ int rpc_send_message (struct connection *c, void *data, int len) {

int send_req_pq_packet (struct connection *c) {
assert (c_state == st_init);
assert (RAND_pseudo_bytes ((unsigned char *) nonce, 16) >= 0);
do_rand (nonce, 16);
unenc_msg_header.out_msg_id = 0;
clear_packet ();
out_int (CODE_req_pq);
Expand Down Expand Up @@ -371,7 +380,7 @@ int process_respq_answer (struct connection *c, char *packet, int len) {
//out_int (0x0501); // q=5
out_ints ((int *) nonce, 4);
out_ints ((int *) server_nonce, 4);
assert (RAND_pseudo_bytes ((unsigned char *) new_nonce, 32) >= 0);
do_rand (new_nonce, 32);
out_ints ((int *) new_nonce, 8);
sha1 ((unsigned char *) (packet_buffer + 5), (packet_ptr - packet_buffer - 5) * 4, (unsigned char *) packet_buffer);

Expand Down Expand Up @@ -564,7 +573,7 @@ int process_dh_answer (struct connection *c, char *packet, int len) {
BN_init (&dh_g);
BN_set_word (&dh_g, g);

assert (RAND_pseudo_bytes ((unsigned char *)s_power, 256) >= 0);
do_rand (s_power, 256);
BIGNUM *dh_power = BN_new ();
assert (BN_bin2bn ((unsigned char *)s_power, 256, dh_power) == dh_power);

Expand Down Expand Up @@ -683,7 +692,7 @@ void init_enc_msg (struct session *S, int useful) {
// assert (DC->server_salt);
enc_msg.server_salt = DC->server_salt;
if (!S->session_id) {
assert (RAND_pseudo_bytes ((unsigned char *) &S->session_id, 8) >= 0);
do_rand (&S->session_id, 8);
}
enc_msg.session_id = S->session_id;
//enc_msg.auth_key_id2 = auth_key_id;
Expand Down
1 change: 1 addition & 0 deletions mtproto-client.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,5 @@ void work_update_binlog (void);
int check_g (unsigned char p[256], BIGNUM *g);
int check_g_bn (BIGNUM *p, BIGNUM *g);
int check_DH_params (BIGNUM *p, int g);
void do_rand (void *s, int l);
#endif
6 changes: 4 additions & 2 deletions queries.c
Original file line number Diff line number Diff line change
Expand Up @@ -2263,8 +2263,10 @@ void do_send_accept_encr_chat (struct secret_chat *E, unsigned char *random) {
}
}
if (ok) { return; } // Already generated key for this chat
for (i = 0; i < 64; i++) {
*(((int *)random) + i) ^= mrand48 ();

This comment has been minimized.

Copy link
@xnyhps

xnyhps Dec 25, 2013

You really should offer to delete and regenerate all keys generated with the old code. This code means that, for the server which knows random, there can be only 2^48 possible keys used. Additionally it is very easy for the server to brute-force the internal rand48 state as it can observe output values from other places where lrand48 is used.

This comment has been minimized.

Copy link
@vysheng

vysheng Dec 25, 2013

Owner

It seems, that almost nobody uses secret chats. At least nobody reported bugs in secret chats, though there were plenty of them (and I beleive there remain some yet). But yes, if you don't trust server and want security, you should regenerate all encrypted chat keys and maybe even auth key, because it used RAND_pseudo_bytes. If you see where this note should be, I can put it there/

This comment has been minimized.

Copy link
@xnyhps

xnyhps Dec 25, 2013

This wasn't even RAND_pseudo_bytes, which has a chance to be secure (it's secure when openssl can guarantee it), but mrand48, which is much worse.

unsigned char random_here[256];
do_rand (random_here, 256);
for (i = 0; i < 256; i++) {
random[i] ^= random_here[i];
}
BIGNUM *b = BN_bin2bn (random, 256, 0);
assert (b);
Expand Down

0 comments on commit b2ba81e

Please sign in to comment.