Skip to content

Commit

Permalink
Differentiate Packet ID types into data channel and control channel ids
Browse files Browse the repository at this point in the history
Data channel packet ids (in the formats that OpenVPN 3.x supports)
are plain 32 or 64 bit ids while control channel is a 32 bit time + 32
bit counter id. Seperate these more clearly and let CBC mode use the
same Packet ID implementation that AEAD mode uses.

Also add more unit tests related to data channel tests packets by
adapting the control channel test where applicable and add a few more
related to packet id wrapping

Signed-off-by: Arne Schwabe <arne@openvpn.net>
  • Loading branch information
schwabe authored and Jenkins-dev committed Sep 11, 2024
1 parent 16b2c4a commit c78aaec
Show file tree
Hide file tree
Showing 18 changed files with 536 additions and 303 deletions.
30 changes: 15 additions & 15 deletions openvpn/crypto/crypto_aead.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
#include <openvpn/buffer/buffer.hpp>
#include <openvpn/frame/frame.hpp>
#include <openvpn/crypto/static_key.hpp>
#include <openvpn/crypto/packet_id_aead.hpp>
#include <openvpn/crypto/packet_id_data.hpp>
#include <openvpn/log/sessionstats.hpp>
#include <openvpn/crypto/cryptodc.hpp>

Expand Down Expand Up @@ -85,12 +85,12 @@ class Crypto : public CryptoDCInstance
}

// for encrypt
Nonce(const Nonce &ref, PacketIDAEADSend &pid_send, const unsigned char *op32)
Nonce(const Nonce &ref, PacketIDDataSend &pid_send, const unsigned char *op32)
{
/** Copy op code and tail of packet ID */
std::memcpy(data, ref.data, sizeof(data));

Buffer buf(data + data_offset_pkt_id, PacketIDAEAD::long_id_size, false);
Buffer buf(data + data_offset_pkt_id, PacketIDData::long_id_size, false);
pid_send.write_next(buf);
if (op32)
{
Expand All @@ -102,13 +102,13 @@ class Crypto : public CryptoDCInstance
}

// for encrypt
void prepend_ad(Buffer &buf, const PacketIDAEADSend &pid_send) const
void prepend_ad(Buffer &buf, const PacketIDDataSend &pid_send) const
{
buf.prepend(data + data_offset_pkt_id, pid_send.length());
}

// for decrypt
Nonce(const Nonce &ref, const PacketIDAEADReceive &recv_pid, Buffer &buf, const unsigned char *op32)
Nonce(const Nonce &ref, const PacketIDDataReceive &recv_pid, Buffer &buf, const unsigned char *op32)
{
/* Copy opcode and tail of packet ID */
std::memcpy(data, ref.data, sizeof(data));
Expand All @@ -125,10 +125,10 @@ class Crypto : public CryptoDCInstance
}

// for decrypt
bool verify_packet_id(PacketIDAEADReceive &pid_recv, const PacketID::time_t now)
bool verify_packet_id(PacketIDDataReceive &pid_recv, const PacketIDControl::time_t now)
{
Buffer buf(data + data_offset_pkt_id, PacketIDAEAD::long_id_size, true);
const PacketIDAEAD pid = pid_recv.read_next(buf);
Buffer buf(data + data_offset_pkt_id, PacketIDData::long_id_size, true);
const PacketIDData pid = pid_recv.read_next(buf);
return pid_recv.test_add(pid, now); // verify packet ID
}

Expand All @@ -142,12 +142,12 @@ class Crypto : public CryptoDCInstance
return ad_op32 ? data : data + data_offset_pkt_id;
}

size_t ad_len(const PacketIDAEADSend &pid_send) const
size_t ad_len(const PacketIDDataSend &pid_send) const
{
return (ad_op32 ? op32_size : 0) + pid_send.length();
}

size_t ad_len(const PacketIDAEADReceive &pid_recv) const
size_t ad_len(const PacketIDDataReceive &pid_recv) const
{
return (ad_op32 ? op32_size : 0) + pid_recv.length();
}
Expand All @@ -168,15 +168,15 @@ class Crypto : public CryptoDCInstance
{
typename CRYPTO_API::CipherContextAEAD impl;
Nonce nonce;
PacketIDAEADSend pid_send{false};
PacketIDDataSend pid_send{false};
BufferAllocated work;
};

struct Decrypt
{
typename CRYPTO_API::CipherContextAEAD impl;
Nonce nonce;
PacketIDAEADReceive pid_recv{};
PacketIDDataReceive pid_recv{};
BufferAllocated work;
};

Expand All @@ -197,7 +197,7 @@ class Crypto : public CryptoDCInstance
// Encrypt/Decrypt

// returns true if packet ID is close to wrapping
bool encrypt(BufferAllocated &buf, const PacketID::time_t now, const unsigned char *op32) override
bool encrypt(BufferAllocated &buf, const unsigned char *op32) override
{
// only process non-null packets
if (buf.size())
Expand Down Expand Up @@ -251,7 +251,7 @@ class Crypto : public CryptoDCInstance
return e.pid_send.wrap_warning();
}

Error::Type decrypt(BufferAllocated &buf, const PacketID::time_t now, const unsigned char *op32) override
Error::Type decrypt(BufferAllocated &buf, const std::time_t now, const unsigned char *op32) override
{
// only process non-null packets
if (buf.size())
Expand Down Expand Up @@ -337,7 +337,7 @@ class Crypto : public CryptoDCInstance
const int recv_unit,
const SessionStats::Ptr &recv_stats_arg) override
{
e.pid_send = PacketIDAEADSend{dc_settings.use64bitPktCounter()};
e.pid_send = PacketIDDataSend{dc_settings.use64bitPktCounter()};
d.pid_recv.init(recv_name, recv_unit, dc_settings.use64bitPktCounter(), recv_stats_arg);
}

Expand Down
12 changes: 6 additions & 6 deletions openvpn/crypto/crypto_chm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,13 @@ class CryptoCHM : public CryptoDCInstance
// Encrypt/Decrypt

/* returns true if packet ID is close to wrapping */
bool encrypt(BufferAllocated &buf, const PacketID::time_t now, const unsigned char *op32) override
bool encrypt(BufferAllocated &buf, const unsigned char *op32) override
{
encrypt_.encrypt(buf, now);
encrypt_.encrypt(buf);
return encrypt_.pid_send.wrap_warning();
}

Error::Type decrypt(BufferAllocated &buf, const PacketID::time_t now, const unsigned char *op32) override
Error::Type decrypt(BufferAllocated &buf, const std::time_t now, const unsigned char *op32) override
{
return decrypt_.decrypt(buf, now);
}
Expand All @@ -90,10 +90,10 @@ class CryptoCHM : public CryptoDCInstance
const SessionStats::Ptr &recv_stats_arg) override
{
/* CBC encryption always uses short packet ID */
auto pid_form = PacketID::SHORT_FORM;
constexpr bool wide = false;

encrypt_.pid_send.init(pid_form);
decrypt_.pid_recv.init(pid_form, recv_name, recv_unit, recv_stats_arg);
encrypt_.pid_send = PacketIDDataSend{wide};
decrypt_.pid_recv.init(recv_name, recv_unit, wide, recv_stats_arg);
}

bool consider_compression(const CompressContext &comp_ctx) override
Expand Down
6 changes: 3 additions & 3 deletions openvpn/crypto/cryptodc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
#include <openvpn/common/rc.hpp>
#include <openvpn/frame/frame.hpp>
#include <openvpn/crypto/static_key.hpp>
#include <openvpn/crypto/packet_id.hpp>
#include <openvpn/crypto/packet_id_control.hpp>
#include <openvpn/crypto/cryptoalgs.hpp>
#include <openvpn/compress/compress.hpp>

Expand All @@ -48,9 +48,9 @@ class CryptoDCInstance : public RC<thread_unsafe_refcount>
// Encrypt/Decrypt

// returns true if packet ID is close to wrapping
virtual bool encrypt(BufferAllocated &buf, const PacketID::time_t now, const unsigned char *op32) = 0;
virtual bool encrypt(BufferAllocated &buf, const unsigned char *op32) = 0;

virtual Error::Type decrypt(BufferAllocated &buf, const PacketID::time_t now, const unsigned char *op32) = 0;
virtual Error::Type decrypt(BufferAllocated &buf, std::time_t now, const unsigned char *op32) = 0;

// Initialization

Expand Down
18 changes: 6 additions & 12 deletions openvpn/crypto/decrypt_chm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
#include <openvpn/crypto/cipher.hpp>
#include <openvpn/crypto/ovpnhmac.hpp>
#include <openvpn/crypto/static_key.hpp>
#include <openvpn/crypto/packet_id.hpp>
#include <openvpn/crypto/packet_id_control.hpp>
#include <openvpn/log/sessionstats.hpp>

namespace openvpn {
Expand All @@ -45,7 +45,7 @@ class DecryptCHM
public:
OPENVPN_SIMPLE_EXCEPTION(chm_unsupported_cipher_mode);

Error::Type decrypt(BufferAllocated &buf, const PacketID::time_t now)
Error::Type decrypt(BufferAllocated &buf, const std::time_t now)
{
// skip null packets
if (!buf.size())
Expand Down Expand Up @@ -118,19 +118,13 @@ class DecryptCHM
Frame::Ptr frame;
CipherContext<CRYPTO_API> cipher;
OvpnHMAC<CRYPTO_API> hmac;
PacketIDReceive pid_recv;
PacketIDDataReceive pid_recv;

private:
bool verify_packet_id(BufferAllocated &buf, const PacketID::time_t now)
bool verify_packet_id(BufferAllocated &buf, const std::time_t now)
{
// ignore packet ID if pid_recv is not initialized
if (pid_recv.initialized())
{
const PacketID pid = pid_recv.read_next(buf);
if (!pid_recv.test_add(pid, now, true)) // verify packet ID
return false;
}
return true;
const PacketIDData pid = pid_recv.read_next(buf);
return pid_recv.test_add(pid, now);
}

BufferAllocated work;
Expand Down
10 changes: 5 additions & 5 deletions openvpn/crypto/encrypt_chm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
#include <openvpn/crypto/cipher.hpp>
#include <openvpn/crypto/ovpnhmac.hpp>
#include <openvpn/crypto/static_key.hpp>
#include <openvpn/crypto/packet_id.hpp>
#include <openvpn/crypto/packet_id_data.hpp>

namespace openvpn {
template <typename CRYPTO_API>
Expand All @@ -44,7 +44,7 @@ class EncryptCHM
public:
OPENVPN_SIMPLE_EXCEPTION(chm_unsupported_cipher_mode);

void encrypt(BufferAllocated &buf, const PacketID::time_t now)
void encrypt(BufferAllocated &buf)
{
// skip null packets
if (!buf.size())
Expand All @@ -64,7 +64,7 @@ class EncryptCHM
rng->rand_bytes(iv_buf, iv_length);

// generate fresh outgoing packet ID and prepend to cleartext buffer
pid_send.write_next(buf, true, now);
pid_send.prepend_next(buf);
}
else
{
Expand Down Expand Up @@ -95,7 +95,7 @@ class EncryptCHM
else // no encryption
{
// generate fresh outgoing packet ID and prepend to cleartext buffer
pid_send.write_next(buf, true, now);
pid_send.prepend_next(buf);

// HMAC the cleartext
prepend_hmac(buf);
Expand All @@ -110,7 +110,7 @@ class EncryptCHM
Frame::Ptr frame;
CipherContext<CRYPTO_API> cipher;
OvpnHMAC<CRYPTO_API> hmac;
PacketIDSend pid_send;
PacketIDDataSend pid_send{false};

private:
// compute HMAC signature of data buffer,
Expand Down
Loading

0 comments on commit c78aaec

Please sign in to comment.