Menu
▾
▴
openvpn-devel
|
From: plaisthos (C. Review) <ge...@op...> - 2025-09-15 17:39:18
|
Attention is currently required from: flichtenheld.
Hello flichtenheld,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1190?usp=email
to review the following change.
Change subject: Clarify some code in epoch with better comments
......................................................................
Clarify some code in epoch with better comments
Change-Id: I34e6b680618a52003d8408852d415c8aeac01feb
Signed-off-by: Arne Schwabe <ar...@rf...>
---
M src/openvpn/crypto.c
M src/openvpn/crypto.h
2 files changed, 5 insertions(+), 2 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/90/1190/1
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 4c0f684..873fda5 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -97,6 +97,9 @@
/* IV starts with packet id to make the IV unique for packet */
if (use_epoch_data_format)
{
+ /* Note this does not check aead_usage_limit but overstepping it by a few
+ * extra block in by one extra write, is not affecting the security margin,
+ * next iteration/call to epoch_check_send_iterate will iterate the epoch */
if (!packet_id_write_epoch(&opt->packet_id.send, ctx->epoch, &iv_buffer))
{
msg(D_CRYPT_ERRORS, "ENCRYPT ERROR: packet ID roll over");
diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index efd7f60..0f0db13 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -298,7 +298,7 @@
/** 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 */
+ * key_ctx_bi.decrypt.epoch */
struct epoch_key epoch_key_send;
/** epoch_key used for the highest receive epoch keys */
@@ -309,7 +309,7 @@
/** The 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
- * limit is reached*/
+ * limit is reached. */
uint64_t aead_usage_limit;
/** Keeps the future epoch data keys for decryption. The current one
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1190?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: I34e6b680618a52003d8408852d415c8aeac01feb
Gerrit-Change-Number: 1190
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newchange
|
|
From: flichtenheld (C. Review) <ge...@op...> - 2025-09-16 13:47:00
|
Attention is currently required from: plaisthos. flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1190?usp=email ) Change subject: Clarify some code in epoch with better comments ...................................................................... Patch Set 1: Code-Review-1 (1 comment) File src/openvpn/crypto.c: http://gerrit.openvpn.net/c/openvpn/+/1190/comment/a9ff5784_41583cb3 : PS1, Line 101: * extra block in by one extra write, is not affecting the security margin, remove "by" pluralize "blocks" remove comma after "write" -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1190?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: I34e6b680618a52003d8408852d415c8aeac01feb Gerrit-Change-Number: 1190 Gerrit-PatchSet: 1 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Comment-Date: Tue, 16 Sep 2025 13:46:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
|
From: plaisthos (C. Review) <ge...@op...> - 2025-12-02 15:36:53
|
Attention is currently required from: flichtenheld. plaisthos has posted comments on this change by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/1190?usp=email ) Change subject: Clarify some code in epoch with better comments ...................................................................... Patch Set 1: (1 comment) File src/openvpn/crypto.c: http://gerrit.openvpn.net/c/openvpn/+/1190/comment/b8e6068f_9a0833b5?usp=email : PS1, Line 101: * extra block in by one extra write, is not affecting the security margin, > remove "by" […] Acknowledged -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1190?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I34e6b680618a52003d8408852d415c8aeac01feb Gerrit-Change-Number: 1190 Gerrit-PatchSet: 1 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Tue, 02 Dec 2025 15:36:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: flichtenheld <fr...@li...> |
|
From: plaisthos (C. Review) <ge...@op...> - 2025-12-02 15:37:16
|
Attention is currently required from: flichtenheld.
Hello flichtenheld,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1190?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review-1 by flichtenheld
Change subject: Clarify some code in epoch with better comments
......................................................................
Clarify some code in epoch with better comments
Change-Id: I34e6b680618a52003d8408852d415c8aeac01feb
Signed-off-by: Arne Schwabe <ar...@rf...>
---
M src/openvpn/crypto.c
M src/openvpn/crypto.h
2 files changed, 8 insertions(+), 2 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/90/1190/2
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 8049b3a..d6b8841 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -97,6 +97,12 @@
/* IV starts with packet id to make the IV unique for packet */
if (use_epoch_data_format)
{
+ /* Note this does not check aead_usage_limit but can overstep it by
+ * a few extra blocks in one extra write. This is not affecting the
+ * security margin as these extra blocks are on a completely
+ * different order of magnitude than the security margin.
+ * The next iteration/call to epoch_check_send_iterate will
+ * iterate the epoch */
if (!packet_id_write_epoch(&opt->packet_id.send, ctx->epoch, &iv_buffer))
{
msg(D_CRYPT_ERRORS, "ENCRYPT ERROR: packet ID roll over");
diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index 72c6821..3842615 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -298,7 +298,7 @@
/** 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 */
+ * key_ctx_bi.decrypt.epoch */
struct epoch_key epoch_key_send;
/** epoch_key used for the highest receive epoch keys */
@@ -309,7 +309,7 @@
/** The 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
- * limit is reached*/
+ * limit is reached. */
uint64_t aead_usage_limit;
/** Keeps the future epoch data keys for decryption. The current one
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1190?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I34e6b680618a52003d8408852d415c8aeac01feb
Gerrit-Change-Number: 1190
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>
|
|
From: flichtenheld (C. Review) <ge...@op...> - 2025-12-03 11:33:23
|
Attention is currently required from: plaisthos. flichtenheld has posted comments on this change by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/1190?usp=email ) Change subject: Clarify some code in epoch with better comments ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1190?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I34e6b680618a52003d8408852d415c8aeac01feb Gerrit-Change-Number: 1190 Gerrit-PatchSet: 2 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Comment-Date: Wed, 03 Dec 2025 11:33:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes |
|
From: Gert D. <ge...@gr...> - 2025-12-03 12:57:50
|
From: Arne Schwabe <ar...@rf...> Change-Id: I34e6b680618a52003d8408852d415c8aeac01feb Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Frank Lichtenheld <fr...@li...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1190 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1190 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): Frank Lichtenheld <fr...@li...> diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 8049b3a..d6b8841 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -97,6 +97,12 @@ /* IV starts with packet id to make the IV unique for packet */ if (use_epoch_data_format) { + /* Note this does not check aead_usage_limit but can overstep it by + * a few extra blocks in one extra write. This is not affecting the + * security margin as these extra blocks are on a completely + * different order of magnitude than the security margin. + * The next iteration/call to epoch_check_send_iterate will + * iterate the epoch */ if (!packet_id_write_epoch(&opt->packet_id.send, ctx->epoch, &iv_buffer)) { msg(D_CRYPT_ERRORS, "ENCRYPT ERROR: packet ID roll over"); diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h index 72c6821..3842615 100644 --- a/src/openvpn/crypto.h +++ b/src/openvpn/crypto.h @@ -298,7 +298,7 @@ /** 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 */ + * key_ctx_bi.decrypt.epoch */ struct epoch_key epoch_key_send; /** epoch_key used for the highest receive epoch keys */ @@ -309,7 +309,7 @@ /** The 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 - * limit is reached*/ + * limit is reached. */ uint64_t aead_usage_limit; /** Keeps the future epoch data keys for decryption. The current one |
|
From: Gert D. <ge...@gr...> - 2025-12-04 13:23:56
|
Improved documentation is always welcome :-)
I have taken Antonio's last-minute complaint into account and adjusted
the end-of-comment '*/' bits to live on their own line. I did not touch
other comments in these files, just those that were touched in Arne's
patch.
Since this is just comments, I've only done a sanity check compile to
ensure I didn't fat-finger one of my changes.
Your patch has been applied to the master branch.
commit c282b62f9072b513c0fa8eef49fd8fc7c47afd15
Author: Arne Schwabe
Date: Wed Dec 3 13:57:34 2025 +0100
Clarify some code in epoch with better comments
Signed-off-by: Arne Schwabe <ar...@rf...>
Acked-by: Frank Lichtenheld <fr...@li...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1190
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg34829.html
Signed-off-by: Gert Doering <ge...@gr...>
--
kind regards,
Gert Doering
|
|
From: ordex (C. Review) <ge...@op...> - 2025-12-04 12:47:35
|
Attention is currently required from: plaisthos. ordex has posted comments on this change by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/1190?usp=email ) Change subject: Clarify some code in epoch with better comments ...................................................................... Patch Set 2: Code-Review-1 (2 comments) File src/openvpn/crypto.h: http://gerrit.openvpn.net/c/openvpn/+/1190/comment/7157deb0_b889163c?usp=email : PS2, Line 312: * limit is reached. */ shouldn't the closing */ be on a new line like all other multiline comments? File src/openvpn/crypto.c: http://gerrit.openvpn.net/c/openvpn/+/1190/comment/ccf561c6_6036bff1?usp=email : PS2, Line 105: * iterate the epoch */ same here -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1190?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I34e6b680618a52003d8408852d415c8aeac01feb Gerrit-Change-Number: 1190 Gerrit-PatchSet: 2 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: ordex <an...@ma...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Comment-Date: Thu, 04 Dec 2025 12:47:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes |
|
From: cron2 (C. Review) <ge...@op...> - 2025-12-04 13:24:10
|
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1190?usp=email ) Change subject: Clarify some code in epoch with better comments ...................................................................... Clarify some code in epoch with better comments Change-Id: I34e6b680618a52003d8408852d415c8aeac01feb Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Frank Lichtenheld <fr...@li...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1190 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg34829.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/crypto.c M src/openvpn/crypto.h 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 8049b3a..e43bc6c 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -97,6 +97,13 @@ /* IV starts with packet id to make the IV unique for packet */ if (use_epoch_data_format) { + /* Note this does not check aead_usage_limit but can overstep it by + * a few extra blocks in one extra write. This is not affecting the + * security margin as these extra blocks are on a completely + * different order of magnitude than the security margin. + * The next iteration/call to epoch_check_send_iterate will + * iterate the epoch + */ if (!packet_id_write_epoch(&opt->packet_id.send, ctx->epoch, &iv_buffer)) { msg(D_CRYPT_ERRORS, "ENCRYPT ERROR: packet ID roll over"); diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h index 72c6821..9424fd7 100644 --- a/src/openvpn/crypto.h +++ b/src/openvpn/crypto.h @@ -298,7 +298,8 @@ /** 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 */ + * key_ctx_bi.decrypt.epoch + */ struct epoch_key epoch_key_send; /** epoch_key used for the highest receive epoch keys */ @@ -309,7 +310,8 @@ /** The 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 - * limit is reached*/ + * limit is reached. + */ uint64_t aead_usage_limit; /** Keeps the future epoch data keys for decryption. The current one -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1190?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I34e6b680618a52003d8408852d415c8aeac01feb Gerrit-Change-Number: 1190 Gerrit-PatchSet: 3 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: ordex <an...@ma...> Gerrit-CC: openvpn-devel <ope...@li...> |
|
From: cron2 (C. Review) <ge...@op...> - 2025-12-04 13:24:12
|
cron2 has uploaded a new patch set (#3) to the change originally created by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/1190?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by flichtenheld, Code-Review-1 by ordex Change subject: Clarify some code in epoch with better comments ...................................................................... Clarify some code in epoch with better comments Change-Id: I34e6b680618a52003d8408852d415c8aeac01feb Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Frank Lichtenheld <fr...@li...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1190 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg34829.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/crypto.c M src/openvpn/crypto.h 2 files changed, 11 insertions(+), 2 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/90/1190/3 diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 8049b3a..e43bc6c 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -97,6 +97,13 @@ /* IV starts with packet id to make the IV unique for packet */ if (use_epoch_data_format) { + /* Note this does not check aead_usage_limit but can overstep it by + * a few extra blocks in one extra write. This is not affecting the + * security margin as these extra blocks are on a completely + * different order of magnitude than the security margin. + * The next iteration/call to epoch_check_send_iterate will + * iterate the epoch + */ if (!packet_id_write_epoch(&opt->packet_id.send, ctx->epoch, &iv_buffer)) { msg(D_CRYPT_ERRORS, "ENCRYPT ERROR: packet ID roll over"); diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h index 72c6821..9424fd7 100644 --- a/src/openvpn/crypto.h +++ b/src/openvpn/crypto.h @@ -298,7 +298,8 @@ /** 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 */ + * key_ctx_bi.decrypt.epoch + */ struct epoch_key epoch_key_send; /** epoch_key used for the highest receive epoch keys */ @@ -309,7 +310,8 @@ /** The 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 - * limit is reached*/ + * limit is reached. + */ uint64_t aead_usage_limit; /** Keeps the future epoch data keys for decryption. The current one -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1190?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I34e6b680618a52003d8408852d415c8aeac01feb Gerrit-Change-Number: 1190 Gerrit-PatchSet: 3 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: ordex <an...@ma...> Gerrit-CC: openvpn-devel <ope...@li...> |
Thanks for helping keep SourceForge clean.
X