openvpn-devel — OpenVPN developers list

[Openvpn-devel] [S] Change in openvpn[master]: Fix new doxygen warnings about using @return in void functions

[flichtenheld (C. Review) <ge...@op...>]

flichtenheld has abandoned this change. ( http://gerrit.openvpn.net/c/openvpn/+/1072?usp=email )

Change subject: Fix new doxygen warnings about using @return in void functions
......................................................................


Abandoned

duplicate
-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1072?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: Ia5a5b8d2a8ca5901346aad41efc4ff90a9a45273
Gerrit-Change-Number: 1072
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: abandon

[Openvpn-devel] [S] Change in openvpn[master]: Fix new doxygen warnings about using @return in void functions

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

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

to review the following change.


Change subject: Fix new doxygen warnings about using @return in void functions
......................................................................

Fix new doxygen warnings about using @return in void functions

These seem to have been added in a more recent doxygen version
than I previously tested with.

Change-Id: Ia5a5b8d2a8ca5901346aad41efc4ff90a9a45273
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/crypto.h
M src/openvpn/fragment.h
M src/openvpn/push.c
3 files changed, 8 insertions(+), 8 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/72/1072/1

diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index 5bd1ad5..68ad901 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -467,12 +467,12 @@
  * If an error occurs during processing, then the \a buf %buffer is set to
  * empty.
  *
- * @param buf          - The %buffer containing the packet on which to
+ * @param[in,out] buf  - The %buffer containing the packet on which to
  *                       perform security operations.
  * @param work         - An initialized working %buffer.
  * @param opt          - The security parameter state for this VPN tunnel.
  *
- * @return This function returns void.\n On return, the \a buf argument
+ * @note On return, the \a buf argument
  *     will point to the resulting %buffer.  This %buffer will either
  *     contain the processed packet ready for sending, or be empty if an
  *     error occurred.
diff --git a/src/openvpn/fragment.h b/src/openvpn/fragment.h
index 5003c96..cc96057 100644
--- a/src/openvpn/fragment.h
+++ b/src/openvpn/fragment.h
@@ -309,7 +309,7 @@
  *
  * @param f            - The \c fragment_master structure for this VPN
  *                       tunnel.
- * @param buf          - A pointer to the buffer structure containing the
+ * @param[in,out] buf  - A pointer to the buffer structure containing the
  *                       incoming packet.  This pointer will have been
  *                       modified on return either to point to a
  *                       completely reassembled packet, or to have length
@@ -317,7 +317,8 @@
  * @param frame        - The packet geometry parameters for this VPN
  *                       tunnel.
  *
- * @return Void.\n On return, the \a buf argument will point to a buffer.
+ * @note On return the \a buf argument buffer will be modified
+ *     to communicate the result of the function.
  *     The buffer will have nonzero length if the incoming packet passed
  *     to this function was whole and unfragmented, or if it was the final
  *     part of a fragmented packet thereby completing reassembly.  On the
@@ -363,14 +364,15 @@
  *
  * @param f            - The \c fragment_master structure for this VPN
  *                       tunnel.
- * @param buf          - A pointer to the buffer structure containing the
+ * @param[in,out] buf  - A pointer to the buffer structure containing the
  *                       outgoing packet.  This pointer will be modified
  *                       to point to a whole unfragmented packet or to the
  *                       first part of a fragmented packet on return.
  * @param frame        - The packet geometry parameters for this VPN
  *                       tunnel.
  *
- * @return Void.\n On return, the \a buf argument will point to a buffer.
+ * @note On return the \a buf argument buffer will be modified
+ *     to communicate the result of the function.
  *     This buffer contains either the whole original outgoing packet if
  *     fragmentation was not necessary, or the first part of the
  *     fragmented outgoing packet if fragmentation was necessary. In both
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index fe289f1..ad8fa3d7 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -582,8 +582,6 @@
  * @param tls_multi     tls multi context of VPN tunnel
  * @param gc            gc arena for allocating push options
  * @param push_list     push list to where options are added
- *
- * @return true on success, false on failure.
  */
 void
 prepare_auth_token_push_reply(struct tls_multi *tls_multi, struct gc_arena *gc,

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1072?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: Ia5a5b8d2a8ca5901346aad41efc4ff90a9a45273
Gerrit-Change-Number: 1072
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newchange

[Openvpn-devel] IRC community meeting summary

[Johan D. <jo...@op...>]

Meeting summary for 2 July 2025:

  *

    *New: cve.mitre.org deprecation notice*
    The MITRE CVE site we are linking to for all CVE record references
    has a deprecation notice.
    Instead cve.org is being advised as the site to use from now on.
    Suggestion from novaflash is to search/replace all links and update
    them accordingly to go to the new site.
    novaflash will make an internal company ticket to update links on
    the main site.
    and novaflash will search/replace things on community wiki to do the
    same.

  *

    *New: minor issue on community.openvpn.net site*
    While not causing critical issues, there are some cache related
    issues caused by cloudflare.
    This requires some finetuning of settings to solve issues like:
    Logging in requires to use query string parameter to defeat cache.
    Changes like enabling dark mode can linger in cache and affect other
    visitors.
    Sometimes pages added don't show in menu until cache is defeated in
    some way.
    The "are you human" screen is quite persistent and annoying, perhaps
    it can be fixed.
    novaflash is working with the company to look into finetuning the
    cloudflare settings to address these issues.

  *

    *New: format code using clang formatting*
    It seems prudent to do this before the real 2.7 release.
    It was discussed and there's some additional work to be paid either
    on reviewer or submitter side.
    Strategy will be; get all code into beta1, collect bugfixes,
    reformat everything, then beta2.
    Collect more bugfixes and then do release 2.7.0 and branch it off
    into its own release branch.

  *

    *Updated: Changes to community pages on main website*
    Company has published the updated community
    page:https://openvpn.net/community/
    This design was created in collaboration with community members lev,
    ordex, and novaflash.
    If any changes are requested please relay them to novaflash as per
    usual, and he will forward it internally.

  *

    *Updated: Release 2.7*
    For the DNS related changes, the macOS DNS script is merged but
    there's still some minor tweaks being made.
    For multisocket, a bug report came in, and we're working to fix that.
    For the DCO related changes, DCO+TCP is improving. Epoch data keys
    still needs to be done, float has a patch ready for review, mssfix
    not planned yet. For the live route updates changes, chances are
    improving that it may get in to 2.7 after all.

  *

    *Updated: push_update / live route updates*
    For client-side support, company did QA on it against the current
    only server implementation (cloudconnexa) and it works as expected.
    cron2 will put reviewing it on his to-do list.
    Server-side support patch is up and requires review and is a bit
    more involved, lev__ and company QA will work on it.

  *

    *Updated: OpenVPN community meetup 2025*
    https://community.openvpn.net/openvpn/wiki/CommunityMeetup2025
    When: weekend of 25 and 26 october.
    Where: Napoli, Italy.
    Meeting room: it looks like this may be
    it;https://www.hotelparadisonapoli.it/en/home-page.aspxwe will work
    to get budget cleared for this.
    Hotel: probably Paradiso Napoli. Beer: yes.
    T-shirts: yes.

As always you're welcome to join at #openvpn-meeting on Libera IRC 
network every Wednesday at 14:00 Central European Time.

Kind regards,
Johan Draaisma

[Openvpn-devel] [S] Change in openvpn[master]: Fix new doxygen warnings about using @return in void functions

[ordex (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld.

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

Change subject: Fix new doxygen warnings about using @return in void functions
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

Patchset:

PS1: 
can we drop the part saying "Does not have a return value." ?
We are not documenting @return anymore and functions are void. so it's pretty redundant



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1029?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: Ia2b3eda18bd6dbce6c470037c7a01097e8147c29
Gerrit-Change-Number: 1029
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: ordex <an...@ma...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Wed, 02 Jul 2025 12:21:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: Fix new doxygen warnings about using @return in void functions

[plaisthos (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld.

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

Change subject: Fix new doxygen warnings about using @return in void functions
......................................................................


Patch Set 1: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1029?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: Ia2b3eda18bd6dbce6c470037c7a01097e8147c29
Gerrit-Change-Number: 1029
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Wed, 02 Jul 2025 12:16:25 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Re: [Openvpn-devel] [PATCH ovpn-net-next] ovpn: reset GSO metadata after decapsulation

[Antonio Q. <a...@un...>]

Hi,

On 01/07/2025 15:51, Gert Doering wrote:
> Hi
> 
> On Tue, Jul 01, 2025 at 02:47:44PM +0200, Ralf Lici wrote:
>> By calling skb_gso_reset(skb) we ensure the inner packet is presented to
>> gro_cells_receive() with a clean slate, correctly indicating it is an
>> individual packet from the perspective of the local stack.
> 
> Amazing find.
> 
> I have tested this on my ubuntu 20.04 (backports) testbed that had the
> "large ping tcp instance -> udp instance fail", and now everything succeeds
> (this patch applied to "DCO version: ovpn-net-next/net-6.15.0-8f0bda6").
> 
> Tested-By: Gert Doering <ge...@gr...>

Thanks a lot for testing this out!
I slightly improved the commit message and added missing tags.

The patch has been queued for sending to net.

Regards,


-- 
Antonio Quartulli

Re: [Openvpn-devel] [PATCH ovpn-net-next] ovpn: reset GSO metadata after decapsulation

[Gert D. <ge...@gr...>]

Hi

On Tue, Jul 01, 2025 at 02:47:44PM +0200, Ralf Lici wrote:
> By calling skb_gso_reset(skb) we ensure the inner packet is presented to
> gro_cells_receive() with a clean slate, correctly indicating it is an
> individual packet from the perspective of the local stack.

Amazing find.

I have tested this on my ubuntu 20.04 (backports) testbed that had the
"large ping tcp instance -> udp instance fail", and now everything succeeds
(this patch applied to "DCO version: ovpn-net-next/net-6.15.0-8f0bda6").

Tested-By: Gert Doering <ge...@gr...>

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
                             Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany                             ge...@gr...

[Openvpn-devel] [PATCH ovpn-net-next] ovpn: reset GSO metadata after decapsulation

[Ralf L. <ra...@ma...>]

The ovpn_netdev_write() function is responsible for injecting
decapsulated and decrypted packets back into the local network stack.
Prior to this change, the skb could retain GSO metadata from the outer,
encrypted tunnel packet. This original GSO metadata, relevant to the
sender's context for the tunnel, becomes invalid and misleading for the
local receive path once the inner packet is exposed.

Leaving this stale metadata intact causes internal GSO validation checks
further down the kernel's network stack (validate_xmit_skb()) to fail,
leading to packet drops. The reasons for these failures vary by
protocol, for example:
- for ICMP, no offload handler is registered;
- for TCP and UDP, the respective offload handlers return errors when
  comparing skb->len to the outdated skb_shinfo(skb)->gso_size.

By calling skb_gso_reset(skb) we ensure the inner packet is presented to
gro_cells_receive() with a clean slate, correctly indicating it is an
individual packet from the perspective of the local stack.

This change eliminates the "Driver has suspect GRO implementation, TCP
performance may be compromised" warning and improves overall TCP
performance by allowing GSO/GRO to function as intended on the
decapsulated traffic. (Note: UDP GSO is not currently supported in ovpn)

Signed-off-by: Ralf Lici <ra...@ma...>
---
 drivers/net/ovpn/io.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ovpn/io.c b/drivers/net/ovpn/io.c
index ebf1e849506b..3e9e7f8444b3 100644
--- a/drivers/net/ovpn/io.c
+++ b/drivers/net/ovpn/io.c
@@ -62,6 +62,13 @@ static void ovpn_netdev_write(struct ovpn_peer *peer, struct sk_buff *skb)
 	unsigned int pkt_len;
 	int ret;
 
+	/*
+	 * GSO state from the transport layer is not valid for the tunnel/data
+	 * path. Reset all GSO fields to prevent any further GSO processing
+	 * from entering an inconsistent state.
+	 */
+	skb_gso_reset(skb);
+
 	/* we can't guarantee the packet wasn't corrupted before entering the
 	 * VPN, therefore we give other layers a chance to check that
 	 */
-- 
2.50.0

[Openvpn-devel] [PATCH applied] Re: packet_id: Fix build with --disable-debug

[Gert D. <ge...@gr...>]

"Obviously correct" ;-) - not tested beyond what the buildbots do
anyway.

Your patch has been applied to the master branch.

commit 16c1da39e93578766acd6ba085f4960190fd6220
Author: Frank Lichtenheld
Date:   Mon Jun 30 18:42:01 2025 +0200

     packet_id: Fix build with --disable-debug

     Signed-off-by: Frank Lichtenheld <fr...@li...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg32013.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [XS] Change in openvpn[master]: packet_id: Fix build with --disable-debug

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1071?usp=email )

Change subject: packet_id: Fix build with --disable-debug
......................................................................

packet_id: Fix build with --disable-debug

Broken since commit
bc62a9a02cb7365a678bcd3f2faf537a420cc5a0
"Add methods to read/write packet ids for epoch data"

Change-Id: I3bed9c7aafee8e62ddae14c0d3e21cf4c146a37c
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32013.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/packet_id.c
1 file changed, 2 insertions(+), 3 deletions(-)




diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c
index c8dae32..76a81c6 100644
--- a/src/openvpn/packet_id.c
+++ b/src/openvpn/packet_id.c
@@ -673,6 +673,8 @@
     gc_free(&gc);
 }
 
+#endif /* ifdef ENABLE_DEBUG */
+
 uint16_t
 packet_id_read_epoch(struct packet_id_net *pin, struct buffer *buf)
 {
@@ -711,6 +713,3 @@
 
     return buf_write(buf, &net_id, sizeof(net_id));
 }
-
-
-#endif /* ifdef ENABLE_DEBUG */

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1071?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: I3bed9c7aafee8e62ddae14c0d3e21cf4c146a37c
Gerrit-Change-Number: 1071
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [XS] Change in openvpn[master]: packet_id: Fix build with --disable-debug

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#2) to the change originally created by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/1071?usp=email )

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


Change subject: packet_id: Fix build with --disable-debug
......................................................................

packet_id: Fix build with --disable-debug

Broken since commit
bc62a9a02cb7365a678bcd3f2faf537a420cc5a0
"Add methods to read/write packet ids for epoch data"

Change-Id: I3bed9c7aafee8e62ddae14c0d3e21cf4c146a37c
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32013.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/packet_id.c
1 file changed, 2 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/71/1071/2

diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c
index c8dae32..76a81c6 100644
--- a/src/openvpn/packet_id.c
+++ b/src/openvpn/packet_id.c
@@ -673,6 +673,8 @@
     gc_free(&gc);
 }
 
+#endif /* ifdef ENABLE_DEBUG */
+
 uint16_t
 packet_id_read_epoch(struct packet_id_net *pin, struct buffer *buf)
 {
@@ -711,6 +713,3 @@
 
     return buf_write(buf, &net_id, sizeof(net_id));
 }
-
-
-#endif /* ifdef ENABLE_DEBUG */

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1071?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: I3bed9c7aafee8e62ddae14c0d3e21cf4c146a37c
Gerrit-Change-Number: 1071
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [PATCH v1] packet_id: Fix build with --disable-debug

[Gert D. <ge...@gr...>]

From: Frank Lichtenheld <fr...@li...>

Broken since commit
bc62a9a02cb7365a678bcd3f2faf537a420cc5a0
"Add methods to read/write packet ids for epoch data"

Change-Id: I3bed9c7aafee8e62ddae14c0d3e21cf4c146a37c
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Gert Doering <ge...@gr...>
---

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/+/1071
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <ge...@gr...>

        
diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c
index c8dae32..76a81c6 100644
--- a/src/openvpn/packet_id.c
+++ b/src/openvpn/packet_id.c
@@ -673,6 +673,8 @@
     gc_free(&gc);
 }
 
+#endif /* ifdef ENABLE_DEBUG */
+
 uint16_t
 packet_id_read_epoch(struct packet_id_net *pin, struct buffer *buf)
 {
@@ -711,6 +713,3 @@
 
     return buf_write(buf, &net_id, sizeof(net_id));
 }
-
-
-#endif /* ifdef ENABLE_DEBUG */

[Openvpn-devel] [XS] Change in openvpn[master]: packet_id: Fix build with --disable-debug

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

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

Change subject: packet_id: Fix build with --disable-debug
......................................................................


Patch Set 1: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1071?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: I3bed9c7aafee8e62ddae14c0d3e21cf4c146a37c
Gerrit-Change-Number: 1071
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Mon, 30 Jun 2025 16:41:52 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [XS] Change in openvpn[master]: packet_id: Fix build with --disable-debug

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

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

to review the following change.


Change subject: packet_id: Fix build with --disable-debug
......................................................................

packet_id: Fix build with --disable-debug

Broken since commit
bc62a9a02cb7365a678bcd3f2faf537a420cc5a0
"Add methods to read/write packet ids for epoch data"

Change-Id: I3bed9c7aafee8e62ddae14c0d3e21cf4c146a37c
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/packet_id.c
1 file changed, 2 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/71/1071/1

diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c
index c8dae32..76a81c6 100644
--- a/src/openvpn/packet_id.c
+++ b/src/openvpn/packet_id.c
@@ -673,6 +673,8 @@
     gc_free(&gc);
 }
 
+#endif /* ifdef ENABLE_DEBUG */
+
 uint16_t
 packet_id_read_epoch(struct packet_id_net *pin, struct buffer *buf)
 {
@@ -711,6 +713,3 @@
 
     return buf_write(buf, &net_id, sizeof(net_id));
 }
-
-
-#endif /* ifdef ENABLE_DEBUG */

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1071?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: I3bed9c7aafee8e62ddae14c0d3e21cf4c146a37c
Gerrit-Change-Number: 1071
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newchange

[Openvpn-devel] [XS] Change in openvpn[master]: dns: create NRPT registry key if it doesn't exist

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1069?usp=email )

Change subject: dns: create NRPT registry key if it doesn't exist
......................................................................

dns: create NRPT registry key if it doesn't exist

Windows 2019 Server by default does not have the key where local system
NRPT rules are stored. Tests have determined that NRPT is actually
working when rules are created under the key. So, instead of failing if
the key doesn't exist, we create it, and things will start working.

Github: OpenVPN/openvpn#768
Change-Id: I46132ebaf3bf3b16798b6f2416f7bf7272f5646b
Signed-off-by: Heiko Hund <he...@is...>
Acked-by: Lev Stipakov <lst...@gm...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32001.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpnserv/interactive.c
1 file changed, 1 insertion(+), 1 deletion(-)




diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 3bd2722..628a96b 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -2662,7 +2662,7 @@
     if (err == ERROR_FILE_NOT_FOUND)
     {
         *gpol = FALSE;
-        err = RegOpenKeyExA(HKEY_LOCAL_MACHINE, sys_key, 0, KEY_ALL_ACCESS, &nrpt);
+        err = RegCreateKeyExA(HKEY_LOCAL_MACHINE, sys_key, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &nrpt, NULL);
         if (err)
         {
             nrpt = INVALID_HANDLE_VALUE;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1069?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: I46132ebaf3bf3b16798b6f2416f7bf7272f5646b
Gerrit-Change-Number: 1069
Gerrit-PatchSet: 4
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [XS] Change in openvpn[master]: dns: create NRPT registry key if it doesn't exist

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#4) to the change originally created by d12fk. ( http://gerrit.openvpn.net/c/openvpn/+/1069?usp=email )

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


Change subject: dns: create NRPT registry key if it doesn't exist
......................................................................

dns: create NRPT registry key if it doesn't exist

Windows 2019 Server by default does not have the key where local system
NRPT rules are stored. Tests have determined that NRPT is actually
working when rules are created under the key. So, instead of failing if
the key doesn't exist, we create it, and things will start working.

Github: OpenVPN/openvpn#768
Change-Id: I46132ebaf3bf3b16798b6f2416f7bf7272f5646b
Signed-off-by: Heiko Hund <he...@is...>
Acked-by: Lev Stipakov <lst...@gm...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32001.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpnserv/interactive.c
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/69/1069/4

diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 3bd2722..628a96b 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -2662,7 +2662,7 @@
     if (err == ERROR_FILE_NOT_FOUND)
     {
         *gpol = FALSE;
-        err = RegOpenKeyExA(HKEY_LOCAL_MACHINE, sys_key, 0, KEY_ALL_ACCESS, &nrpt);
+        err = RegCreateKeyExA(HKEY_LOCAL_MACHINE, sys_key, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &nrpt, NULL);
         if (err)
         {
             nrpt = INVALID_HANDLE_VALUE;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1069?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: I46132ebaf3bf3b16798b6f2416f7bf7272f5646b
Gerrit-Change-Number: 1069
Gerrit-PatchSet: 4
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [PATCH applied] Re: dns: create NRPT registry key if it doesn't exist

[Gert D. <ge...@gr...>]

I have not tested this beyond "does it compile on a MinGW buildhost" (it
does), and "what does this function do?" (open a registry key if it exists,
and create a new key if it doesn't exist yet) - so this all seems to make
sense, and Lev confirms that it does what it wants to achieve ;-)

Your patch has been applied to the master branch.

commit df4863aa0e43544ea82ab9d98966a03a95c62334
Author: Heiko Hund
Date:   Fri Jun 27 10:24:53 2025 +0200

     dns: create NRPT registry key if it doesn't exist

     Signed-off-by: Heiko Hund <he...@is...>
     Acked-by: Lev Stipakov <lst...@gm...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg32001.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [M] Change in openvpn[master]: run forced --dns-updown without --script-security

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1065?usp=email )

Change subject: run forced --dns-updown without --script-security
......................................................................

run forced --dns-updown without --script-security

Due to a shortcut in the `--dns-updown force' implementation, running the
default dns-updown script required `--script-security 2'. This makes the
forced default script run without --script-security set.

Change-Id: I55940b78e35f0e3d74aa6cba14378afed97a444e
Signed-off-by: Heiko Hund <he...@is...>
Acked-by: Frank Lichtenheld <fr...@li...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31994.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dns.c
M src/openvpn/dns.h
M src/openvpn/options.c
3 files changed, 39 insertions(+), 12 deletions(-)




diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 939ae09..ea3d91b 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -264,7 +264,7 @@
     clone.servers = clone_dns_servers(o->servers, gc);
     clone.servers_prepull = clone_dns_servers(o->servers_prepull, gc);
     clone.updown = o->updown;
-    clone.user_set_updown = o->user_set_updown;
+    clone.updown_flags = o->updown_flags;
 
     return clone;
 }
@@ -580,7 +580,7 @@
     argv_printf(&argv, "%s", o->updown);
     argv_msg(M_INFO, &argv);
     int res;
-    if (o->user_set_updown)
+    if (dns_updown_user_set(o))
     {
         res = openvpn_run_script(&argv, es, S_EXITCODE, "dns updown");
     }
@@ -692,7 +692,7 @@
 run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct dns_updown_runner_info *updown_runner)
 {
     struct dns_options *dns = &o->dns_options;
-    if (!dns->updown || (o->up_script && !dns->user_set_updown))
+    if (!dns->updown || (o->up_script && !dns_updown_user_set(dns) && !dns_updown_forced(dns)))
     {
         return;
     }
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index 688daa7..d33f64e 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -42,13 +42,18 @@
     DNS_TRANSPORT_TLS
 };
 
+enum dns_updown_flags {
+    DNS_UPDOWN_NO_FLAGS,
+    DNS_UPDOWN_USER_SET,
+    DNS_UPDOWN_FORCED
+};
+
 struct dns_domain {
     struct dns_domain *next;
     const char *name;
 };
 
-struct dns_server_addr
-{
+struct dns_server_addr {
     union {
         struct in_addr a4;
         struct in6_addr a6;
@@ -103,7 +108,7 @@
     struct dns_server *servers;
     struct gc_arena gc;
     const char *updown;
-    bool user_set_updown;
+    enum dns_updown_flags updown_flags;
 };
 
 /**
@@ -195,4 +200,26 @@
  */
 void show_dns_options(const struct dns_options *o);
 
+/**
+ * Returns whether dns-updown is user defined
+ *
+ * @param   o           Pointer to the DNS options struct
+ */
+static inline bool
+dns_updown_user_set(const struct dns_options *o)
+{
+    return o->updown_flags == DNS_UPDOWN_USER_SET;
+}
+
+/**
+ * Returns whether dns-updown is forced to run
+ *
+ * @param   o           Pointer to the DNS options struct
+ */
+static inline bool
+dns_updown_forced(const struct dns_options *o)
+{
+    return o->updown_flags == DNS_UPDOWN_FORCED;
+}
+
 #endif /* ifndef DNS_H */
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 7e26069..af097f8 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3593,7 +3593,7 @@
     struct gc_arena gc = gc_new();
     struct dns_options *dns = &o->dns_options;
 
-    if (dns->servers || dns->user_set_updown)
+    if (dns->servers || dns_updown_user_set(dns) || dns_updown_forced(dns))
     {
         /* Clean up env from --dhcp-option DNS config */
         struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
@@ -3667,7 +3667,7 @@
             }
         }
     }
-    else if (o->up_script && !dns->user_set_updown)
+    else if (o->up_script && !dns_updown_user_set(dns) && !dns_updown_forced(dns))
     {
         /* Set foreign option env vars from --dns config */
         const char *p[] = { "dhcp-option", NULL, NULL };
@@ -8182,15 +8182,15 @@
         if (streq(p[1], "disable"))
         {
             dns->updown = NULL;
-            dns->user_set_updown = false;
+            dns->updown_flags = DNS_UPDOWN_NO_FLAGS;
         }
         else if (streq(p[1], "force"))
         {
             /* force dns-updown run, even if a --up script is defined */
-            if (dns->user_set_updown == false)
+            if (!dns_updown_user_set(dns))
             {
                 dns->updown = DEFAULT_DNS_UPDOWN;
-                dns->user_set_updown = true;
+                dns->updown_flags = DNS_UPDOWN_FORCED;
             }
         }
         else
@@ -8201,7 +8201,7 @@
                 dns->updown = NULL;
             }
             set_user_script(options, &dns->updown, p[1], p[0], false);
-            dns->user_set_updown = true;
+            dns->updown_flags = DNS_UPDOWN_USER_SET;
         }
     }
     else if (streq(p[0], "dns") && p[1])

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1065?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: I55940b78e35f0e3d74aa6cba14378afed97a444e
Gerrit-Change-Number: 1065
Gerrit-PatchSet: 4
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [M] Change in openvpn[master]: run forced --dns-updown without --script-security

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#4) to the change originally created by d12fk. ( http://gerrit.openvpn.net/c/openvpn/+/1065?usp=email )

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


Change subject: run forced --dns-updown without --script-security
......................................................................

run forced --dns-updown without --script-security

Due to a shortcut in the `--dns-updown force' implementation, running the
default dns-updown script required `--script-security 2'. This makes the
forced default script run without --script-security set.

Change-Id: I55940b78e35f0e3d74aa6cba14378afed97a444e
Signed-off-by: Heiko Hund <he...@is...>
Acked-by: Frank Lichtenheld <fr...@li...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31994.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dns.c
M src/openvpn/dns.h
M src/openvpn/options.c
3 files changed, 39 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/65/1065/4

diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 939ae09..ea3d91b 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -264,7 +264,7 @@
     clone.servers = clone_dns_servers(o->servers, gc);
     clone.servers_prepull = clone_dns_servers(o->servers_prepull, gc);
     clone.updown = o->updown;
-    clone.user_set_updown = o->user_set_updown;
+    clone.updown_flags = o->updown_flags;
 
     return clone;
 }
@@ -580,7 +580,7 @@
     argv_printf(&argv, "%s", o->updown);
     argv_msg(M_INFO, &argv);
     int res;
-    if (o->user_set_updown)
+    if (dns_updown_user_set(o))
     {
         res = openvpn_run_script(&argv, es, S_EXITCODE, "dns updown");
     }
@@ -692,7 +692,7 @@
 run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct dns_updown_runner_info *updown_runner)
 {
     struct dns_options *dns = &o->dns_options;
-    if (!dns->updown || (o->up_script && !dns->user_set_updown))
+    if (!dns->updown || (o->up_script && !dns_updown_user_set(dns) && !dns_updown_forced(dns)))
     {
         return;
     }
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index 688daa7..d33f64e 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -42,13 +42,18 @@
     DNS_TRANSPORT_TLS
 };
 
+enum dns_updown_flags {
+    DNS_UPDOWN_NO_FLAGS,
+    DNS_UPDOWN_USER_SET,
+    DNS_UPDOWN_FORCED
+};
+
 struct dns_domain {
     struct dns_domain *next;
     const char *name;
 };
 
-struct dns_server_addr
-{
+struct dns_server_addr {
     union {
         struct in_addr a4;
         struct in6_addr a6;
@@ -103,7 +108,7 @@
     struct dns_server *servers;
     struct gc_arena gc;
     const char *updown;
-    bool user_set_updown;
+    enum dns_updown_flags updown_flags;
 };
 
 /**
@@ -195,4 +200,26 @@
  */
 void show_dns_options(const struct dns_options *o);
 
+/**
+ * Returns whether dns-updown is user defined
+ *
+ * @param   o           Pointer to the DNS options struct
+ */
+static inline bool
+dns_updown_user_set(const struct dns_options *o)
+{
+    return o->updown_flags == DNS_UPDOWN_USER_SET;
+}
+
+/**
+ * Returns whether dns-updown is forced to run
+ *
+ * @param   o           Pointer to the DNS options struct
+ */
+static inline bool
+dns_updown_forced(const struct dns_options *o)
+{
+    return o->updown_flags == DNS_UPDOWN_FORCED;
+}
+
 #endif /* ifndef DNS_H */
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 7e26069..af097f8 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3593,7 +3593,7 @@
     struct gc_arena gc = gc_new();
     struct dns_options *dns = &o->dns_options;
 
-    if (dns->servers || dns->user_set_updown)
+    if (dns->servers || dns_updown_user_set(dns) || dns_updown_forced(dns))
     {
         /* Clean up env from --dhcp-option DNS config */
         struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
@@ -3667,7 +3667,7 @@
             }
         }
     }
-    else if (o->up_script && !dns->user_set_updown)
+    else if (o->up_script && !dns_updown_user_set(dns) && !dns_updown_forced(dns))
     {
         /* Set foreign option env vars from --dns config */
         const char *p[] = { "dhcp-option", NULL, NULL };
@@ -8182,15 +8182,15 @@
         if (streq(p[1], "disable"))
         {
             dns->updown = NULL;
-            dns->user_set_updown = false;
+            dns->updown_flags = DNS_UPDOWN_NO_FLAGS;
         }
         else if (streq(p[1], "force"))
         {
             /* force dns-updown run, even if a --up script is defined */
-            if (dns->user_set_updown == false)
+            if (!dns_updown_user_set(dns))
             {
                 dns->updown = DEFAULT_DNS_UPDOWN;
-                dns->user_set_updown = true;
+                dns->updown_flags = DNS_UPDOWN_FORCED;
             }
         }
         else
@@ -8201,7 +8201,7 @@
                 dns->updown = NULL;
             }
             set_user_script(options, &dns->updown, p[1], p[0], false);
-            dns->user_set_updown = true;
+            dns->updown_flags = DNS_UPDOWN_USER_SET;
         }
     }
     else if (streq(p[0], "dns") && p[1])

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1065?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: I55940b78e35f0e3d74aa6cba14378afed97a444e
Gerrit-Change-Number: 1065
Gerrit-PatchSet: 4
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [PATCH applied] Re: run forced --dns-updown without --script-security

[Gert D. <ge...@gr...>]

Thanks for addressing this imbalance wrt --script-security - I have not
tested the various combinations, just --dns-updown $builddir/... 
in my t_client tests (still works, and correctly still requires
script-security).  A brief stare at the code also looks reasonable.

The message "I am going to run *this* script now" is still a bit barebones,
though ;-)

2025-06-28 18:18:00 ../distro/dns-scripts/dns-updown
2025-06-28 18:18:00 WARNING: External program may not be called unless '--script-security 2' or higher...

Your patch has been applied to the master branch.

commit cbf3621825c9e2f2542a370f4c049411c71d2329
Author: Heiko Hund
Date:   Thu Jun 26 11:30:00 2025 +0200

     run forced --dns-updown without --script-security

     Signed-off-by: Heiko Hund <he...@is...>
     Acked-by: Frank Lichtenheld <fr...@li...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg31994.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [M] Change in openvpn[master]: Define a .clang-format file for the project

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: cron2, plaisthos.

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

Change subject: Define a .clang-format file for the project
......................................................................


Patch Set 16:

(1 comment)

File .pre-commit-config.yaml:

http://gerrit.openvpn.net/c/openvpn/+/790/comment/5597360b_bf2ebcec :
PS15, Line 9: 
> This is "take it or leave it". […]
See https://gerrit.openvpn.net/c/openvpn/+/1070



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/790?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: I40f6af10c5ee2f5aed4185d783fc622a2e3c19ff
Gerrit-Change-Number: 790
Gerrit-PatchSet: 16
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Comment-Date: Fri, 27 Jun 2025 16:50:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: cron2 <ge...@gr...>
Comment-In-Reply-To: flichtenheld <fr...@li...>
Gerrit-MessageType: comment

[Openvpn-devel] [L] Change in openvpn[master]: Remove uncrustify config and reformat-all.sh, switch GHA

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: cron2, flichtenheld, plaisthos.

Hello cron2, plaisthos, 

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

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

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


Change subject: Remove uncrustify config and reformat-all.sh, switch GHA
......................................................................

Remove uncrustify config and reformat-all.sh, switch GHA

Replaced with clang-format and pre-commit.

Add a README file that explains how to use pre-commit
and how to combine this with the old hook.

Old hook does not get removed and will be updated
to be compatible with manually installed clang-format
in a separate commit.

Change-Id: I15d4946800cbfaead67a73450ff3b12193814e54
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M .github/workflows/build.yaml
D dev-tools/reformat-all.sh
D dev-tools/special-files.lst
D dev-tools/uncrustify.conf
4 files changed, 12 insertions(+), 241 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/30/830/11

diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
index d4fdc9d..37c09fe 100644
--- a/.github/workflows/build.yaml
+++ b/.github/workflows/build.yaml
@@ -5,35 +5,26 @@
   pull_request:
 
 jobs:
-  checkuncrustify:
-    name: "Check code style with Uncrustify"
-    # Ubuntu 22.04 has uncrustify 0.72_f
-    runs-on: ubuntu-22.04
+  clang-format:
+    name: Check code style with clang-format
+    runs-on: ubuntu-24.04
     steps:
       - name: Install dependencies
-        run: sudo apt update && sudo apt install -y uncrustify
+        run: |
+          sudo apt update && sudo apt install -y python3-pip
+          pip3 install pre-commit
       - name: Checkout OpenVPN
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
-        with:
-          path: openvpn
-      - name: Show uncrustify version
-        run: uncrustify --version
-      - name: Run uncrustify
-        run: ./dev-tools/reformat-all.sh
-        working-directory: openvpn
+      - name: Run clang-format
+        run: pre-commit run -a --show-diff-on-failure || true
       - name: Check for changes
-        run: git diff --output=uncrustify-changes.patch
-        working-directory: openvpn
-      - name: Show changes on standard output
-        run: git diff
-        working-directory: openvpn
+        run: git diff --output=format-changes.patch
       - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
         with:
-          name: uncrustify-changes.patch
-          path: 'openvpn/uncrustify-changes.patch'
+          name: format-changes.patch
+          path: format-changes.patch
       - name: Set job status
-        run: test ! -s uncrustify-changes.patch
-        working-directory: openvpn
+        run: test ! -s format-changes.patch
 
   android:
     strategy:
diff --git a/dev-tools/reformat-all.sh b/dev-tools/reformat-all.sh
deleted file mode 100755
index 02421c1..0000000
--- a/dev-tools/reformat-all.sh
+++ /dev/null
@@ -1,136 +0,0 @@
-#!/bin/sh
-# reformat-all.sh - Reformat all git files in the checked out
-#                   git branch using uncrustify.
-#
-# Copyright (C) 2016-2025 - David Sommerseth <da...@op...>
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License
-# as published by the Free Software Foundation; either version 2
-# of the License.
-#
-# 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.
-#
-
-tstamp="$(date +%Y%m%d-%H%M%S)"
-files="$(pwd)/reformat-all_files-$tstamp.lst"
-log="$(pwd)/reformat-all_log-$tstamp.txt"
-
-srcroot="$(git rev-parse --show-toplevel)"
-cfg="$srcroot/dev-tools/uncrustify.conf"
-specialfiles="$srcroot/dev-tools/special-files.lst"
-
-export gitfiles=0
-export procfiles=0
-
-# Go to the root of the source tree
-cd "$srcroot"
-
-{
-    echo -n "** Starting $0: "
-    date
-
-    # Find all C source/header files
-    git ls-files | grep -E ".*\.[ch](\.in$|$)" > "${files}.git"
-
-    # Manage files which needs special treatment
-    awk -F\# '{gsub("\n| ", "", $1); print $1}' "$specialfiles" > "${files}.sp"
-    while read srcfile
-    do
-        res=$(grep "$srcfile" "${files}.sp" 2>/dev/null)
-        if [ $? -ne 0 ]; then
-            # If grep didn't find the file among special files,
-            # process it normally
-            echo "$srcfile" >> "$files"
-        else
-            mode=$(echo "$res" | cut -d:  -f1)
-            case "$mode" in
-                E)
-                    echo "** INFO **  Excluding '$srcfile'"
-                    ;;
-                P)
-                    echo "** INFO **  Pre-patching '$srcfile'"
-                    patchfile="${srcroot}"/dev-tools/reformat-patches/before_$(echo "$srcfile" | tr "/" "_").patch
-                    if [ -r "$patchfile" ]; then
-                        git apply "$patchfile"
-                        if [ $? -ne 0 ]; then
-                            echo "** ERROR **  Failed to apply pre-patch file: $patchfile"
-                            exit 2
-                        fi
-                    else
-                        echo "** WARN ** Pre-patch file for $srcfile is missing: $patchfile"
-                    fi
-                    echo "$srcfile" >> "${files}.postpatch"
-                    echo "$srcfile" >> "$files"
-                    ;;
-                *)
-                    echo "** WARN ** Unknown mode '$mode' for file '$srcfile'"
-                    ;;
-            esac
-        fi
-    done < "${files}.git"
-    rm -f "${files}.git" "${files}.sp"
-
-    # Kick off uncrustify
-    echo
-    echo "** INFO ** Running: uncrustify -c $cfg --no-backup -l C -F $files"
-    uncrustify -c "$cfg" --no-backup -l C -F "$files" 2>&1
-    res=$?
-    echo "** INFO ** Uncrustify completed (exit code $res)"
-} | tee "${log}-1"  # Log needs to be closed here, to be processed in next block
-
-{
-    # Check the results
-    gitfiles=$(wc -l "$files" | cut -d\  -f1)
-    procfiles=$(grep "Parsing: " "${log}-1" | wc -l)
-    echo
-    echo "C source/header files checked into git: $gitfiles"
-    echo "Files processed by uncrustify:          $procfiles"
-    echo
-
-    # Post-Patch files modified after we uncrustify have adjusted them
-    if [ -r "${files}.postpatch" ]; then
-        while read srcfile;
-        do
-            patchfile="${srcroot}"/dev-tools/reformat-patches/after_$(echo "$srcfile" | tr "/" "_").patch
-            if [ -r "$patchfile" ]; then
-                echo "** INFO **  Post-patching '$srcfile'"
-                git apply "$patchfile"
-                if [ $? -ne 0 ]; then
-                    echo "** WARN ** Failed to apply $patchfile"
-                fi
-            else
-                echo "** WARN ** Post-patch file for $srcfile is missing: $patchfile"
-            fi
-        done < "${files}.postpatch"
-        rm -f "${files}.postpatch"
-    fi
-} | tee "${log}-2" # Log needs to be closed here, to be processed in next block
-
-cat "${log}-1" "${log}-2" > "$log"
-
-{
-    ec=1
-    echo
-    if [ "$gitfiles" -eq "$procfiles" ]; then
-        echo "Reformatting completed successfully"
-        ec=0
-    else
-        last=$(tail -n1 "${log}-1")
-        echo "** ERROR ** Reformating failed to process all files."
-        echo "            uncrustify exit code: $res"
-        echo "            Last log line: $last"
-        echo
-    fi
-    rm -f "${log}-1" "${log}-2"
-} | tee -a "$log"
-rm -f "${files}"
-
-exit $ec
diff --git a/dev-tools/special-files.lst b/dev-tools/special-files.lst
deleted file mode 100644
index e5f2fc2..0000000
--- a/dev-tools/special-files.lst
+++ /dev/null
@@ -1,5 +0,0 @@
-E:doc/doxygen/doc_key_generation.h     # @verbatim section gets mistreated, exclude it
-E:src/compat/compat-lz4.c              # Preserve LZ4 upstream formatting
-E:src/compat/compat-lz4.h              # Preserve LZ4 upstream formatting
-E:src/openvpn/ovpn_dco_linux.h         # Preserve ovpn-dco upstream formatting
-E:src/openvpn/ovpn_dco_win.h           # Preserve ovpn-dco-win upstream formatting
diff --git a/dev-tools/uncrustify.conf b/dev-tools/uncrustify.conf
deleted file mode 100644
index 325f310..0000000
--- a/dev-tools/uncrustify.conf
+++ /dev/null
@@ -1,79 +0,0 @@
-# Use Allman-style
-indent_columns=4
-indent_braces=false
-indent_else_if=false
-indent_switch_case=4
-indent_label=1
-nl_if_brace=add
-nl_brace_else=add
-nl_elseif_brace=add
-nl_else_brace=add
-nl_else_if=remove
-nl_for_brace=add
-nl_while_brace=add
-nl_switch_brace=add
-nl_fdef_brace=add
-nl_do_brace=add
-sp_func_proto_paren=Remove
-sp_func_def_paren=Remove
-sp_func_call_paren=Remove
-sp_sizeof_paren=Remove
-
-# No tabs, spaces only
-indent_with_tabs=0
-align_with_tabs=false
-cmt_convert_tab_to_spaces=true
-
-# Do not put spaces between the # and preprocessor statements
-pp_space=remove
-
-# Various whitespace fiddling
-sp_assign=add
-sp_before_sparen=add
-sp_inside_sparen=remove
-sp_cond_colon=add
-sp_cond_question=add
-sp_bool=add
-sp_else_brace=add
-sp_brace_else=add
-sp_after_comma=add
-pos_arith=Lead
-pos_bool=Lead
-nl_func_type_name=add
-nl_before_case=true
-nl_assign_leave_one_liners=true
-nl_enum_leave_one_liners=true
-nl_brace_fparen=add
-nl_max=4
-nl_after_func_proto=2
-nl_end_of_file_min=1
-nl_end_of_file=force
-
-# Always use scoping braces for conditionals
-mod_full_brace_if=add
-mod_full_brace_if_chain=false
-mod_full_brace_while=add
-mod_full_brace_for=add
-mod_full_brace_do=add
-
-# Annotate #else and #endif statements
-mod_add_long_ifdef_endif_comment=20
-mod_add_long_ifdef_else_comment=5
-
-# Misc cleanup
-mod_remove_extra_semicolon=true
-
-# leave blank at end of empty for() statements
-sp_after_semi_for_empty=Add
-
-# Use C-style comments (/* .. */)
-cmt_c_nl_end=true
-cmt_star_cont=true
-cmt_cpp_to_c=true
-
-# Use "char **a"-style pointer stars/dereferences
-sp_before_ptr_star=Add
-sp_between_ptr_star=Remove
-sp_after_ptr_star=Remove
-sp_before_byref=Add
-sp_after_byref=Remove

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/830?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: I15d4946800cbfaead67a73450ff3b12193814e54
Gerrit-Change-Number: 830
Gerrit-PatchSet: 11
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [S] Change in openvpn[master]: Update git-pre-commit-uncrustify.sh to handle clang-format

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

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

to review the following change.


Change subject: Update git-pre-commit-uncrustify.sh to handle clang-format
......................................................................

Update git-pre-commit-uncrustify.sh to handle clang-format

Rename it as well, since it is not specific to
uncrustify anymore.

Change-Id: I03195c21807cdef0a2f903f424982ec29a555103
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
R dev-tools/git-pre-commit-format.sh
1 file changed, 32 insertions(+), 14 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/70/1070/1

diff --git a/dev-tools/git-pre-commit-uncrustify.sh b/dev-tools/git-pre-commit-format.sh
similarity index 81%
rename from dev-tools/git-pre-commit-uncrustify.sh
rename to dev-tools/git-pre-commit-format.sh
index 9851c21..6e1ac71 100755
--- a/dev-tools/git-pre-commit-uncrustify.sh
+++ b/dev-tools/git-pre-commit-format.sh
@@ -2,6 +2,7 @@
 
 # Copyright (c) 2015, David Martin
 #               2022, Heiko Hund
+#               2025, Frank Lichtenheld
 # All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -26,10 +27,12 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 
-# git pre-commit hook that runs an Uncrustify stylecheck.
+# git pre-commit hook that runs a stylecheck.
 # Features:
 #  - abort commit when commit does not comply with the style guidelines
 #  - create a patch of the proposed style changes
+#  - use clang-format or uncrustify depending on presence of .clang-format
+#    config file
 #
 # More info on Uncrustify: http://uncrustify.sourceforge.net/
 
@@ -77,27 +80,42 @@
     against=4b825dc642cb6eb9a060e54bf8d69288fbee4904
 fi
 
-UNCRUSTIFY=$(command -v uncrustify)
-UNCRUST_CONFIG="$(git rev-parse --show-toplevel)/dev-tools/uncrustify.conf"
+TOPDIR="$(git rev-parse --show-toplevel)"
+if [ -e "${TOPDIR}/.clang-format" ]; then
+    TOOL=clang-format
+    TOOL_BIN=$(command -v clang-format)
+    TOOL_CMD="$TOOL_BIN"
 
-# make sure the config file and executable are correctly set
-if [ ! -f "$UNCRUST_CONFIG" ] ; then
-    printf "Error: uncrustify config file not found.\n"
-    printf "Expected to find it at $UNCRUST_CONFIG.\n"
-    printf "Aborting commit.\n"
-    exit 1
+    # Allow to use in parallel with pre-commit
+    if [ $(basename "$0") = "pre-commit.legacy" ]; then
+       echo "Skipping clang-format check in favor of pre-commit"
+       exit 0
+    fi
+else
+    TOOL=uncrustify
+    TOOL_BIN=$(command -v uncrustify)
+    UNCRUST_CONFIG="${TOPDIR}/dev-tools/uncrustify.conf"
+    TOOL_CMD="$TOOL_BIN -q -l C -c $UNCRUST_CONFIG"
+
+    # make sure the config file is correctly set
+    if [ ! -f "$UNCRUST_CONFIG" ] ; then
+        printf "Error: uncrustify config file not found.\n"
+        printf "Expected to find it at $UNCRUST_CONFIG.\n"
+        printf "Aborting commit.\n"
+        exit 1
+    fi
 fi
 
-if [ -z "$UNCRUSTIFY" ] ; then
-    printf "Error: uncrustify executable not found.\n"
+if [ -z "$TOOL_BIN" ] ; then
+    printf "Error: $TOOL executable not found.\n"
     printf "Is it installed and in your \$PATH?\n"
     printf "Aborting commit.\n"
     exit 1
 fi
 
 # create a filename to store our generated patch
-patch=$(mktemp /tmp/ovpn-fmt-XXXXXX)
-tmpout=$(mktemp /tmp/uncrustify-XXXXXX)
+patch=$(mktemp /tmp/ovpn-fmt-patch-XXXXXX)
+tmpout=$(mktemp /tmp/ovpn-fmt-tmp-XXXXXX)
 
 # create one patch containing all changes to the files
 # sed to remove quotes around the filename, if inserted by the system
@@ -131,7 +149,7 @@
     #    +++ $tmpout timestamp
     # to both lines working on the same file and having a a/ and b/ prefix.
     # Else it can not be applied with 'git apply'.
-    git show ":$file" | "$UNCRUSTIFY" -q -l C -c "$UNCRUST_CONFIG" -o "$tmpout"
+    git show ":$file" | $TOOL_CMD > "$tmpout"
     git show ":$file" | diff -u -- - "$tmpout" | \
         sed -e "1s|--- -|--- \"b/$file_escaped_target\"|" -e "2s|+++ $tmpout|+++ \"a/$file_escaped_target\"|" >> "$patch"
 done

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1070?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: I03195c21807cdef0a2f903f424982ec29a555103
Gerrit-Change-Number: 1070
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newchange

[Openvpn-devel] [PATCH v3] dns: create NRPT registry key if it doesn't exist

[Gert D. <ge...@gr...>]

From: Heiko Hund <he...@is...>

Windows 2019 Server by default does not have the key where local system
NRPT rules are stored. Tests have determined that NRPT is actually
working when rules are created under the key. So, instead of failing if
the key doesn't exist, we create it, and things will start working.

Github: OpenVPN/openvpn#768
Change-Id: I46132ebaf3bf3b16798b6f2416f7bf7272f5646b
Signed-off-by: Heiko Hund <he...@is...>
Acked-by: Lev Stipakov <lst...@gm...>
---

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/+/1069
This mail reflects revision 3 of this Change.

Acked-by according to Gerrit (reflected above):
Lev Stipakov <lst...@gm...>

        
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 3bd2722..628a96b 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -2662,7 +2662,7 @@
     if (err == ERROR_FILE_NOT_FOUND)
     {
         *gpol = FALSE;
-        err = RegOpenKeyExA(HKEY_LOCAL_MACHINE, sys_key, 0, KEY_ALL_ACCESS, &nrpt);
+        err = RegCreateKeyExA(HKEY_LOCAL_MACHINE, sys_key, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &nrpt, NULL);
         if (err)
         {
             nrpt = INVALID_HANDLE_VALUE;

[Openvpn-devel] [XS] Change in openvpn[master]: dns: create NRPT registry key if it doesn't exist

[stipa (C. Review) <ge...@op...>]

Attention is currently required from: d12fk, flichtenheld, plaisthos.

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

Change subject: dns: create NRPT registry key if it doesn't exist
......................................................................


Patch Set 3: Code-Review+2

(1 comment)

Patchset:

PS3: 
Tested with DnsPolicyConfig key presented and not presented, works as expected - a subkey for NRPT rule got created.



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1069?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: I46132ebaf3bf3b16798b6f2416f7bf7272f5646b
Gerrit-Change-Number: 1069
Gerrit-PatchSet: 3
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: d12fk <he...@op...>
Gerrit-Comment-Date: Thu, 26 Jun 2025 13:00:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment