Attention is currently required from: plaisthos.

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

The change is no longer submittable: Code-Review is unsatisfied now.

Change subject: Implement the --tls-export-cert feature
......................................................................


Patch Set 6: Code-Review-2

(4 comments)

Patchset:

PS6:
Tested, does not crash, but only exports level 0 cert (level 1 variable is set, 
but no such file exists).


File doc/man-sections/script-options.rst:

http://gerrit.openvpn.net/c/openvpn/+/466/comment/bff18d9b_f279e622 :
PS6, Line 426: --tls-export-cert-path dir
the manpage calls the option "tls-export-cert-path", while options.c checks for 
"tls-export-cert" (only, no "new option and also old option for compat 
reasons") - this needs to be resolved (and keeping the old option name 
everywhere is better for not breaking people's configs - so the documentation 
needs to be fixed)


File src/openvpn/init.c:

http://gerrit.openvpn.net/c/openvpn/+/466/comment/189e173c_161fa78f :
PS6, Line 3339:     to.export_peer_cert_dir = 
options->tls_export_peer_cert_path;
why call this "_dir" in the to, and "_path" in options-> ?


File src/openvpn/ssl_verify.c:

http://gerrit.openvpn.net/c/openvpn/+/466/comment/97340248_881cd351 :
PS6, Line 734:     if (opt->export_peer_cert_dir)
Something is not right here.  So the function does set up multiple environment 
variables, but only one file is ever created...

I do a "ls -l $peer_cert_2 $peer_cert_1 $peer_cert0" in my tls-verify-script, 
and this is what I see

peer_cert_1=/var/tmp/openvpn_pef_6a5f2055b342424a15139e5787303c57.tmp
peer_cert_0=/var/tmp/openvpn_pef_18e5d27eafdb9fb54c12a8c446b56c76.tmp
peer_cert=/var/tmp/openvpn_pef_18e5d27eafdb9fb54c12a8c446b56c76.tmp
-rw------- 1 root root 1830 Dec 18 18:39 
/var/tmp/openvpn_pef_18e5d27eafdb9fb54c12a8c446b56c76.tmp

... only one file.

For "multiple files", I would have expected to find the filenames in an array 
so they can all be deleted at the end (and no dangling files), but if only one 
file is ever created, no array is needed...



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/466?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: Ia9b3f1813d2d0d492d17c87348b4cebd0bf19ce2
Gerrit-Change-Number: 466
Gerrit-PatchSet: 6
Gerrit-Owner: plaisthos <arne-open...@rfc2549.org>
Gerrit-Reviewer: cron2 <g...@greenie.muc.de>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-Comment-Date: Mon, 18 Dec 2023 17:42:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to