From a18295788a381a5083e42fde7d7615b328bb6509 Mon Sep 17 00:00:00 2001 From: Andre Noll Date: Sun, 21 Jun 2009 20:55:07 +0200 Subject: [PATCH] Stronger crypto for client authentication. This patch changes the way clients are authenticated: - The size of the challenge has been increased from sizeof(unsigned long) to 64. Openssl's Rand_bytes() is used to get the random buffer for the challenge and the rc4 keys. - The client responds with the sha1 hash of the challenge rather than sending back the decrypted challenge in plain text. - The rc4 keys are now 2 x 32 bytes long. They are rsa encrypted and sent together with the challenge. - Authentication requests for invalid users are not immediatedly denied as this would reveal the fact that the user does not exist. - rsa keys are required to be at least 2048 bits long. --- INSTALL | 2 +- client_common.c | 103 ++++++++++++++++++++++++++---------------------- command.c | 93 ++++++++++++++++++++++--------------------- configure.ac | 5 ++- crypt.c | 56 ++------------------------ crypt.h | 5 +-- error.h | 5 ++- para.h | 7 ++-- rc4.h | 4 +- user_list.c | 5 +++ 10 files changed, 125 insertions(+), 160 deletions(-) diff --git a/INSTALL b/INSTALL index 70c30947..e2e7ac97 100644 --- a/INSTALL +++ b/INSTALL @@ -69,7 +69,7 @@ with the commands key=~/.paraslash/key.$LOGNAME mkdir -p ~/.paraslash - (umask 077 && openssl genrsa -out $key) + (umask 077 && openssl genrsa -out $key 2048) Next, extract its public part: diff --git a/client_common.c b/client_common.c index b1eaa78b..dd951c15 100644 --- a/client_common.c +++ b/client_common.c @@ -22,6 +22,7 @@ #include "string.h" #include "client.cmdline.h" #include "client.h" +#include "hash.h" /** * Close the connection to para_server and free all resources. @@ -110,7 +111,7 @@ static ssize_t client_recv_buffer(struct client_task *ct) { ssize_t ret; - if (ct->status < CL_RECEIVED_PROCEED) + if (ct->status < CL_SENT_CH_RESPONSE) ret = recv_buffer(ct->rc4c.fd, ct->buf + ct->loaded, CLIENT_BUFSIZE - ct->loaded); else @@ -121,7 +122,6 @@ static ssize_t client_recv_buffer(struct client_task *ct) if (ret > 0) ct->loaded += ret; return ret; - } /** @@ -140,6 +140,7 @@ static ssize_t client_recv_buffer(struct client_task *ct) static void client_post_select(struct sched *s, struct task *t) { struct client_task *ct = container_of(t, struct client_task, task); + unsigned char crypt_buf[1024]; t->error = 0; if (ct->rc4c.fd < 0) @@ -153,66 +154,65 @@ static void client_post_select(struct sched *s, struct task *t) switch (ct->status) { case CL_CONNECTED: /* receive welcome message */ t->error = client_recv_buffer(ct); - if (t->error > 0) - ct->status = CL_RECEIVED_WELCOME; + if (t->error < 0) + goto err; + ct->status = CL_RECEIVED_WELCOME; return; case CL_RECEIVED_WELCOME: /* send auth command */ - sprintf(ct->buf, "auth rc4 %s", ct->user); + sprintf(ct->buf, AUTH_REQUEST_MSG "%s", ct->user); PARA_INFO_LOG("--> %s\n", ct->buf); t->error = send_buffer(ct->rc4c.fd, ct->buf); - if (t->error >= 0) - ct->status = CL_SENT_AUTH; + if (t->error < 0) + goto err; + ct->status = CL_SENT_AUTH; return; - case CL_SENT_AUTH: /* receive challenge number */ + case CL_SENT_AUTH: /* receive challenge and rc4 keys */ ct->loaded = 0; t->error = client_recv_buffer(ct); if (t->error < 0) - return; - if (t->error != 64) { - t->error = -E_INVALID_CHALLENGE; - PARA_ERROR_LOG("received the following: %s\n", ct->buf); - return; - } - PARA_INFO_LOG("<-- [challenge]\n"); - /* decrypt challenge number */ - t->error = para_decrypt_challenge(ct->key_file, &ct->challenge_nr, - (unsigned char *) ct->buf, 64); - if (t->error > 0) - ct->status = CL_RECEIVED_CHALLENGE; + goto err; + PARA_INFO_LOG("<-- [challenge] (%d bytes)\n", t->error); + /* decrypt challenge/rc4 buffer */ + t->error = para_decrypt_buffer(ct->key_file, crypt_buf, + (unsigned char *)ct->buf, t->error); + if (t->error < 0) + goto err; + ct->status = CL_RECEIVED_CHALLENGE; + RC4_set_key(&ct->rc4c.send_key, RC4_KEY_LEN, + crypt_buf + CHALLENGE_SIZE); + RC4_set_key(&ct->rc4c.recv_key, RC4_KEY_LEN, + crypt_buf + CHALLENGE_SIZE + RC4_KEY_LEN); return; - case CL_RECEIVED_CHALLENGE: /* send decrypted challenge */ - PARA_INFO_LOG("--> %lu\n", ct->challenge_nr); - t->error = send_va_buffer(ct->rc4c.fd, "%s%lu", CHALLENGE_RESPONSE_MSG, - ct->challenge_nr); - if (t->error > 0) - ct->status = CL_SENT_CH_RESPONSE; + case CL_RECEIVED_CHALLENGE: + { + unsigned char challenge_sha1[HASH_SIZE]; + /* send sha1 of decrypted challenge */ + sha1_hash((char *)crypt_buf, CHALLENGE_SIZE, challenge_sha1); + hash_to_asc(challenge_sha1, ct->buf); + PARA_INFO_LOG("--> %s\n", ct->buf); + t->error = send_bin_buffer(ct->rc4c.fd, (char *)challenge_sha1, + HASH_SIZE); + if (t->error < 0) + goto err; + ct->status = CL_SENT_CH_RESPONSE; return; + } case CL_SENT_CH_RESPONSE: /* read server response */ { size_t bytes_received; - unsigned char rc4_buf[2 * RC4_KEY_LEN] = ""; ct->loaded = 0; t->error = client_recv_buffer(ct); if (t->error < 0) - return; + goto err; bytes_received = t->error; - PARA_DEBUG_LOG("++++ server info ++++\n%s\n++++ end of server " - "info ++++\n", ct->buf); - /* check if server has sent "Proceed" message and the rc4 keys */ + /* check if server has sent "Proceed" message */ t->error = -E_CLIENT_AUTH; - if (bytes_received < PROCEED_MSG_LEN + 2 * RC4_KEY_LEN) - return; + if (bytes_received < PROCEED_MSG_LEN) + goto err; if (!strstr(ct->buf, PROCEED_MSG)) - return; - PARA_INFO_LOG("decrypting session key\n"); - t->error = para_decrypt_buffer(ct->key_file, rc4_buf, - (unsigned char *)ct->buf + PROCEED_MSG_LEN + 1, - bytes_received - PROCEED_MSG_LEN - 1); - if (t->error < 0) - return; - RC4_set_key(&ct->rc4c.send_key, RC4_KEY_LEN, rc4_buf); - RC4_set_key(&ct->rc4c.recv_key, RC4_KEY_LEN, rc4_buf + RC4_KEY_LEN); + goto err; ct->status = CL_RECEIVED_PROCEED; + t->error = 0; return; } case CL_RECEIVED_PROCEED: /* concat args and send command */ @@ -229,31 +229,38 @@ static void client_post_select(struct sched *s, struct task *t) PARA_DEBUG_LOG("--> %s\n", command); t->error = rc4_send_buffer(&ct->rc4c, command); free(command); - if (t->error > 0) - ct->status = CL_SENT_COMMAND; + if (t->error < 0) + goto err; + ct->status = CL_SENT_COMMAND; return; } case CL_SENT_COMMAND: ct->loaded = 0; t->error = client_recv_buffer(ct); if (t->error < 0) - return; + goto err; if (strstr(ct->buf, AWAITING_DATA_MSG)) ct->status = CL_SENDING; else ct->status = CL_RECEIVING; return; - case CL_SENDING: /* FIXME: might block */ + case CL_SENDING: PARA_INFO_LOG("loaded: %zd\n", *ct->in_loaded); - t->error = rc4_send_bin_buffer(&ct->rc4c, ct->inbuf, *ct->in_loaded); + t->error = rc4_send_bin_buffer(&ct->rc4c, ct->inbuf, + *ct->in_loaded); if (t->error < 0) - return; + goto err; *ct->in_loaded = 0; return; case CL_RECEIVING: t->error = client_recv_buffer(ct); + if (t->error < 0) + goto err; return; } +err: + if (t->error != -E_SERVER_EOF) + PARA_ERROR_LOG("%s\n", para_strerror(-t->error)); } /* connect to para_server and register the client task */ diff --git a/command.c b/command.c index 4b3a1552..a14fff8b 100644 --- a/command.c +++ b/command.c @@ -38,19 +38,10 @@ /** Commands including options must be shorter than this. */ #define MAX_COMMAND_LEN 32768 -static unsigned char rc4_buf[2 * RC4_KEY_LEN]; - extern int mmd_mutex; extern struct misc_meta_data *mmd; extern struct sender senders[]; -static void init_rc4_keys(struct rc4_context *rcc) -{ - get_random_bytes_or_die(rc4_buf, 2 * RC4_KEY_LEN); - RC4_set_key(&rcc->recv_key, RC4_KEY_LEN, rc4_buf); - RC4_set_key(&rcc->send_key, RC4_KEY_LEN, rc4_buf + RC4_KEY_LEN); -} - static void dummy(__a_unused int s) { } @@ -671,10 +662,10 @@ __noreturn void handle_connect(int fd, const char *peername) { int ret, argc; char buf[4096]; - unsigned char crypt_buf[MAXLINE]; + unsigned char rand_buf[CHALLENGE_SIZE + 2 * RC4_KEY_LEN]; + unsigned char challenge_sha1[HASH_SIZE]; struct user *u; struct server_command *cmd = NULL; - long unsigned challenge_nr, chall_response; char **argv = NULL; char *p, *command = NULL; size_t numbytes; @@ -695,52 +686,65 @@ __noreturn void handle_connect(int fd, const char *peername) if (ret < 0) goto err_out; if (ret < 10) { - ret = -E_AUTH; + ret = -E_AUTH_REQUEST; goto err_out; } numbytes = ret; - ret = -E_AUTH; - if (strncmp(buf, "auth rc4 ", 9)) + ret = -E_AUTH_REQUEST; + if (strncmp(buf, AUTH_REQUEST_MSG, strlen(AUTH_REQUEST_MSG))) goto err_out; - p = buf + 9; + p = buf + strlen(AUTH_REQUEST_MSG); PARA_DEBUG_LOG("received auth request for user %s\n", p); ret = -E_BAD_USER; u = lookup_user(p); - if (!u) - goto err_out; - get_random_bytes_or_die((unsigned char *)&challenge_nr, - sizeof(challenge_nr)); - ret = para_encrypt_challenge(u->rsa, challenge_nr, crypt_buf); - if (ret <= 0) - goto err_out; - numbytes = ret; - PARA_DEBUG_LOG("sending %zu byte challenge\n", numbytes); - /* We can't use send_buffer here since buf may contain null bytes */ - ret = send_bin_buffer(fd,(char *) crypt_buf, numbytes); + if (u) { + get_random_bytes_or_die(rand_buf, sizeof(rand_buf)); + ret = para_encrypt_buffer(u->rsa, rand_buf, sizeof(rand_buf), + (unsigned char *)buf); + if (ret < 0) + goto err_out; + numbytes = ret; + } else { + /* + * We don't want to reveal our user names, so we send a + * challenge to the client even if the user does not exist, and + * fail the authentication later. + */ + numbytes = 256; + get_random_bytes_or_die((unsigned char *)buf, numbytes); + } + PARA_DEBUG_LOG("sending %zu byte challenge + rc4 keys (%u bytes)\n", + CHALLENGE_SIZE, numbytes); + ret = send_bin_buffer(fd, buf, numbytes); if (ret < 0) goto net_err; - /* recv decrypted number */ - ret = recv_buffer(fd, buf, sizeof(buf)); + /* recv challenge response */ + ret = recv_bin_buffer(fd, buf, HASH_SIZE); if (ret < 0) goto net_err; numbytes = ret; - ret = -E_AUTH; - if (!numbytes) + PARA_DEBUG_LOG("received %zu bytes challenge response\n", ret); + ret = -E_BAD_USER; + if (!u) goto net_err; - if (sscanf(buf, CHALLENGE_RESPONSE_MSG "%lu", &chall_response) < 1 - || chall_response != challenge_nr) - goto err_out; - /* auth successful, send 'Proceed' message */ - PARA_INFO_LOG("good auth for %s (%lu)\n", u->name, challenge_nr); - sprintf(buf, "%s", PROCEED_MSG); - init_rc4_keys(&rc4c); - /* Should we also encrypt the proceed message? */ - ret = para_encrypt_buffer(u->rsa, rc4_buf, 2 * RC4_KEY_LEN, - (unsigned char *)buf + PROCEED_MSG_LEN + 1); - if (ret <= 0) - goto err_out; - numbytes = ret + strlen(PROCEED_MSG) + 1; - ret = send_bin_buffer(fd, buf, numbytes); + /* + * The correct response is the sha1 of the first CHALLENGE_SIZE bytes + * of the random data. + */ + ret = -E_BAD_AUTH; + if (numbytes != HASH_SIZE) + goto net_err; + sha1_hash((char *)rand_buf, CHALLENGE_SIZE, challenge_sha1); + if (memcmp(challenge_sha1, buf, HASH_SIZE)) + goto net_err; + /* auth successful */ + alarm(0); + PARA_INFO_LOG("good auth for %s\n", u->name); + /* init rc4 keys with the second part of the random buffer */ + RC4_set_key(&rc4c.recv_key, RC4_KEY_LEN, rand_buf + CHALLENGE_SIZE); + RC4_set_key(&rc4c.send_key, RC4_KEY_LEN, rand_buf + CHALLENGE_SIZE + + RC4_KEY_LEN); + ret = rc4_send_buffer(&rc4c, PROCEED_MSG); if (ret < 0) goto net_err; ret = read_command(&rc4c, &command); @@ -757,7 +761,6 @@ __noreturn void handle_connect(int fd, const char *peername) if (ret < 0) goto err_out; /* valid command and sufficient perms */ - alarm(0); argc = split_args(command, &argv, "\n"); PARA_NOTICE_LOG("calling com_%s() for %s@%s\n", cmd->name, u->name, peername); diff --git a/configure.ac b/configure.ac index 85a32729..b0aba937 100644 --- a/configure.ac +++ b/configure.ac @@ -111,7 +111,7 @@ audioc_ldflags="" audiod_cmdline_objs="audiod.cmdline grab_client.cmdline compress_filter.cmdline http_recv.cmdline dccp_recv.cmdline file_write.cmdline client.cmdline audiod_command_list amp_filter.cmdline udp_recv.cmdline - prebuffer_filter.cmdline" + prebuffer_filter.cmdline sha1" audiod_errlist_objs="audiod signal string daemon stat net time grab_client filter_common wav_filter compress_filter amp_filter http_recv dccp_recv recv_common fd sched write_common file_write audiod_command crypt fecdec_filter @@ -138,7 +138,8 @@ writers=" file" default_writer="FILE_WRITE" client_cmdline_objs="client.cmdline" -client_errlist_objs="client net string crypt fd sched stdin stdout client_common" +client_errlist_objs="client net string crypt fd sched stdin stdout + client_common sha1" client_ldflags="" fsck_cmdline_objs="fsck.cmdline" diff --git a/crypt.c b/crypt.c index 679ba35d..a2642929 100644 --- a/crypt.c +++ b/crypt.c @@ -145,38 +145,11 @@ int para_decrypt_buffer(char *key_file, unsigned char *outbuf, unsigned char *in ret = get_rsa_key(key_file, &rsa, LOAD_PRIVATE_KEY); if (ret < 0) return ret; - ret = RSA_private_decrypt(inlen, inbuf, outbuf, rsa, RSA_PKCS1_PADDING); + ret = RSA_private_decrypt(inlen, inbuf, outbuf, rsa, RSA_PKCS1_OAEP_PADDING); rsa_free(rsa); return (ret > 0)? ret : -E_DECRYPT; } -/** - * decrypt the challenge number sent by para_server - * - * \param key_file full path of the rsa key - * \param challenge_nr result is stored here - * \param inbuf the input buffer - * \param rsa_inlen the length of \a inbuf - * - * \return positive on success, negative on errors - * - * \sa para_decrypt_buffer() - */ -int para_decrypt_challenge(char *key_file, long unsigned *challenge_nr, - unsigned char *inbuf, unsigned rsa_inlen) -{ - unsigned char *rsa_out = OPENSSL_malloc(rsa_inlen + 1); - int ret = para_decrypt_buffer(key_file, rsa_out, inbuf, rsa_inlen); - - if (ret >= 0) { - rsa_out[ret] = '\0'; - ret = sscanf((char *)rsa_out, "%lu", challenge_nr) == 1? - 1 : -E_CHALLENGE; - } - OPENSSL_free(rsa_out); - return ret; -} - /** * encrypt a buffer using an RSA key * @@ -196,31 +169,8 @@ int para_encrypt_buffer(RSA *rsa, unsigned char *inbuf, if (flen < 0) return -E_ENCRYPT; - ret = RSA_public_encrypt(flen, inbuf, outbuf, rsa, RSA_PKCS1_PADDING); - return ret < 0? -E_ENCRYPT : ret; -} - -/** - * encrypt the given challenge number - * - * \param rsa: public rsa key - * \param challenge_nr the number to be encrypted - * \param outbuf the output buffer - * - * \a outbuf must be at least 64 bytes long - * - * \return The size of the encrypted data on success, negative on errors - * - * \sa para_encrypt_buffer() - * - */ -int para_encrypt_challenge(RSA* rsa, long unsigned challenge_nr, - unsigned char *outbuf) -{ - unsigned char *inbuf = (unsigned char*) make_message("%lu", challenge_nr); - int ret = para_encrypt_buffer(rsa, inbuf, strlen((char *)inbuf), outbuf); - free(inbuf); - return ret; + ret = RSA_public_encrypt(flen, inbuf, outbuf, rsa, RSA_PKCS1_OAEP_PADDING); + return ret < 0? -E_ENCRYPT : ret; } /** diff --git a/crypt.h b/crypt.h index c430406f..019b643e 100644 --- a/crypt.h +++ b/crypt.h @@ -7,10 +7,6 @@ /** \file crypt.h prototypes for the RSA crypt functions */ #include -int para_decrypt_challenge(char *key_file, long unsigned *challenge_nr, - unsigned char *buf, unsigned rsa_inlen); -int para_encrypt_challenge(RSA* rsa, long unsigned challenge_nr, - unsigned char *outbuf); int para_encrypt_buffer(RSA* rsa, unsigned char *inbuf, unsigned len, unsigned char *outbuf); int para_decrypt_buffer(char *key_file, unsigned char *outbuf, unsigned char *inbuf, @@ -35,4 +31,5 @@ int rc4_recv_buffer(struct rc4_context *rcc, char *buf, size_t size); /** \cond used to distinguish between loading of private/public key */ #define LOAD_PUBLIC_KEY 0 #define LOAD_PRIVATE_KEY 1 +#define CHALLENGE_SIZE 64 /** \endcond **/ diff --git a/error.h b/error.h index c53ab89a..20f6b767 100644 --- a/error.h +++ b/error.h @@ -368,14 +368,15 @@ extern const char **para_errlist[]; #define COMMAND_ERRORS \ PARA_ERROR(COMMAND_SYNTAX, "syntax error in command"), \ - PARA_ERROR(AUTH, "did not receive auth request"), \ + PARA_ERROR(AUTH_REQUEST, "did not receive auth request"), \ PARA_ERROR(NO_AUDIO_FILE, "no audio file"), \ PARA_ERROR(BAD_CMD, "invalid command"), \ PARA_ERROR(PERM, "permission denied"), \ PARA_ERROR(LOCK, "lock error"), \ PARA_ERROR(SENDER_CMD, "command not supported by this sender"), \ PARA_ERROR(SERVER_CRASH, "para_server crashed -- can not live without it"), \ - PARA_ERROR(BAD_USER, "you don't exist. Go away."), \ + PARA_ERROR(BAD_USER, "auth request for invalid user"), \ + PARA_ERROR(BAD_AUTH, "authentication failure"), \ #define DCCP_RECV_ERRORS \ diff --git a/para.h b/para.h index 1dbffdf8..112d5fef 100644 --- a/para.h +++ b/para.h @@ -145,16 +145,17 @@ printf("%s", VERSION_TEXT(_prefix)); \ exit(EXIT_SUCCESS); \ } + +/* Sent by para_client to initiate the authentication procedure. */ +#define AUTH_REQUEST_MSG "auth rsa " /** sent by para_server for commands that expect a data file */ #define AWAITING_DATA_MSG "\nAwaiting Data." /** sent by para_server if authentication was successful */ -#define PROCEED_MSG "\nProceed.\n" +#define PROCEED_MSG "Proceed." /** length of the \p PROCEED_MSG string */ #define PROCEED_MSG_LEN strlen(PROCEED_MSG) /** sent by para_client to indicate the end of the command line */ #define EOC_MSG "\nEnd of Command." -/** sent by para_client, followed by the decrypted challenge number */ -#define CHALLENGE_RESPONSE_MSG "challenge_response:" /* exec */ int para_exec_cmdline_pid(pid_t *pid, const char *cmdline, int *fds); diff --git a/rc4.h b/rc4.h index 9ddd8224..1815e3b8 100644 --- a/rc4.h +++ b/rc4.h @@ -1,4 +1,4 @@ /** \file rc4.h common symbols of command.c and client_common.c */ -/** number of bytes of the rc4 session key */ -#define RC4_KEY_LEN 16 +/** Number of bytes of the rc4 session key. */ +#define RC4_KEY_LEN 32 diff --git a/user_list.c b/user_list.c index 840802b3..1d3f21cf 100644 --- a/user_list.c +++ b/user_list.c @@ -55,6 +55,11 @@ static void populate_user_list(char *user_list_file) para_strerror(-ret)); continue; } + if (ret < CHALLENGE_SIZE + 2 * CHALLENGE_SIZE + 41) { + PARA_WARNING_LOG("rsa key for %s too small\n", n); + rsa_free(rsa); + continue; + } u = para_malloc(sizeof(*u)); u->name = para_strdup(n); u->rsa = rsa; -- 2.39.5