On 15-08-17 10:10, Steffan Karger wrote: > On 14-08-17 22:59, Guido Vranken wrote: >> this concerns key_method 1. I know it's deprecated, but reporting it >> just in case people still use it.. >> >> So key_method_1_read() calls read_key() which doesn't perform adequate >> bounds checks. cipher_length and hmac_length are specified by the >> peer: >> >> 1643 uint8_t cipher_length; >> 1644 uint8_t hmac_length; >> 1645 >> 1646 CLEAR(*key); >> 1647 if (!buf_read(buf, &cipher_length, 1)) >> 1648 { >> 1649 goto read_err; >> 1650 } >> 1651 if (!buf_read(buf, &hmac_length, 1)) >> 1652 { >> 1653 goto read_err; >> 1654 } >> >> And this many bytes of data are then read into key->cipher and key->hmac: >> >> 1656 if (!buf_read(buf, key->cipher, cipher_length)) >> 1657 { >> 1658 goto read_err; >> 1659 } >> 1660 if (!buf_read(buf, key->hmac, hmac_length)) >> 1661 { >> 1662 goto read_err; >> 1663 } >> >> key->hmac is only 64 bytes. So it's trivial to overflow key->hmac. > > Ouch. Thanks once more for the report! > > To all: following <54fc26bb.2000...@karger.me> and commit 1ce06386, I > think it really is time to remove key method 1 from the master branch. > > Attached a proposed patch to fix this issue in release/2.4 and master.
What I should mention too is that 'key-method 1' only works for 'peer to peer TLS setups' (i.e. 'tls-server' and 'tls-client'). As soon as someone uses 'mode server' (or 'server'), or 'pull' (or 'client'), key-method 2 must be used. This strongly limits the possible affected users. -Steffan ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel