aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimo Sorce <simo@redhat.com>2018-10-01 12:56:32 -0400
committerAndreas Schneider <asn@cryptomilk.org>2019-03-07 12:03:27 +0100
commitc180211c6b804628d308743a51a0270873cf7c6f (patch)
treeaaa6c6d80d7d0d5d042b4a1ddd309236d2b97a2a
parentc235841436ae45a0220097fa9721d09719700194 (diff)
downloadlibssh-c180211c6b804628d308743a51a0270873cf7c6f.tar.gz
libssh-c180211c6b804628d308743a51a0270873cf7c6f.tar.xz
libssh-c180211c6b804628d308743a51a0270873cf7c6f.zip
Clean up code that generates session keys
This patch simply reworks the code to make it more understandable and reduce if() branches. It also avoids reallocs, and instead uses a support buffer to hold intermediate results of the hmac function so that no buffer overrides happen when the requested size is not an exact mutiple of the digest_len. Signed-off-by: Simo Sorce <simo@redhat.com> Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
-rw-r--r--src/kex.c257
1 files changed, 110 insertions, 147 deletions
diff --git a/src/kex.c b/src/kex.c
index 03d66e70..1aaa4154 100644
--- a/src/kex.c
+++ b/src/kex.c
@@ -1346,17 +1346,15 @@ int ssh_hashbufin_add_cookie(ssh_session session, unsigned char *cookie)
return 0;
}
-static int generate_one_key(ssh_string k,
- struct ssh_crypto_struct *crypto,
- unsigned char **output,
- char letter,
+static int generate_one_key(ssh_string k, struct ssh_crypto_struct *crypto,
+ unsigned char *output, char letter,
size_t requested_size)
{
ssh_mac_ctx ctx;
- unsigned char *tmp;
size_t size = crypto->digest_len;
- ctx = ssh_mac_ctx_init(crypto->mac_type);
+ unsigned char digest[size];
+ ctx = ssh_mac_ctx_init(crypto->mac_type);
if (ctx == NULL) {
return -1;
}
@@ -1365,25 +1363,27 @@ static int generate_one_key(ssh_string k,
ssh_mac_update(ctx, crypto->secret_hash, crypto->digest_len);
ssh_mac_update(ctx, &letter, 1);
ssh_mac_update(ctx, crypto->session_id, crypto->digest_len);
- ssh_mac_final(*output, ctx);
+ ssh_mac_final(digest, ctx);
- while(requested_size > size) {
- tmp = realloc(*output, size + crypto->digest_len);
- if (tmp == NULL) {
- return -1;
- }
- *output = tmp;
+ if (requested_size < size) {
+ size = requested_size;
+ }
+ memcpy(output, digest, size);
+ while (requested_size > size) {
ctx = ssh_mac_ctx_init(crypto->mac_type);
if (ctx == NULL) {
return -1;
}
ssh_mac_update(ctx, k, ssh_string_len(k) + 4);
- ssh_mac_update(ctx,
- crypto->secret_hash,
- crypto->digest_len);
- ssh_mac_update(ctx, tmp, size);
- ssh_mac_final(tmp + size, ctx);
+ ssh_mac_update(ctx, crypto->secret_hash, crypto->digest_len);
+ ssh_mac_update(ctx, output, size);
+ ssh_mac_final(digest, ctx);
+ if (requested_size < size + crypto->digest_len) {
+ memcpy(output+size, digest, requested_size - size);
+ } else {
+ memcpy(output+size, digest, crypto->digest_len);
+ }
size += crypto->digest_len;
}
@@ -1394,6 +1394,17 @@ int ssh_generate_session_keys(ssh_session session)
{
ssh_string k_string = NULL;
struct ssh_crypto_struct *crypto = session->next_crypto;
+ unsigned char *IV_cli_to_srv = NULL;
+ unsigned char *IV_srv_to_cli = NULL;
+ unsigned char *enckey_cli_to_srv = NULL;
+ unsigned char *enckey_srv_to_cli = NULL;
+ unsigned char *intkey_cli_to_srv = NULL;
+ unsigned char *intkey_srv_to_cli = NULL;
+ size_t IV_len = 0;
+ size_t enckey_cli_to_srv_len = 0;
+ size_t enckey_srv_to_cli_len = 0;
+ size_t intkey_cli_to_srv_len = 0;
+ size_t intkey_srv_to_cli_len = 0;
int rc = -1;
k_string = ssh_make_bignum_string(crypto->k);
@@ -1402,153 +1413,105 @@ int ssh_generate_session_keys(ssh_session session)
goto error;
}
- crypto->encryptIV = malloc(crypto->digest_len);
- crypto->decryptIV = malloc(crypto->digest_len);
- crypto->encryptkey = malloc(crypto->digest_len);
- crypto->decryptkey = malloc(crypto->digest_len);
- crypto->encryptMAC = malloc(crypto->digest_len);
- crypto->decryptMAC = malloc(crypto->digest_len);
- if (crypto->encryptIV == NULL ||
- crypto->decryptIV == NULL ||
- crypto->encryptkey == NULL || crypto->decryptkey == NULL ||
- crypto->encryptMAC == NULL || crypto->decryptMAC == NULL){
+ IV_len = crypto->digest_len;
+ if (session->client) {
+ enckey_cli_to_srv_len = crypto->out_cipher->keysize / 8;
+ enckey_srv_to_cli_len = crypto->in_cipher->keysize / 8;
+ intkey_cli_to_srv_len = hmac_digest_len(crypto->out_hmac);
+ intkey_srv_to_cli_len = hmac_digest_len(crypto->in_hmac);
+ } else {
+ enckey_cli_to_srv_len = crypto->in_cipher->keysize / 8;
+ enckey_srv_to_cli_len = crypto->out_cipher->keysize / 8;
+ intkey_cli_to_srv_len = hmac_digest_len(crypto->in_hmac);
+ intkey_srv_to_cli_len = hmac_digest_len(crypto->out_hmac);
+ }
+
+ IV_cli_to_srv = malloc(IV_len);
+ IV_srv_to_cli = malloc(IV_len);
+ enckey_cli_to_srv = malloc(enckey_cli_to_srv_len);
+ enckey_srv_to_cli = malloc(enckey_srv_to_cli_len);
+ intkey_cli_to_srv = malloc(intkey_cli_to_srv_len);
+ intkey_srv_to_cli = malloc(intkey_srv_to_cli_len);
+ if (IV_cli_to_srv == NULL || IV_srv_to_cli == NULL ||
+ enckey_cli_to_srv == NULL || enckey_srv_to_cli == NULL ||
+ intkey_cli_to_srv == NULL || intkey_srv_to_cli == NULL) {
ssh_set_error_oom(session);
goto error;
}
/* IV */
- if (session->client) {
- rc = generate_one_key(k_string,
- crypto,
- &crypto->encryptIV,
- 'A',
- crypto->digest_len);
- if (rc < 0) {
- goto error;
- }
- rc = generate_one_key(k_string,
- crypto,
- &crypto->decryptIV,
- 'B',
- crypto->digest_len);
- if (rc < 0) {
- goto error;
- }
- } else {
- rc = generate_one_key(k_string,
- crypto,
- &crypto->decryptIV,
- 'A',
- crypto->digest_len);
- if (rc < 0) {
- goto error;
- }
- rc = generate_one_key(k_string,
- crypto,
- &crypto->encryptIV,
- 'B',
- crypto->digest_len);
- if (rc < 0) {
- goto error;
- }
+ rc = generate_one_key(k_string, crypto, IV_cli_to_srv, 'A', IV_len);
+ if (rc < 0) {
+ goto error;
}
- if (session->client) {
- rc = generate_one_key(k_string,
- crypto,
- &crypto->encryptkey,
- 'C',
- crypto->out_cipher->keysize / 8);
- if (rc < 0) {
- goto error;
- }
- rc = generate_one_key(k_string,
- crypto,
- &crypto->decryptkey,
- 'D',
- crypto->in_cipher->keysize / 8);
- if (rc < 0) {
- goto error;
- }
- } else {
- rc = generate_one_key(k_string,
- crypto,
- &crypto->decryptkey,
- 'C',
- crypto->in_cipher->keysize / 8);
- if (rc < 0) {
- goto error;
- }
- rc = generate_one_key(k_string,
- crypto,
- &crypto->encryptkey,
- 'D',
- crypto->out_cipher->keysize / 8);
- if (rc < 0) {
- goto error;
- }
+ rc = generate_one_key(k_string, crypto, IV_srv_to_cli, 'B', IV_len);
+ if (rc < 0) {
+ goto error;
+ }
+ /* Encryption Key */
+ rc = generate_one_key(k_string, crypto, enckey_cli_to_srv, 'C',
+ enckey_cli_to_srv_len);
+ if (rc < 0) {
+ goto error;
+ }
+ rc = generate_one_key(k_string, crypto, enckey_srv_to_cli, 'D',
+ enckey_srv_to_cli_len);
+ if (rc < 0) {
+ goto error;
+ }
+ /* Integrity Key */
+ rc = generate_one_key(k_string, crypto, intkey_cli_to_srv, 'E',
+ intkey_cli_to_srv_len);
+ if (rc < 0) {
+ goto error;
+ }
+ rc = generate_one_key(k_string, crypto, intkey_srv_to_cli, 'F',
+ intkey_srv_to_cli_len);
+ if (rc < 0) {
+ goto error;
}
- if(session->client) {
- rc = generate_one_key(k_string,
- crypto,
- &crypto->encryptMAC,
- 'E',
- hmac_digest_len(crypto->out_hmac));
- if (rc < 0) {
- goto error;
- }
- rc = generate_one_key(k_string,
- crypto,
- &crypto->decryptMAC,
- 'F',
- hmac_digest_len(crypto->in_hmac));
- if (rc < 0) {
- goto error;
- }
+ if (session->client) {
+ crypto->encryptIV = IV_cli_to_srv;
+ crypto->decryptIV = IV_srv_to_cli;
+ crypto->encryptkey = enckey_cli_to_srv;
+ crypto->decryptkey = enckey_srv_to_cli;
+ crypto->encryptMAC = intkey_cli_to_srv;
+ crypto->decryptMAC = intkey_srv_to_cli;
} else {
- rc = generate_one_key(k_string,
- crypto,
- &crypto->decryptMAC,
- 'E',
- hmac_digest_len(crypto->in_hmac));
- if (rc < 0) {
- goto error;
- }
- rc = generate_one_key(k_string,
- crypto,
- &crypto->encryptMAC,
- 'F',
- hmac_digest_len(crypto->out_hmac));
- if (rc < 0) {
- goto error;
- }
+ crypto->encryptIV = IV_srv_to_cli;
+ crypto->decryptIV = IV_cli_to_srv;
+ crypto->encryptkey = enckey_srv_to_cli;
+ crypto->decryptkey = enckey_cli_to_srv;
+ crypto->encryptMAC = intkey_srv_to_cli;
+ crypto->decryptMAC = intkey_cli_to_srv;
}
#ifdef DEBUG_CRYPTO
- ssh_print_hexa("Encrypt IV",
- crypto->encryptIV,
- crypto->digest_len);
- ssh_print_hexa("Decrypt IV",
- crypto->decryptIV,
- crypto->digest_len);
- ssh_print_hexa("Encryption key",
- crypto->encryptkey,
- crypto->out_cipher->keysize / 8);
- ssh_print_hexa("Decryption key",
- crypto->decryptkey,
- crypto->in_cipher->keysize / 8);
- ssh_print_hexa("Encryption MAC",
- crypto->encryptMAC,
- hmac_digest_len(crypto->out_hmac));
- ssh_print_hexa("Decryption MAC",
- crypto->decryptMAC,
- hmac_digest_len(crypto->in_hmac));
+ ssh_print_hexa("Client to Server IV", IV_cli_to_srv, IV_len);
+ ssh_print_hexa("Server to Client IV", IV_srv_to_cli, IV_len);
+ ssh_print_hexa("Client to Server Encryption Key", enckey_cli_to_srv,
+ enckey_cli_to_srv_len);
+ ssh_print_hexa("Server to Client Encryption Key", enckey_srv_to_cli,
+ enckey_srv_to_cli_len);
+ ssh_print_hexa("Client to Server Integrity Key", intkey_cli_to_srv,
+ intkey_cli_to_srv_len);
+ ssh_print_hexa("Server to Client Integrity Key", intkey_srv_to_cli,
+ intkey_srv_to_cli_len);
#endif
rc = 0;
error:
ssh_string_burn(k_string);
ssh_string_free(k_string);
+ if (rc != 0) {
+ free(IV_cli_to_srv);
+ free(IV_srv_to_cli);
+ free(enckey_cli_to_srv);
+ free(enckey_srv_to_cli);
+ free(intkey_cli_to_srv);
+ free(intkey_srv_to_cli);
+ }
return rc;
}