Skip to content

Commit

Permalink
ovpn-dco: remove cbc-hmac crypto
Browse files Browse the repository at this point in the history
Since modern OpenVPN deployments negotiate AES-GCM,
there is no need to support AES-CBC / HMAC.

ovpn-dco doesn't support it, so clean up core as well.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
  • Loading branch information
lstipakov authored and ordex committed Nov 16, 2020
1 parent 9574172 commit 8ce41b7
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 73 deletions.
5 changes: 1 addition & 4 deletions openvpn/dco/key.hpp
Expand Up @@ -27,10 +27,8 @@ namespace KoRekey {

struct KeyDirection {
const unsigned char *cipher_key;
const unsigned char *hmac_key; // only CBC
unsigned char nonce_tail[8]; // only GCM
unsigned char nonce_tail[8]; // only AEAD
unsigned int cipher_key_size;
unsigned int hmac_key_size; // only CBC
};

struct KeyConfig {
Expand All @@ -40,7 +38,6 @@ struct KeyConfig {
int key_id;
int remote_peer_id;
unsigned int cipher_alg;
unsigned int hmac_alg; // only CBC
};

} // namespace KoRekey
Expand Down
64 changes: 5 additions & 59 deletions openvpn/dco/ovpndcokocrypto.hpp
Expand Up @@ -57,18 +57,6 @@ class OvpnDcoKey : public Key {
kc.cipher_alg = OVPN_CIPHER_ALG_AES_GCM;
kc.encrypt.cipher_key_size = 256 / 8;
break;
case CryptoAlgs::AES_128_CBC:
kc.cipher_alg = OVPN_CIPHER_ALG_AES_CBC;
kc.encrypt.cipher_key_size = 128 / 8;
break;
case CryptoAlgs::AES_192_CBC:
kc.cipher_alg = OVPN_CIPHER_ALG_AES_CBC;
kc.encrypt.cipher_key_size = 192 / 8;
break;
case CryptoAlgs::AES_256_CBC:
kc.cipher_alg = OVPN_CIPHER_ALG_AES_CBC;
kc.encrypt.cipher_key_size = 256 / 8;
break;
default:
OPENVPN_THROW(korekey_error,
"cipher alg " << calg.name()
Expand All @@ -77,58 +65,16 @@ class OvpnDcoKey : public Key {
}
kc.decrypt.cipher_key_size = kc.encrypt.cipher_key_size;

if (kc.cipher_alg != CryptoAlgs::NONE)
{
kc.encrypt.cipher_key = verify_key("cipher encrypt", rkinfo.encrypt_cipher,
kc.encrypt.cipher_key_size);
kc.decrypt.cipher_key = verify_key("cipher decrypt", rkinfo.decrypt_cipher,
kc.decrypt.cipher_key_size);
}

switch (calg.mode()) {
case CryptoAlgs::CBC_HMAC:
// if CBC mode, process HMAC digest
{
const CryptoAlgs::Alg &halg = CryptoAlgs::get(ci.hmac_alg);
switch (ci.hmac_alg) {
case CryptoAlgs::SHA256:
kc.hmac_alg = OVPN_HMAC_ALG_SHA256;
break;
case CryptoAlgs::SHA512:
kc.hmac_alg = OVPN_HMAC_ALG_SHA512;
break;
case CryptoAlgs::NONE:
kc.hmac_alg = OVPN_HMAC_ALG_NONE;
break;
default:
OPENVPN_THROW(korekey_error,
"HMAC alg "
<< halg.name()
<< " is not currently supported by ovpn-dco");
}
kc.encrypt.hmac_key_size = halg.size();
kc.decrypt.hmac_key_size = kc.encrypt.hmac_key_size;

// set hmac keys
kc.encrypt.hmac_key = verify_key("hmac encrypt", rkinfo.encrypt_hmac,
kc.encrypt.hmac_key_size);
kc.decrypt.hmac_key = verify_key("hmac decrypt", rkinfo.decrypt_hmac,
kc.decrypt.hmac_key_size);
}
break;
if (calg.mode() == CryptoAlgs::AEAD) {
kc.encrypt.cipher_key = verify_key(
"cipher encrypt", rkinfo.encrypt_cipher, kc.encrypt.cipher_key_size);
kc.decrypt.cipher_key = verify_key(
"cipher decrypt", rkinfo.decrypt_cipher, kc.decrypt.cipher_key_size);

case CryptoAlgs::AEAD:
set_nonce_tail("AEAD nonce tail encrypt", kc.encrypt.nonce_tail,
sizeof(kc.encrypt.nonce_tail), rkinfo.encrypt_hmac);
set_nonce_tail("AEAD nonce tail decrypt", kc.decrypt.nonce_tail,
sizeof(kc.decrypt.nonce_tail), rkinfo.decrypt_hmac);

break;

default: {
// should have been caught above
throw korekey_error("internal error");
}
}

kc.key_id = rkinfo.key_id;
Expand Down
10 changes: 0 additions & 10 deletions openvpn/tun/linux/client/genl.hpp
Expand Up @@ -220,20 +220,13 @@ template <typename ReadHandler> class GeNL : public RC<thread_unsafe_refcount> {
NLA_PUT_U8(msg, OVPN_ATTR_KEY_SLOT, key_slot);
NLA_PUT_U16(msg, OVPN_ATTR_KEY_ID, kc->key_id);
NLA_PUT_U16(msg, OVPN_ATTR_CIPHER_ALG, kc->cipher_alg);
if ((kc->cipher_alg == OVPN_CIPHER_ALG_AES_CBC) ||
(kc->cipher_alg == OVPN_CIPHER_ALG_NONE)) {
NLA_PUT_U16(msg, OVPN_ATTR_HMAC_ALG, kc->hmac_alg);
}

key_dir = nla_nest_start(msg, OVPN_ATTR_ENCRYPT_KEY);
NLA_PUT(msg, OVPN_KEY_DIR_ATTR_CIPHER_KEY, kc->encrypt.cipher_key_size,
kc->encrypt.cipher_key);
if (kc->cipher_alg == OVPN_CIPHER_ALG_AES_GCM) {
NLA_PUT(msg, OVPN_KEY_DIR_ATTR_NONCE_TAIL, NONCE_TAIL_LEN,
kc->encrypt.nonce_tail);
} else {
NLA_PUT(msg, OVPN_KEY_DIR_ATTR_HMAC_KEY, kc->encrypt.hmac_key_size,
kc->encrypt.hmac_key);
}
nla_nest_end(msg, key_dir);

Expand All @@ -243,9 +236,6 @@ template <typename ReadHandler> class GeNL : public RC<thread_unsafe_refcount> {
if (kc->cipher_alg == OVPN_CIPHER_ALG_AES_GCM) {
NLA_PUT(msg, OVPN_KEY_DIR_ATTR_NONCE_TAIL, NONCE_TAIL_LEN,
kc->decrypt.nonce_tail);
} else {
NLA_PUT(msg, OVPN_KEY_DIR_ATTR_HMAC_KEY, kc->decrypt.hmac_key_size,
kc->decrypt.hmac_key);
}
nla_nest_end(msg, key_dir);

Expand Down

0 comments on commit 8ce41b7

Please sign in to comment.