diff options
author | djm@openbsd.org <djm@openbsd.org> | 2016-07-08 03:44:42 +0000 |
---|---|---|
committer | Damien Miller <djm@mindrot.org> | 2016-07-08 13:50:03 +1000 |
commit | 6d31193d0baa3da339c196ac49625b7ba1c2ecc7 (patch) | |
tree | 83c1b9c11099ff8577178f702f2cb34765229d9b /packet.c | |
parent | 71f5598f06941f645a451948c4a5125c83828e1c (diff) |
upstream commit
Improve crypto ordering for Encrypt-then-MAC (EtM) mode
MAC algorithms.
Previously we were computing the MAC, decrypting the packet and then
checking the MAC. This gave rise to the possibility of creating a
side-channel oracle in the decryption step, though no such oracle has
been identified.
This adds a mac_check() function that computes and checks the MAC in
one pass, and uses it to advance MAC checking for EtM algorithms to
before payload decryption.
Reported by Jean Paul Degabriele, Kenny Paterson, Torben Hansen and
Martin Albrecht. feedback and ok markus@
Upstream-ID: 1999bb67cab47dda5b10b80d8155fe83d4a1867b
Diffstat (limited to 'packet.c')
-rw-r--r-- | packet.c | 35 |
1 files changed, 17 insertions, 18 deletions
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: packet.c,v 1.230 2016/03/07 19:02:43 djm Exp $ */ | 1 | /* $OpenBSD: packet.c,v 1.231 2016/07/08 03:44:42 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Author: Tatu Ylonen <ylo@cs.hut.fi> | 3 | * Author: Tatu Ylonen <ylo@cs.hut.fi> |
4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland | 4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland |
@@ -1690,7 +1690,7 @@ ssh_packet_read_poll2(struct ssh *ssh, u_char *typep, u_int32_t *seqnr_p) | |||
1690 | { | 1690 | { |
1691 | struct session_state *state = ssh->state; | 1691 | struct session_state *state = ssh->state; |
1692 | u_int padlen, need; | 1692 | u_int padlen, need; |
1693 | u_char *cp, macbuf[SSH_DIGEST_MAX_LENGTH]; | 1693 | u_char *cp; |
1694 | u_int maclen, aadlen = 0, authlen = 0, block_size; | 1694 | u_int maclen, aadlen = 0, authlen = 0, block_size; |
1695 | struct sshenc *enc = NULL; | 1695 | struct sshenc *enc = NULL; |
1696 | struct sshmac *mac = NULL; | 1696 | struct sshmac *mac = NULL; |
@@ -1790,17 +1790,21 @@ ssh_packet_read_poll2(struct ssh *ssh, u_char *typep, u_int32_t *seqnr_p) | |||
1790 | * 'maclen' bytes of message authentication code. | 1790 | * 'maclen' bytes of message authentication code. |
1791 | */ | 1791 | */ |
1792 | if (sshbuf_len(state->input) < aadlen + need + authlen + maclen) | 1792 | if (sshbuf_len(state->input) < aadlen + need + authlen + maclen) |
1793 | return 0; | 1793 | return 0; /* packet is incomplete */ |
1794 | #ifdef PACKET_DEBUG | 1794 | #ifdef PACKET_DEBUG |
1795 | fprintf(stderr, "read_poll enc/full: "); | 1795 | fprintf(stderr, "read_poll enc/full: "); |
1796 | sshbuf_dump(state->input, stderr); | 1796 | sshbuf_dump(state->input, stderr); |
1797 | #endif | 1797 | #endif |
1798 | /* EtM: compute mac over encrypted input */ | 1798 | /* EtM: check mac over encrypted input */ |
1799 | if (mac && mac->enabled && mac->etm) { | 1799 | if (mac && mac->enabled && mac->etm) { |
1800 | if ((r = mac_compute(mac, state->p_read.seqnr, | 1800 | if ((r = mac_check(mac, state->p_read.seqnr, |
1801 | sshbuf_ptr(state->input), aadlen + need, | 1801 | sshbuf_ptr(state->input), aadlen + need, |
1802 | macbuf, sizeof(macbuf))) != 0) | 1802 | sshbuf_ptr(state->input) + aadlen + need + authlen, |
1803 | maclen)) != 0) { | ||
1804 | if (r == SSH_ERR_MAC_INVALID) | ||
1805 | logit("Corrupted MAC on input."); | ||
1803 | goto out; | 1806 | goto out; |
1807 | } | ||
1804 | } | 1808 | } |
1805 | if ((r = sshbuf_reserve(state->incoming_packet, aadlen + need, | 1809 | if ((r = sshbuf_reserve(state->incoming_packet, aadlen + need, |
1806 | &cp)) != 0) | 1810 | &cp)) != 0) |
@@ -1810,26 +1814,21 @@ ssh_packet_read_poll2(struct ssh *ssh, u_char *typep, u_int32_t *seqnr_p) | |||
1810 | goto out; | 1814 | goto out; |
1811 | if ((r = sshbuf_consume(state->input, aadlen + need + authlen)) != 0) | 1815 | if ((r = sshbuf_consume(state->input, aadlen + need + authlen)) != 0) |
1812 | goto out; | 1816 | goto out; |
1813 | /* | ||
1814 | * compute MAC over seqnr and packet, | ||
1815 | * increment sequence number for incoming packet | ||
1816 | */ | ||
1817 | if (mac && mac->enabled) { | 1817 | if (mac && mac->enabled) { |
1818 | if (!mac->etm) | 1818 | /* Not EtM: check MAC over cleartext */ |
1819 | if ((r = mac_compute(mac, state->p_read.seqnr, | 1819 | if (!mac->etm && (r = mac_check(mac, state->p_read.seqnr, |
1820 | sshbuf_ptr(state->incoming_packet), | 1820 | sshbuf_ptr(state->incoming_packet), |
1821 | sshbuf_len(state->incoming_packet), | 1821 | sshbuf_len(state->incoming_packet), |
1822 | macbuf, sizeof(macbuf))) != 0) | 1822 | sshbuf_ptr(state->input), maclen)) != 0) { |
1823 | if (r != SSH_ERR_MAC_INVALID) | ||
1823 | goto out; | 1824 | goto out; |
1824 | if (timingsafe_bcmp(macbuf, sshbuf_ptr(state->input), | ||
1825 | mac->mac_len) != 0) { | ||
1826 | logit("Corrupted MAC on input."); | 1825 | logit("Corrupted MAC on input."); |
1827 | if (need > PACKET_MAX_SIZE) | 1826 | if (need > PACKET_MAX_SIZE) |
1828 | return SSH_ERR_INTERNAL_ERROR; | 1827 | return SSH_ERR_INTERNAL_ERROR; |
1829 | return ssh_packet_start_discard(ssh, enc, mac, | 1828 | return ssh_packet_start_discard(ssh, enc, mac, |
1830 | state->packlen, PACKET_MAX_SIZE - need); | 1829 | state->packlen, PACKET_MAX_SIZE - need); |
1831 | } | 1830 | } |
1832 | 1831 | /* Remove MAC from input buffer */ | |
1833 | DBG(debug("MAC #%d ok", state->p_read.seqnr)); | 1832 | DBG(debug("MAC #%d ok", state->p_read.seqnr)); |
1834 | if ((r = sshbuf_consume(state->input, mac->mac_len)) != 0) | 1833 | if ((r = sshbuf_consume(state->input, mac->mac_len)) != 0) |
1835 | goto out; | 1834 | goto out; |