|
From: plaisthos (C. Review) <ge...@op...> - 2025-10-29 13:22:32
|
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/+/1327?usp=email
to review the following change.
Change subject: fix key_state_gen_auth_control_files probably checking file creation
......................................................................
fix key_state_gen_auth_control_files probably checking file creation
When the auth_failed_reason_file was added, it was forgotten to also add it
to the conditions that determine if the file creation was successful.
Reported-by: Joshua Rogers <co...@jo...>
Found-by: ZeroPath (https://zeropath.com/)
Change-Id: I94d2bdd234a1c416b78924d044bf7e57f1bed8c4
Signed-off-by: Arne Schwabe <ar...@rf...>
---
M src/openvpn/ssl_verify.c
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/27/1327/1
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 04ef27e..446c4a7 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -992,7 +992,7 @@
const char *apf = platform_create_temp_file(opt->tmp_dir, "apf", &gc);
const char *afr = platform_create_temp_file(opt->tmp_dir, "afr", &gc);
- if (acf && apf)
+ if (acf && apf && afr)
{
ads->auth_control_file = string_alloc(acf, NULL);
ads->auth_pending_file = string_alloc(apf, NULL);
@@ -1004,7 +1004,7 @@
}
gc_free(&gc);
- return (acf && apf);
+ return (acf && apf && afr);
}
/**
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1327?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I94d2bdd234a1c416b78924d044bf7e57f1bed8c4
Gerrit-Change-Number: 1327
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-30 19:39:40
|
Attention is currently required from: flichtenheld, plaisthos. cron2 has posted comments on this change by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/1327?usp=email ) Change subject: fix key_state_gen_auth_control_files probably checking file creation ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1327?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: I94d2bdd234a1c416b78924d044bf7e57f1bed8c4 Gerrit-Change-Number: 1327 Gerrit-PatchSet: 1 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Thu, 30 Oct 2025 19:39:25 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes |
|
From: Gert D. <ge...@gr...> - 2025-10-30 19:39:54
|
From: Arne Schwabe <ar...@rf...> When the auth_failed_reason_file was added, it was forgotten to also add it to the conditions that determine if the file creation was successful. Reported-by: Joshua Rogers <co...@jo...> Found-by: ZeroPath (https://zeropath.com/) Change-Id: I94d2bdd234a1c416b78924d044bf7e57f1bed8c4 Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1327 --- 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/+/1327 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering <ge...@gr...> diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 04ef27e..446c4a7 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -992,7 +992,7 @@ const char *apf = platform_create_temp_file(opt->tmp_dir, "apf", &gc); const char *afr = platform_create_temp_file(opt->tmp_dir, "afr", &gc); - if (acf && apf) + if (acf && apf && afr) { ads->auth_control_file = string_alloc(acf, NULL); ads->auth_pending_file = string_alloc(apf, NULL); @@ -1004,7 +1004,7 @@ } gc_free(&gc); - return (acf && apf); + return (acf && apf && afr); } /** |
|
From: Gert D. <ge...@gr...> - 2025-10-30 20:58:32
|
Yeah, trivial oversight, in the build-up to the rewritten async things
handling for 2.6... the consequences are not huge, as usually "all the
files fail" or "none", but still, this is the correct check.
Your patch has been applied to the master and release/2.6 branch (bugfix).
commit 2f8cbf5bc95b80832c84b0396cb7851bf5a2c579 (master)
commit da394db7477300c79953c8b0da710f62698756b0 (release/2.6)
Author: Arne Schwabe
Date: Thu Oct 30 20:39:34 2025 +0100
fix key_state_gen_auth_control_files probably checking file creation
Signed-off-by: Arne Schwabe <ar...@rf...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1327
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg34067.html
Signed-off-by: Gert Doering <ge...@gr...>
--
kind regards,
Gert Doering
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-30 20:58:47
|
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1327?usp=email ) Change subject: fix key_state_gen_auth_control_files probably checking file creation ...................................................................... fix key_state_gen_auth_control_files probably checking file creation When the auth_failed_reason_file was added, it was forgotten to also add it to the conditions that determine if the file creation was successful. Reported-by: Joshua Rogers <co...@jo...> Found-by: ZeroPath (https://zeropath.com/) Change-Id: I94d2bdd234a1c416b78924d044bf7e57f1bed8c4 Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1327 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg34067.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/ssl_verify.c 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 993d22c..a16f5fa 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -992,7 +992,7 @@ const char *apf = platform_create_temp_file(opt->tmp_dir, "apf", &gc); const char *afr = platform_create_temp_file(opt->tmp_dir, "afr", &gc); - if (acf && apf) + if (acf && apf && afr) { ads->auth_control_file = string_alloc(acf, NULL); ads->auth_pending_file = string_alloc(apf, NULL); @@ -1004,7 +1004,7 @@ } gc_free(&gc); - return (acf && apf); + return (acf && apf && afr); } /** -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1327?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: I94d2bdd234a1c416b78924d044bf7e57f1bed8c4 Gerrit-Change-Number: 1327 Gerrit-PatchSet: 2 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> |
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-30 20:58:51
|
cron2 has uploaded a new patch set (#2) to the change originally created by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/1327?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by cron2 Change subject: fix key_state_gen_auth_control_files probably checking file creation ...................................................................... fix key_state_gen_auth_control_files probably checking file creation When the auth_failed_reason_file was added, it was forgotten to also add it to the conditions that determine if the file creation was successful. Reported-by: Joshua Rogers <co...@jo...> Found-by: ZeroPath (https://zeropath.com/) Change-Id: I94d2bdd234a1c416b78924d044bf7e57f1bed8c4 Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1327 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg34067.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/ssl_verify.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/27/1327/2 diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 993d22c..a16f5fa 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -992,7 +992,7 @@ const char *apf = platform_create_temp_file(opt->tmp_dir, "apf", &gc); const char *afr = platform_create_temp_file(opt->tmp_dir, "afr", &gc); - if (acf && apf) + if (acf && apf && afr) { ads->auth_control_file = string_alloc(acf, NULL); ads->auth_pending_file = string_alloc(apf, NULL); @@ -1004,7 +1004,7 @@ } gc_free(&gc); - return (acf && apf); + return (acf && apf && afr); } /** -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1327?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: I94d2bdd234a1c416b78924d044bf7e57f1bed8c4 Gerrit-Change-Number: 1327 Gerrit-PatchSet: 2 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> |