[Openvpn-devel] [M] Change in openvpn[master]: Add methods to read/write packet ids for epoch data

2024-11-30 Thread plaisthos (Code Review)
Attention is currently required from: flichtenheld, stipa.

plaisthos has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/803?usp=email )

Change subject: Add methods to read/write packet ids for epoch data
..


Patch Set 5:

(1 comment)

File src/openvpn/packet_id.c:

http://gerrit.openvpn.net/c/openvpn/+/803/comment/feb6ca8b_77e908f0 :
PS5, Line 667: pin->id = id & 0xull;
> shouldn't we use PACKET_ID_EPOCH_MAX here?
yes, will fix that. Technically it is a mask here but the mask is identical.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/803?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I2a104decdb1e77a460f7a6976bcd0560b67a07b5
Gerrit-Change-Number: 803
Gerrit-PatchSet: 5
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-CC: stipa 
Gerrit-Attention: flichtenheld 
Gerrit-Attention: stipa 
Gerrit-Comment-Date: Sat, 30 Nov 2024 13:06:25 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: stipa 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [M] Change in openvpn[master]: Change internal id of packet id to uint64

2024-11-30 Thread plaisthos (Code Review)
Attention is currently required from: flichtenheld, stipa.

plaisthos has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/802?usp=email )

Change subject: Change internal id of packet id to uint64
..


Patch Set 5:

(1 comment)

File src/openvpn/packet_id.c:

http://gerrit.openvpn.net/c/openvpn/+/802/comment/1a5815f5_02ca1ead :
PS5, Line 123:  * If time value increases, start a new sequence number 
sequence.
> sequence number sequence?
This is not a comment that I change but I agree that the wording is not very 
clear.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/802?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: If470af2eb456b2b10f9f2806933e026842188c42
Gerrit-Change-Number: 802
Gerrit-PatchSet: 5
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: stipa 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: flichtenheld 
Gerrit-Attention: stipa 
Gerrit-Comment-Date: Sat, 30 Nov 2024 13:01:39 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: stipa 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [M] Change in openvpn[master]: Use XOR instead of concatenation for calculation of IV from implicit IV

2024-11-30 Thread plaisthos (Code Review)
Attention is currently required from: cron2, flichtenheld, ordex, plaisthos.

Hello flichtenheld, ordex,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/797?usp=email

to look at the new patch set (#7).

The following approvals got outdated and were removed:
Code-Review+2 by ordex

The change is no longer submittable: Code-Review and checks~ChecksSubmitRule 
are unsatisfied now.


Change subject: Use XOR instead of concatenation for calculation of  IV from 
implicit IV
..

Use XOR instead of concatenation for calculation of  IV from implicit IV

This change prepares the extended packet id data where also the packet id
part of the IV will be derived using xor.  Using xor also in the AEAD
case where this degenerates to a concatenation allows using the same
IV generation code later.

Change-Id: I74216d776d3e0a8dc987ec7b1671c8e8dcccdbd6
Signed-off-by: Arne Schwabe 
---
M src/openvpn/crypto.c
M src/openvpn/crypto.h
M src/openvpn/ssl.c
M tests/unit_tests/openvpn/test_ssl.c
4 files changed, 32 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/97/797/7

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 67c5b04..d6cd256 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -94,12 +94,16 @@
 goto err;
 }

-/* Remainder of IV consists of implicit part (unique per session) */
-ASSERT(buf_write(&iv_buffer, ctx->implicit_iv, ctx->implicit_iv_len));
-ASSERT(iv_buffer.len == iv_len);
+/* Write packet id part of IV to work buffer */
+ASSERT(buf_write(&work, iv, packet_id_size(false)));

-/* Write explicit part of IV to work buffer */
-ASSERT(buf_write(&work, iv, iv_len - ctx->implicit_iv_len));
+/* This generates the IV by XORing the implicit part of the IV
+ * with the packet id already written to the iv buffer */
+for (int i = 0; i < iv_len; i++)
+{
+iv[i] = iv[i] ^ ctx->implicit_iv[i];
+}
+
 dmsg(D_PACKET_CONTENT, "ENCRYPT IV: %s", format_hex(iv, iv_len, 0, 
&gc));

 /* Init cipher_ctx with IV.  key & keylen are already initialized */
@@ -426,16 +430,21 @@
 {
 uint8_t iv[OPENVPN_MAX_IV_LENGTH] = { 0 };
 const int iv_len = cipher_ctx_iv_length(ctx->cipher);
-const size_t packet_iv_len = iv_len - ctx->implicit_iv_len;
+const size_t packet_iv_len = packet_id_size(false);

-ASSERT(ctx->implicit_iv_len <= iv_len);
-if (buf->len + ctx->implicit_iv_len < iv_len)
+if (buf->len < packet_iv_len)
 {
 CRYPT_ERROR("missing IV info");
 }

 memcpy(iv, BPTR(buf), packet_iv_len);
-memcpy(iv + packet_iv_len, ctx->implicit_iv, ctx->implicit_iv_len);
+
+/* This generates the IV by XORing the implicit part of the IV
+ * with the packet id already written to the iv buffer */
+for (int i = 0; i < iv_len; i++)
+{
+iv[i] = iv[i] ^ ctx->implicit_iv[i];
+}

 dmsg(D_PACKET_CONTENT, "DECRYPT IV: %s", format_hex(iv, iv_len, 0, 
&gc));

@@ -962,7 +971,6 @@
 hmac_ctx_free(ctx->hmac);
 ctx->hmac = NULL;
 }
-ctx->implicit_iv_len = 0;
 }

 void
@@ -1078,18 +1086,15 @@
 cipher_ctx_t *cipher = co->key_ctx_bi.encrypt.cipher;
 if (cipher_ctx_mode_aead(cipher))
 {
-size_t impl_iv_len = cipher_ctx_iv_length(cipher) - 
sizeof(packet_id_type);
 ASSERT(cipher_ctx_iv_length(cipher) <= OPENVPN_MAX_IV_LENGTH);
 ASSERT(cipher_ctx_iv_length(cipher) >= OPENVPN_AEAD_MIN_IV_LEN);

 /* Generate dummy implicit IV */
 ASSERT(rand_bytes(co->key_ctx_bi.encrypt.implicit_iv,
   OPENVPN_MAX_IV_LENGTH));
-co->key_ctx_bi.encrypt.implicit_iv_len = impl_iv_len;

 memcpy(co->key_ctx_bi.decrypt.implicit_iv,
co->key_ctx_bi.encrypt.implicit_iv, OPENVPN_MAX_IV_LENGTH);
-co->key_ctx_bi.decrypt.implicit_iv_len = impl_iv_len;
 }
 }

diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index 7d32353..531664d 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -163,6 +163,17 @@
 {
 cipher_ctx_t *cipher;   /**< Generic cipher %context. */
 hmac_ctx_t *hmac;   /**< Generic HMAC %context. */
+/**
+ * This implicit IV will be always XORed with the packet id that is sent on
+ * the wire to get the IV. For the common AEAD ciphers of AES-GCM and
+ * Chacha20-Poly1305, the length of the IV is 12 bytes (96 bits).
+ *
+ * For non-epoch 32bit packet id AEAD format we set the first 32
+ * bits of implicit_iv to 0.
+ * Xor with the packet id in this case works as concatenation:
+ * after xor the lower 32 bit of the IV are the packet id and
+ * the re

[Openvpn-devel] [M] Change in openvpn[master]: Trigger renegotiation of data key if getting close to the AEAD usage ...

2024-11-30 Thread plaisthos (Code Review)
Attention is currently required from: MaxF, cron2, flichtenheld.

Hello MaxF, flichtenheld,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/796?usp=email

to look at the new patch set (#8).

The following approvals got outdated and were removed:
Code-Review-1 by MaxF


Change subject: Trigger renegotiation of data key if getting close to the AEAD 
usage limit
..

Trigger renegotiation of data key if getting close to the AEAD usage limit

This implements the limitation of AEAD key usage[1] with a confidentiality
margin of 2^-57, the same as TLS 1.3.  In this implementation, unlike
TLS 1.3 that counts the number of records, we count the actual number of
packets and plaintext blocks. TLS 1.3 can reasonable assume that for
large data transfers, full records are used and therefore the maximum
record size of 2**14 (2*10 blocks) is used to calculate the number of
records before a new key needs to be used.

For a VPN like OpenVPN, the same calculation would either require using a
pessimistic assumption of using a MTU size of 65k which limits us to
2^24 packets, which equals only 24 GB with more common MTU/MSS of 1400
or requiring a dynamic calculation which includes the actual MTU that
we allow to send. For 1500 the calculation yields 2*29.4 which is a
quite significant higher number of packets (923 GB at 1400 MSS/MTU).

To avoid this dynamic calculation and also avoid needing to know the
MSS/MTU size in the crypto layer, this implementation foregoes the
simplification of counting just packets but will count blocks and packets
instead and determines the limit from that.

This also has the side effect that connections with a lot of small packets
(like TCP ACKs) mixed with large packets will be able to keep using the same
key much longer until requiring a renegotiation.

This patch will set the limit where to trigger the renegotiation at 7/8
of the recommended maximum value.

[1]  https://www.ietf.org/archive/id/draft-irtf-cfrg-aead-limits-08.html

Testing instructions:

The easiest way to test if this patch works as
intended is to manually change the return value of cipher_get_aead_limits
to some silly low value like 2048. After a bit of VPN traffic, a soft
reset should occur that indicates being over the

TLS: soft reset sec=41/3600 bytes=59720/-1 pkts=78/0 
aead_limit_send=1883/1792 aead_limit_recv=1937/1792

Here the send limit is over the limit (1792 = 2048 * 8/7).

Change-Id: I057f007577f10c6ac917ee4620ee3d2559187dc7
Signed-off-by: Arne Schwabe 
---
M Changes.rst
M src/openvpn/crypto.c
M src/openvpn/crypto.h
M src/openvpn/ssl.c
M src/openvpn/ssl_common.h
M tests/unit_tests/openvpn/test_crypto.c
6 files changed, 159 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/96/796/8

diff --git a/Changes.rst b/Changes.rst
index a6cc3be..34542a5 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -22,6 +22,14 @@

 For more details see [lwipovpn on 
Gihtub](https://github.com/OpenVPN/lwipovpn).

+Enforcement of AES-GCM usage limit
+OpenVPN will now enforce the usage limits on AES-GCM with the same
+confidentiality margin as TLS 1.3 does. This mean that renegotiation will
+be triggered after roughly 2^28 to 2^31 packets depending of the packet
+size. More details about usage limit of AES-GCM can be found here:
+
+https://datatracker.ietf.org/doc/draft-irtf-cfrg-aead-limits/
+
 Deprecated features
 ---
 ``secret`` support has been removed by default.
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 8f34eaa..67c5b04 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -134,6 +134,11 @@
 ASSERT(cipher_ctx_update(ctx->cipher, BEND(&work), &outlen, BPTR(buf), 
BLEN(buf)));
 ASSERT(buf_inc_len(&work, outlen));

+/* update number of plaintext blocks encrypted. Use the (x + (n-1))/n trick
+ * to round up the result to the number of blocks used */
+const int blocksize = AEAD_LIMIT_BLOCKSIZE;
+opt->key_ctx_bi.encrypt.plaintext_blocks += (outlen + (blocksize - 
1))/blocksize;
+
 /* Flush the encryption buffer */
 ASSERT(cipher_ctx_final(ctx->cipher, BEND(&work), &outlen));
 ASSERT(buf_inc_len(&work, outlen));
@@ -321,6 +326,37 @@
 }
 }

+int64_t
+cipher_get_aead_limits(const char *ciphername)
+{
+if (!cipher_kt_mode_aead(ciphername))
+{
+return 0;
+}
+
+if (cipher_kt_name(ciphername) == cipher_kt_name("CHACHA20-POLY1305"))
+{
+return 0;
+}
+
+/* Assume all other ciphers require the limit */
+
+/* We focus here on the equation
+ *
+ *   q + s <= p^(1/2) * 2^(129/2) - 1
+ *
+ * as is the one that is limiting us.
+ *
+ *  With p = 2^-57 this becomes
+ *
+ *  q + s <= (2^36 - 1)
+ *
+ */
+int64_t rs = (1ull << 36) - 1;
+
+return rs;
+}
+
 bool
 crypto_check_replay(struct crypto_o

[Openvpn-devel] [L] Change in openvpn[master]: Implement HKDF expand function based on RFC 8446

2024-11-30 Thread plaisthos (Code Review)
Attention is currently required from: MaxF, flichtenheld, plaisthos.

Hello MaxF, flichtenheld,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/798?usp=email

to look at the new patch set (#7).

The following approvals got outdated and were removed:
Code-Review+2 by MaxF, Code-Review-1 by flichtenheld


Change subject: Implement HKDF expand function based on RFC 8446
..

Implement HKDF expand function based on RFC 8446

Use crypto_epoch.c/h for the new functions since they are
linked to the epoch key usage in OpenVPN.

Change-Id: I3a1c6561f4d9a69e2a441d49dff620b4258a1bcc
Signed-off-by: Arne Schwabe 
---
M CMakeLists.txt
M src/openvpn/Makefile.am
A src/openvpn/crypto_epoch.c
A src/openvpn/crypto_epoch.h
M src/openvpn/crypto_mbedtls.h
M tests/unit_tests/openvpn/Makefile.am
M tests/unit_tests/openvpn/test_crypto.c
7 files changed, 433 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/98/798/7

diff --git a/CMakeLists.txt b/CMakeLists.txt
index ca58cd7..61f0cc5 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -387,6 +387,8 @@
 src/openvpn/crypto.c
 src/openvpn/crypto.h
 src/openvpn/crypto_backend.h
+src/openvpn/crypto_epoch.c
+src/openvpn/crypto_epoch.h
 src/openvpn/crypto_openssl.c
 src/openvpn/crypto_openssl.h
 src/openvpn/crypto_mbedtls.c
@@ -715,6 +717,7 @@
 target_sources(test_crypto PRIVATE
 src/openvpn/crypto_mbedtls.c
 src/openvpn/crypto_openssl.c
+src/openvpn/crypto_epoch.c
 src/openvpn/crypto.c
 src/openvpn/otime.c
 src/openvpn/packet_id.c
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index ecb2bcf..d6d6592 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -53,6 +53,7 @@
crypto.c crypto.h crypto_backend.h \
crypto_openssl.c crypto_openssl.h \
crypto_mbedtls.c crypto_mbedtls.h \
+   crypto_epoch.c crypto_epoch.h \
dco.c dco.h dco_internal.h \
dco_freebsd.c dco_freebsd.h \
dco_linux.c dco_linux.h \
diff --git a/src/openvpn/crypto_epoch.c b/src/openvpn/crypto_epoch.c
new file mode 100644
index 000..667e12a
--- /dev/null
+++ b/src/openvpn/crypto_epoch.c
@@ -0,0 +1,114 @@
+/*
+ *  OpenVPN -- An application to securely tunnel IP networks
+ * over a single TCP/UDP port, with support for SSL/TLS-based
+ * session authentication and key exchange,
+ * packet encryption, packet authentication, and
+ * packet compression.
+ *
+ *  Copyright (C) 2024 OpenVPN Inc 
+ *  Copyright (C) 2024 Arne Schwabe 
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include 
+#include "crypto_backend.h"
+#include "buffer.h"
+#include "integer.h"
+
+void
+ovpn_hkdf_expand(const uint8_t *secret,
+ const uint8_t *info, int info_len,
+ uint8_t *out, int out_len)
+{
+hmac_ctx_t *hmac_ctx = hmac_ctx_new();
+hmac_ctx_init(hmac_ctx, secret, "SHA256");
+
+const int digest_size = SHA256_DIGEST_LENGTH;
+
+/* T(0) = empty string */
+uint8_t t_prev[SHA256_DIGEST_LENGTH];
+int t_prev_len = 0;
+
+for (uint8_t block = 1; (block - 1) * digest_size < out_len; block++)
+{
+hmac_ctx_reset(hmac_ctx);
+
+/* calculate T(block) */
+hmac_ctx_update(hmac_ctx, t_prev, t_prev_len);
+hmac_ctx_update(hmac_ctx, info, info_len);
+hmac_ctx_update(hmac_ctx, &block, 1);
+hmac_ctx_final(hmac_ctx, t_prev);
+t_prev_len = digest_size;
+
+/* Copy a full hmac output or remaining bytes */
+int out_offset = (block - 1) * digest_size;
+int copylen = min_int(digest_size, out_len - out_offset);
+
+memcpy(out + out_offset, t_prev, copylen);
+}
+hmac_ctx_cleanup(hmac_ctx);
+hmac_ctx_free(hmac_ctx);
+}
+
+bool
+ovpn_expand_label(const uint8_t *secret, size_t secret_len,
+  const uint8_t *label, size_t label_len,
+  const uint8_t *context, size_t context_len,
+  uint8_t *out, uint16_t out_len)
+{
+if (secret_len != 32 || label_len > 250 || context_len > 255
+|| label_len < 1)
+{
+/* Our 

[Openvpn-devel] [M] Change in openvpn[master]: Add methods to read/write packet ids for epoch data

2024-11-30 Thread plaisthos (Code Review)
Attention is currently required from: flichtenheld, stipa.

Hello flichtenheld,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/803?usp=email

to look at the new patch set (#6).


Change subject: Add methods to read/write packet ids for epoch data
..

Add methods to read/write packet ids for epoch data

Change-Id: I2a104decdb1e77a460f7a6976bcd0560b67a07b5
Signed-off-by: Arne Schwabe 
---
M src/openvpn/packet_id.c
M src/openvpn/packet_id.h
M tests/unit_tests/openvpn/test_packet_id.c
3 files changed, 141 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/03/803/6

diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c
index 117c95f..e3a29f5 100644
--- a/src/openvpn/packet_id.c
+++ b/src/openvpn/packet_id.c
@@ -120,7 +120,8 @@
 int64_t diff;

 /*
- * If time value increases, start a new sequence number sequence.
+ * If time value increases, start a new sequence list of number
+ * sequence for the new time point.
  */
 if (!CIRC_LIST_SIZE(p->seq_list)
 || pin->time > p->time
@@ -343,6 +344,21 @@
 return true;
 }

+static bool
+packet_id_send_update_epoch(struct packet_id_send *p)
+{
+if (!p->time)
+{
+p->time = now;
+}
+if (p->id == PACKET_ID_EPOCH_MAX)
+{
+return false;
+}
+p->id++;
+return true;
+}
+
 bool
 packet_id_write(struct packet_id_send *p, struct buffer *buf, bool long_form,
 bool prepend)
@@ -634,4 +650,46 @@
 gc_free(&gc);
 }

+uint16_t
+packet_id_read_epoch(struct packet_id_net *pin, struct buffer *buf)
+{
+uint64_t packet_id;
+
+
+if (!buf_read(buf, &packet_id, sizeof(packet_id)))
+{
+return 0;
+}
+
+uint64_t id = ntohll(packet_id);
+/* top most 16 bits */
+uint16_t epoch = id >> 48;
+
+pin->id = id & PACKET_ID_MASK;
+return epoch;
+}
+
+bool
+packet_id_write_epoch(struct packet_id_send *p, uint16_t epoch, struct buffer 
*buf)
+{
+if (!packet_id_send_update_epoch(p))
+{
+return false;
+}
+
+/* First 16 bits of packet id is the epoch
+ *
+ * Next 48 bits are the per-epoch packet id counter. Since we are
+ * writing a big endian counter, the last 6 bytes of the 64 bit
+ * integer contain our counter but the upper two bytes are always 0 */
+
+uint64_t net_id = ((uint64_t) epoch) << 48 | p->id;
+
+/* convert to network order */
+net_id = htonll(net_id);
+
+return buf_write(buf, &net_id, sizeof(net_id));
+}
+
+
 #endif /* ifdef ENABLE_DEBUG */
diff --git a/src/openvpn/packet_id.h b/src/openvpn/packet_id.h
index d8a3e1a..f6b1463 100644
--- a/src/openvpn/packet_id.h
+++ b/src/openvpn/packet_id.h
@@ -44,7 +44,10 @@
  * These are ephemeral and are never saved to a file.
  */
 typedef uint32_t packet_id_type;
-#define PACKET_ID_MAX UINT32_MAX
+#define PACKET_ID_MAXUINT32_MAX
+#define PACKET_ID_EPOCH_MAX  0xull
+/** Maks of the bits that make out the epoch in the packet counter */
+#define PACKET_ID_MASK   0xull
 typedef uint32_t net_time_t;

 /*
@@ -158,7 +161,8 @@
  * includes a timestamp as well.
  *
  * An epoch packet-id is a 16 bit epoch
- * counter plus a 48 per-epoch packet-id
+ * counter plus a 48 per-epoch packet-id.
+ *
  *
  * Long packet-ids are used as IVs for
  * CFB/OFB ciphers and for control channel
@@ -321,4 +325,25 @@
 }
 }

+/**
+ * Writes the packet ID containing both the epoch and the packet id to the
+ * buffer specified by buf.
+ * @param p packed id send structure to use for the packet id
+ * @param epoch epoch to write to the packet
+ * @param buf   buffer to write the packet id/epcoh to
+ * @return  false if the packet id space is exhausted and cannot be 
written
+ */
+bool
+packet_id_write_epoch(struct packet_id_send *p, uint16_t epoch, struct buffer 
*buf);
+
+/**
+ * Reads the packet ID containing both the epoch and the per-epoch counter
+ * from the buf.  Will return 0 as epoch id if there is any error.
+ * @param p   packet_id struct to popupulate with the on-wire counter
+ * @param buf buffer to read the packet id from.
+ * @return0 for an error/invalid id, epoch otherwise
+ */
+uint16_t
+packet_id_read_epoch(struct packet_id_net *p, struct buffer *buf);
+
 #endif /* PACKET_ID_H */
diff --git a/tests/unit_tests/openvpn/test_packet_id.c 
b/tests/unit_tests/openvpn/test_packet_id.c
index d918985..a24580b 100644
--- a/tests/unit_tests/openvpn/test_packet_id.c
+++ b/tests/unit_tests/openvpn/test_packet_id.c
@@ -44,6 +44,7 @@
 } test_buf_data;
 struct buffer test_buf;
 struct packet_id_send pis;
+struct gc_arena gc;
 };

 static int
@@ -59,6 +60,7 @@

 data->test_buf.data = (void *) &data->test_buf_data;
 data->test_buf.capacity = sizeof(data->test_buf_data);
+data->

[Openvpn-devel] [L] Change in openvpn[master]: Implement methods to generate and manage OpenVPN Epoch keys

2024-11-30 Thread plaisthos (Code Review)
Attention is currently required from: flichtenheld.

Hello flichtenheld, 

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/804?usp=email

to look at the new patch set (#6).


Change subject: Implement methods to generate and manage OpenVPN Epoch keys
..

Implement methods to generate and manage OpenVPN Epoch keys

This implements functions that allow these keys to be generated and
managed. It does not yet implement using them for the data channel.

Change-Id: Id7d6a576ca8c9560cb2dfae82fc62175820e9b80
Signed-off-by: Arne Schwabe 
---
M src/openvpn/crypto.c
M src/openvpn/crypto.h
M src/openvpn/crypto_epoch.c
M src/openvpn/crypto_epoch.h
M src/openvpn/packet_id.c
M src/openvpn/packet_id.h
M src/openvpn/ssl_common.h
M tests/unit_tests/openvpn/test_crypto.c
8 files changed, 774 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/804/6

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 4b16630..4d6d83a 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -909,14 +909,27 @@
 if (cipher_ctx_mode_aead(ctx->cipher))
 {
 size_t impl_iv_len = 0;
+size_t impl_iv_offset = 0;
 ASSERT(cipher_ctx_iv_length(ctx->cipher) >= OPENVPN_AEAD_MIN_IV_LEN);
-impl_iv_len = cipher_ctx_iv_length(ctx->cipher) - 
sizeof(packet_id_type);
+
+/* Epoch keys use XOR of full IV length with the packet id to generate
+ * IVs. Old data format uses concatenation instead (XOR with 0 for the
+ * first 4 bytes (sizeof(packet_id_type) */
+if (key->epoch)
+{
+impl_iv_len = cipher_ctx_iv_length(ctx->cipher);
+impl_iv_offset = 0;
+}
+else
+{
+impl_iv_len = cipher_ctx_iv_length(ctx->cipher) - 
sizeof(packet_id_type);
+impl_iv_offset = sizeof(packet_id_type);
+}
 ASSERT(impl_iv_len <= OPENVPN_MAX_IV_LENGTH);
 ASSERT(impl_iv_len <= MAX_HMAC_KEY_LENGTH);
 ASSERT(impl_iv_len <= key->hmac_size);
 CLEAR(ctx->implicit_iv);
-/* The first bytes of the IV are filled with the packet id */
-memcpy(ctx->implicit_iv + sizeof(packet_id_type), key->hmac, 
impl_iv_len);
+memcpy(ctx->implicit_iv + impl_iv_offset, key->hmac, impl_iv_len);
 }
 }

@@ -965,6 +978,7 @@
  hmac_ctx_size(ctx->hmac));

 }
+ctx->epoch = key->epoch;
 gc_free(&gc);
 }
 
@@ -977,6 +991,7 @@
 snprintf(log_prefix, sizeof(log_prefix), "Outgoing %s", name);
 init_key_ctx(ctx, key_params, kt, OPENVPN_OP_ENCRYPT, log_prefix);
 key_ctx_update_implicit_iv(ctx, key_params);
+ctx->epoch = key_params->epoch;
 }

 void
@@ -988,6 +1003,7 @@
 snprintf(log_prefix, sizeof(log_prefix), "Incoming %s", name);
 init_key_ctx(ctx, key_params, kt, OPENVPN_OP_DECRYPT, log_prefix);
 key_ctx_update_implicit_iv(ctx, key_params);
+ctx->epoch = key_params->epoch;
 }

 void
@@ -1025,6 +1041,7 @@
 }
 CLEAR(ctx->implicit_iv);
 ctx->plaintext_blocks = 0;
+ctx->epoch = 0;
 }

 void
diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index 726c07c..805bbf3 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -170,6 +170,9 @@

 /** Number of bytes set in the HMac key material */
 int hmac_size;
+
+/** the epoch of the key is if it was generated as epoch data key material 
*/
+uint16_t epoch;
 };

 /**
@@ -180,6 +183,11 @@
 void
 key_parameters_from_key(struct key_parameters *key_params, const struct key 
*key);

+struct epoch_key {
+uint8_t epoch_key[SHA256_DIGEST_LENGTH];
+uint16_t epoch;
+};
+
 /**
  * Container for one set of cipher and/or HMAC contexts.
  * @ingroup control_processor
@@ -206,6 +214,10 @@
  * with the current key in number of 128 bit blocks (only used for
  * AEAD ciphers) */
 uint64_t plaintext_blocks;
+/** OpenVPN data channel epoch, this variable holds the
+ *  epoch number that is key belongs to. Note that epoch 0 is not used
+ *  and epoch is always non-zero for epoch key contexts */
+uint16_t epoch;
 };

 #define KEY_DIRECTION_BIDIRECTIONAL 0 /* same keys for both directions */
@@ -275,6 +287,39 @@
 /**< OpenSSL cipher and HMAC contexts for
  *   both sending and receiving
  *   directions. */
+
+/** last epoch_key used for generation of the current send data keys.
+ * As invariant, the epoch of epoch_key_send is always kept >= the epoch of
+ * epoch_key_recv */
+struct epoch_key epoch_key_send;
+
+/** epoch_key used for the highest receive epoch keys */
+struct epoch_key epoch_key_recv;
+
+/** the key_type that is used to generate the epoch keys */
+struct key_type epoch_key_type;
+
+/** This limit for AEAD cipher, this is the sum of packets + blocks
+ * that are allowed to be used. Will switch to a new epoch if this
+ * li

[Openvpn-devel] [M] Change in openvpn[master]: Change API of init_key_ctx to use struct key_paramters

2024-11-30 Thread plaisthos (Code Review)
Attention is currently required from: flichtenheld.

Hello flichtenheld, 

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/801?usp=email

to look at the new patch set (#6).


Change subject: Change API of init_key_ctx to use struct key_paramters
..

Change API of init_key_ctx to use struct key_paramters

This introduces a new structure key_paramters. The reason is that the
current struct serves both as an internal struct as well as an
on-wire/in-file format. Separate these two different usages to allow
extending the struct.

Change-Id: I4a981c5a70717e2276d89bf83a06c7fdbe6712d7
Signed-off-by: Arne Schwabe 
---
M src/openvpn/auth_token.c
M src/openvpn/crypto.c
M src/openvpn/crypto.h
M src/openvpn/tls_crypt.c
M tests/unit_tests/openvpn/test_auth_token.c
M tests/unit_tests/openvpn/test_tls_crypt.c
6 files changed, 90 insertions(+), 41 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/01/801/6

diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index 192c7c2..3cf55e8 100644
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -152,7 +152,10 @@
 {
 msg(M_FATAL, "ERROR: not enough data in auth-token secret");
 }
-init_key_ctx(key_ctx, &key, &kt, false, "auth-token secret");
+
+struct key_parameters key_params;
+key_parameters_from_key(&key_params, &key);
+init_key_ctx(key_ctx, &key_params, &kt, false, "auth-token secret");

 free_buf(&server_secret_key);
 }
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 7c6cfeb..4b16630 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -892,17 +892,18 @@
 }

 /**
- * Update the implicit IV for a key_ctx_bi based on TLS session ids and cipher
+ * Update the implicit IV for a key_ctx based on TLS session ids and cipher
  * used.
  *
- * Note that the implicit IV is based on the HMAC key, but only in AEAD modes
- * where the HMAC key is not used for an actual HMAC.
+ * Note that the implicit IV is based on the HMAC key of the \c key parameter,
+ * but only in AEAD modes where the HMAC key is not used for an actual HMAC.
  *
  * @param ctx   Encrypt/decrypt key context
- * @param key   key, hmac part used to calculate implicit IV
+ * @param key   key parameters holding the key and hmac/
+ *  implicit iv used to calculate implicit IV
  */
 static void
-key_ctx_update_implicit_iv(struct key_ctx *ctx, const struct key *key)
+key_ctx_update_implicit_iv(struct key_ctx *ctx, const struct key_parameters 
*key)
 {
 /* Only use implicit IV in AEAD cipher mode, where HMAC key is not used */
 if (cipher_ctx_mode_aead(ctx->cipher))
@@ -912,6 +913,7 @@
 impl_iv_len = cipher_ctx_iv_length(ctx->cipher) - 
sizeof(packet_id_type);
 ASSERT(impl_iv_len <= OPENVPN_MAX_IV_LENGTH);
 ASSERT(impl_iv_len <= MAX_HMAC_KEY_LENGTH);
+ASSERT(impl_iv_len <= key->hmac_size);
 CLEAR(ctx->implicit_iv);
 /* The first bytes of the IV are filled with the packet id */
 memcpy(ctx->implicit_iv + sizeof(packet_id_type), key->hmac, 
impl_iv_len);
@@ -920,7 +922,7 @@

 /* given a key and key_type, build a key_ctx */
 void
-init_key_ctx(struct key_ctx *ctx, const struct key *key,
+init_key_ctx(struct key_ctx *ctx, const struct key_parameters *key,
  const struct key_type *kt, int enc,
  const char *prefix)
 {
@@ -928,7 +930,7 @@
 CLEAR(*ctx);
 if (cipher_defined(kt->cipher))
 {
-
+ASSERT(key->cipher_size >= cipher_kt_key_size(kt->cipher));
 ctx->cipher = cipher_ctx_new();
 cipher_ctx_init(ctx->cipher, key->cipher, kt->cipher, enc);

@@ -943,8 +945,10 @@
  cipher_kt_iv_size(kt->cipher));
 warn_insecure_key_type(ciphername);
 }
+
 if (md_defined(kt->digest))
 {
+ASSERT(key->hmac_size >= md_kt_size(kt->digest));
 ctx->hmac = hmac_ctx_new();
 hmac_ctx_init(ctx->hmac, key->hmac, kt->digest);

@@ -965,41 +969,43 @@
 }

 void
-init_key_bi_ctx_send(struct key_ctx *ctx, const struct key2 *key2,
- int key_direction, const struct key_type *kt, const char 
*name)
+init_key_bi_ctx_send(struct key_ctx *ctx, const struct key_parameters 
*key_params,
+ const struct key_type *kt, const char *name)
 {
 char log_prefix[128] = { 0 };
-struct key_direction_state kds;
-
-key_direction_state_init(&kds, key_direction);

 snprintf(log_prefix, sizeof(log_prefix), "Outgoing %s", name);
-init_key_ctx(ctx, &key2->keys[kds.out_key], kt,
- OPENVPN_OP_ENCRYPT, log_prefix);
-key_ctx_update_implicit_iv(ctx, &key2->keys[kds.out_key]);
+init_key_ctx(ctx, key_params, kt, OPENVPN_OP_ENCRYPT, log_prefix);
+key_ctx_update_implicit_iv(ctx, key_params);
 }

 void
-init_key_bi_ctx_recv(struct key_ctx *c

[Openvpn-devel] [M] Change in openvpn[master]: Change API of init_key_ctx to use struct key_paramters

2024-11-30 Thread plaisthos (Code Review)
Attention is currently required from: flichtenheld.

plaisthos has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/801?usp=email )

Change subject: Change API of init_key_ctx to use struct key_paramters
..


Patch Set 5:

(6 comments)

Patchset:

PS5:
thanks for the review


File src/openvpn/crypto.h:

http://gerrit.openvpn.net/c/openvpn/+/801/comment/8d53de3a_4d1c5e54 :
PS5, Line 158: /** internal structure similar to struct key that hold key 
information
> "holds"
Done


File src/openvpn/crypto.c:

http://gerrit.openvpn.net/c/openvpn/+/801/comment/ce856d23_fcfbd114 :
PS5, Line 902: ,
> Needs update
Done


http://gerrit.openvpn.net/c/openvpn/+/801/comment/a387265a_62cbf33e :
PS5, Line 915: impl_iv_len
> Change to `ASSERT(impl_iv_len <= key->hmac_size);` to keep consistency
Done


http://gerrit.openvpn.net/c/openvpn/+/801/comment/0fbf3998_33a8a7fa :
PS5, Line 1128: key_params->cipher_size = MAX_HMAC_KEY_LENGTH;
> `MAX_CIPHER_KEY_LENGTH`
Acknowledged


http://gerrit.openvpn.net/c/openvpn/+/801/comment/e070181c_273bb98a :
PS5, Line 1129: MAX_CIPHER_KEY_LENGTH
> `MAX_HMAC_KEY_LENGTH`
Acknowledged



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/801?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I4a981c5a70717e2276d89bf83a06c7fdbe6712d7
Gerrit-Change-Number: 801
Gerrit-PatchSet: 5
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Sat, 30 Nov 2024 12:41:23 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [M] Change in openvpn[master]: Trigger renegotiation of data key if getting close to the AEAD usage ...

2024-11-30 Thread plaisthos (Code Review)
Attention is currently required from: MaxF, cron2, flichtenheld.

plaisthos has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/796?usp=email )

Change subject: Trigger renegotiation of data key if getting close to the AEAD 
usage limit
..


Patch Set 7:

(2 comments)

File src/openvpn/crypto.h:

http://gerrit.openvpn.net/c/openvpn/+/796/comment/425b7b90_d202b3d9 :
PS7, Line 169: /** Counter for the number of plaintext encrypted using this 
cipher
 :  * in number of 128 bit blocks (only used for AEAD ciphers) 
*/
> This looks a bit garbled. […]
Done


File src/openvpn/crypto.c:

http://gerrit.openvpn.net/c/openvpn/+/796/comment/3b5c18b9_d1b170ca :
PS7, Line 347: q <= (p^(1/2) * 2^(129/2) - 1) / (L + 1)
> We're not doing anything with this inequality. […]
Yes, I will remove that one. The earlier version used L but it is easier do the 
block counting that actually figuring out L.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/796?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I057f007577f10c6ac917ee4620ee3d2559187dc7
Gerrit-Change-Number: 796
Gerrit-PatchSet: 7
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: MaxF 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: cron2 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: cron2 
Gerrit-Attention: flichtenheld 
Gerrit-Attention: MaxF 
Gerrit-Comment-Date: Sat, 30 Nov 2024 12:46:35 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: MaxF 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [M] Change in openvpn[master]: Trigger renegotiation of data key if getting close to the AEAD usage ...

2024-11-30 Thread plaisthos (Code Review)
Attention is currently required from: MaxF, cron2, flichtenheld.

plaisthos has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/796?usp=email )

Change subject: Trigger renegotiation of data key if getting close to the AEAD 
usage limit
..


Patch Set 7:

(1 comment)

Patchset:

PS7:
> The limit matches the one in the RFC for p = 2^57. […]
I will add that as seperate commit later in the patch (to be sent, it is not 
there yet). We will probably aim for the 2^36 limits that also TLS uses.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/796?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I057f007577f10c6ac917ee4620ee3d2559187dc7
Gerrit-Change-Number: 796
Gerrit-PatchSet: 7
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: MaxF 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: cron2 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: cron2 
Gerrit-Attention: flichtenheld 
Gerrit-Attention: MaxF 
Gerrit-Comment-Date: Sat, 30 Nov 2024 12:48:30 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: MaxF 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel