[Openvpn-devel] Add daemon_pid to --tls-crypt-v2-verify environment

Message ID 20210429171504.1081888-1-tincantech@protonmail.com
State New
Headers show
Series [Openvpn-devel] Add daemon_pid to --tls-crypt-v2-verify environment | expand

Commit Message

Richard T Bonhomme April 29, 2021, 7:15 a.m. UTC
From: string vest <stringvest88@gmail.com>

Under Windows, programmatically retrieving the parent process ID of
the openvpn instance which called a script is practically impossible.
The only sensible way, currently available, is to write a PID file.

This patch adds a single integer variable, named daemon_pid, to the
script environment. The value of which is set to the openvpn process
ID that called the script.

Providing this variable via the running openvpn process is more secure,
faster and far less prone to user-error than using a PID file.

Signed-off-by: Richard T Bonhomme <tincantech@protonmail.com>
---
 src/openvpn/tls_crypt.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Kristof Provost via Openvpn-devel May 3, 2021, 7:22 a.m. UTC | #1
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Thursday, 29 April 2021 18:15, Richard T Bonhomme <stringvest88@gmail.com> wrote:

> From: string vest stringvest88@gmail.com
>
> Under Windows, programmatically retrieving the parent process ID of
> the openvpn instance which called a script is practically impossible.
> The only sensible way, currently available, is to write a PID file.
>
> This patch adds a single integer variable, named daemon_pid, to the
> script environment. The value of which is set to the openvpn process
> ID that called the script.
>
> Providing this variable via the running openvpn process is more secure,
> faster and far less prone to user-error than using a PID file.
>
> Signed-off-by: Richard T Bonhomme tincantech@protonmail.com
>
> src/openvpn/tls_crypt.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
> index 7b5016d3..23d93a6c 100644
> --- a/src/openvpn/tls_crypt.c
> +++ b/src/openvpn/tls_crypt.c
> @@ -537,6 +537,7 @@ tls_crypt_v2_verify_metadata(const struct tls_wrap_ctx *ctx,
> setenv_str(es, "script_type", "tls-crypt-v2-verify");
> setenv_str(es, "metadata_type", metadata_type_str);
> setenv_str(es, "metadata_file", tmp_file);
>
> -   setenv_int(es, "daemon_pid", platform_getpid());
>
>     struct argv argv = argv_new();
>     argv_parse_cmd(&argv, opt->tls_crypt_v2_verify_script);
>
>
> --
> 2.25.1

Bump.
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wsBzBAEBCAAGBQJgkDFOACEJEE+XnPZrkLidFiEECbw9RGejjXJ5xVVVT5ec
9muQuJ1Wywf/bDBG1X2K9a5NfjvSb5X2npD8VOq4d66Dy8uwDnhCkJoT5exm
MFRhaLYhQXXK22GVSqX/n7aNDly6HveyMRkuUzoDnKNMhxJ9NUfgwCpgc+Ap
5nJtYfss13mcaHQzwP1CPuQWpjupKQ4nAi+OWT3tPBhc0zkKq8O/VXOjff8g
KSE3WMlwCHrrXqZ5XV4Y8FqyeN0mqkVnhKfJy0UxKR1zh+E+a70cCT1mUR0x
mlBAXMDoS/p+EoIW6PqJNt+4qgzSQbH8b77XmAkR1eR9LS4GoZG1OHYkwQiW
e8SRm6tKLpjTIw9Ob9HTIoIt9kSjFfRVgBVyM37s2KSyeYG0YjPTAg==
=DN5K
-----END PGP SIGNATURE-----
Gert Doering May 3, 2021, 8:09 a.m. UTC | #2
Hi,

On Mon, May 03, 2021 at 05:22:26PM +0000, tincantech via Openvpn-devel wrote:
> Bump.

This is auto-deprioritizing review of the patch.

We have patchwork to remind us what is open.  Submitters that ask
multiple times a day about their patch needlessly eat up reviewer 
brain time, and this is not causing positive feelings.

gert
Arne Schwabe May 3, 2021, 8:15 a.m. UTC | #3
Am 03.05.21 um 19:22 schrieb tincantech via Openvpn-devel:
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Thursday, 29 April 2021 18:15, Richard T Bonhomme <stringvest88@gmail.com> wrote:
> 
>> From: string vest stringvest88@gmail.com
> 
>> Under Windows, programmatically retrieving the parent process ID of
>> the openvpn instance which called a script is practically impossible.
>> The only sensible way, currently available, is to write a PID file.
> 
>> This patch adds a single integer variable, named daemon_pid, to the
>> script environment. The value of which is set to the openvpn process
>> ID that called the script.
> 
>> Providing this variable via the running openvpn process is more secure,
>> faster and far less prone to user-error than using a PID file.
> 
>> Signed-off-by: Richard T Bonhomme tincantech@protonmail.com
> 
>> src/openvpn/tls_crypt.c | 1 +
>> 1 file changed, 1 insertion(+)
> 
>> diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
>> index 7b5016d3..23d93a6c 100644
>> --- a/src/openvpn/tls_crypt.c
>> +++ b/src/openvpn/tls_crypt.c
>> @@ -537,6 +537,7 @@ tls_crypt_v2_verify_metadata(const struct tls_wrap_ctx *ctx,
>> setenv_str(es, "script_type", "tls-crypt-v2-verify");
>> setenv_str(es, "metadata_type", metadata_type_str);
>> setenv_str(es, "metadata_file", tmp_file);
> 
>> -   setenv_int(es, "daemon_pid", platform_getpid());
> 
>>     struct argv argv = argv_new();
>>     argv_parse_cmd(&argv, opt->tls_crypt_v2_verify_script);
> 
> 
>> --
>> 2.25.1
> 
> Bump.

Are you trying to annoy the people that review patches? You should be
long enough on the mailing list to understand that reviews in under a
week are not often.

Bumping things with replying "bump" feel like a bad forum etiquette.

Arne
Arne Schwabe May 4, 2021, 12:50 a.m. UTC | #4
Am 29.04.21 um 19:15 schrieb Richard T Bonhomme:
> From: string vest <stringvest88@gmail.com>
> 
> Under Windows, programmatically retrieving the parent process ID of
> the openvpn instance which called a script is practically impossible.
> The only sensible way, currently available, is to write a PID file.
> 
> This patch adds a single integer variable, named daemon_pid, to the
> script environment. The value of which is set to the openvpn process
> ID that called the script.
> 
> Providing this variable via the running openvpn process is more secure,
> faster and far less prone to user-error than using a PID file.

Could you explain why you need the process ID of the daemon? I am trying
to figure out why that is needed. I also don't understand the secure in
this context. What are you protecting yourself against? You are not
protecting your script being called from a malicious program as that
could lookup the PID of openvpn and just set the daemon_id variable.

Arne
Kristof Provost via Openvpn-devel May 4, 2021, 2:43 a.m. UTC | #5
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Tuesday, 4 May 2021 11:50, Arne Schwabe <arne@rfc2549.org> wrote:

> Am 29.04.21 um 19:15 schrieb Richard T Bonhomme:
>
> > From: string vest stringvest88@gmail.com
> > Under Windows, programmatically retrieving the parent process ID of
> > the openvpn instance which called a script is practically impossible.
> > The only sensible way, currently available, is to write a PID file.
> > This patch adds a single integer variable, named daemon_pid, to the
> > script environment. The value of which is set to the openvpn process
> > ID that called the script.
> > Providing this variable via the running openvpn process is more secure,
> > faster and far less prone to user-error than using a PID file.
>
> Could you explain why you need the process ID of the daemon? I am trying
> to figure out why that is needed. I also don't understand the secure in
> this context. What are you protecting yourself against? You are not
> protecting your script being called from a malicious program as that
> could lookup the PID of openvpn and just set the daemon_id variable.
>

The reason I am using the process ID is as follows:

When --tls-crypt-v2-verify is executed, it saves a file named:
$(certificate_serial_number}.${daemon_pid}
with data from the TLS-Crypt-V2 key metadata field, which can then
be read by the following scripts: --tls-verify and --client-connect.

The --tls-verify and --client-connect script have:
$(certificate_serial_number} -> ${tls_serial_hex_0} and ${daemon_pid},
in their environment and can guarantee to pick-up the correct data file.

This is OK for one running server but when there are more than one server
instance running, using a PID file becomes messy and cumbersome.

The "secure" in this sense is that, having openvpn provide the PID is much
more reliable than relying on multiple PID files.

Also, while it is "trivial" for *nix to retrieve the Parent PID, under
Windows, programmatically doing this is not "trivial" at all:

PID:
https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/finding-the-process-id
PPID:
https://stackoverflow.com/questions/7486717/finding-parent-process-id-on-windows

Therefore, barring any known security reasons for not providing the openvpn PID
to all scripts which it executes, it makes more sense to have openvpn provide
daemon_pid.  The only script currently missing this data is --tls-crypt-v2-verify
(And probably --learn-address but I have not tested that).

Thanks
R
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wsBzBAEBCAAGBQJgkUFuACEJEE+XnPZrkLidFiEECbw9RGejjXJ5xVVVT5ec
9muQuJ0Zcgf+MpbxgsNS/eKpPsbafA5Qmdotc1HoQuxp+4mlw+Fr7uGxJT1y
cIAf5akt6ox+y/c0tOdFAPvczNirZh0j598TISFXbQtdEFG+budjBXK6peTc
ZKTlxvUSzZNterBcnjmCYYsQBxUdWrsH65cb23nvJ6G9m3dgkAPnt8w8NLe/
Z4/xHAElwU1kOoyGcpG4DMVQM55ikvXSmdDQx6BU8ksUueBHR4m3mMtkjFgq
krvjr+ycEZNcOX5601dOgNZS0AIT8TFvdFPEvMIXrSKJsmXtFLIXhLckM+3v
cUoV65+V3nQpdkJGumWHvCA1HB9nCSh75R8MdlD4mc0efaM2IiElog==
=KHtU
-----END PGP SIGNATURE-----
Kristof Provost via Openvpn-devel May 4, 2021, 9:02 a.m. UTC | #6
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Tuesday, 4 May 2021 13:43, tincantech via Openvpn-devel <openvpn-devel@lists.sourceforge.net> wrote:

> Hi,
>
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Tuesday, 4 May 2021 11:50, Arne Schwabe arne@rfc2549.org wrote:
>
> > Am 29.04.21 um 19:15 schrieb Richard T Bonhomme:
> >
> > > From: string vest stringvest88@gmail.com
> > > Under Windows, programmatically retrieving the parent process ID of
> > > the openvpn instance which called a script is practically impossible.
> > > The only sensible way, currently available, is to write a PID file.
> > > This patch adds a single integer variable, named daemon_pid, to the
> > > script environment. The value of which is set to the openvpn process
> > > ID that called the script.
> > > Providing this variable via the running openvpn process is more secure,
> > > faster and far less prone to user-error than using a PID file.
> >
> > Could you explain why you need the process ID of the daemon? I am trying
> > to figure out why that is needed. I also don't understand the secure in
> > this context. What are you protecting yourself against? You are not
> > protecting your script being called from a malicious program as that
> > could lookup the PID of openvpn and just set the daemon_id variable.
>
> The reason I am using the process ID is as follows:
>
> When --tls-crypt-v2-verify is executed, it saves a file named:
> $(certificate_serial_number}.${daemon_pid}
> with data from the TLS-Crypt-V2 key metadata field, which can then
> be read by the following scripts: --tls-verify and --client-connect.
>
> The --tls-verify and --client-connect script have:
> $(certificate_serial_number} -> ${tls_serial_hex_0} and ${daemon_pid},
> in their environment and can guarantee to pick-up the correct data file.
>
> This is OK for one running server but when there are more than one server
> instance running, using a PID file becomes messy and cumbersome.
>
> The "secure" in this sense is that, having openvpn provide the PID is much
> more reliable than relying on multiple PID files.
>
> Also, while it is "trivial" for *nix to retrieve the Parent PID, under
> Windows, programmatically doing this is not "trivial" at all:
>
> PID:
> https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/finding-the-process-id
> PPID:
> https://stackoverflow.com/questions/7486717/finding-parent-process-id-on-windows
>
> Therefore, barring any known security reasons for not providing the openvpn PID
> to all scripts which it executes, it makes more sense to have openvpn provide
> daemon_pid. The only script currently missing this data is --tls-crypt-v2-verify
> (And probably --learn-address but I have not tested that).
>

Due to the inordinate resistance this patch has received, consider this my official
withdrawal.  I hereby NACK.

Thanks
R

-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wsBzBAEBCAAGBQJgkZoxACEJEE+XnPZrkLidFiEECbw9RGejjXJ5xVVVT5ec
9muQuJ1HTQf7BSnvVR9LHZTcPyn+1oHv71TOxIMuFqckmxmQk/PZDSU+yq0h
OdjDWjSLLW/ZbQwS3Zcs09h50GEWBtUM5xoghAsBtUpGLCDMtvbU37JI/mMu
IfSI04+afMqi3xSsu1N4NMlAhVJTg2u0wfB6i46/Ltf/gLr9a0w3IAR7z1l4
Ykaxl5pBkNTZjuT6AtSVuVv8VUmr5+xQGWaUAxfPLIHeNeZGfCR7iJWd2L6L
zcnM8j3lLfzz1Tx2Ry3asVU40G6kp826F2LvuBH2mSZQeFENR/74HtAG0yY9
GcZg17oMkgBUmOZJzYupgrRwU1LFRGUIgk9ygS3Ew96M13C4lV90Sw==
=9B+C
-----END PGP SIGNATURE-----
Selva Nair May 4, 2021, 9:41 a.m. UTC | #7
On Tue, May 4, 2021 at 3:04 PM tincantech via Openvpn-devel
<openvpn-devel@lists.sourceforge.net> wrote:
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Hi,
>
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Tuesday, 4 May 2021 13:43, tincantech via Openvpn-devel <openvpn-devel@lists.sourceforge.net> wrote:
>
> > Hi,
> >
> > ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> > On Tuesday, 4 May 2021 11:50, Arne Schwabe arne@rfc2549.org wrote:
> >
> > > Am 29.04.21 um 19:15 schrieb Richard T Bonhomme:
> > >
> > > > From: string vest stringvest88@gmail.com
> > > > Under Windows, programmatically retrieving the parent process ID of
> > > > the openvpn instance which called a script is practically impossible.
> > > > The only sensible way, currently available, is to write a PID file.
> > > > This patch adds a single integer variable, named daemon_pid, to the
> > > > script environment. The value of which is set to the openvpn process
> > > > ID that called the script.
> > > > Providing this variable via the running openvpn process is more secure,
> > > > faster and far less prone to user-error than using a PID file.
> > >
> > > Could you explain why you need the process ID of the daemon? I am trying
> > > to figure out why that is needed. I also don't understand the secure in
> > > this context. What are you protecting yourself against? You are not
> > > protecting your script being called from a malicious program as that
> > > could lookup the PID of openvpn and just set the daemon_id variable.
> >
> > The reason I am using the process ID is as follows:
> >
> > When --tls-crypt-v2-verify is executed, it saves a file named:
> > $(certificate_serial_number}.${daemon_pid}
> > with data from the TLS-Crypt-V2 key metadata field, which can then
> > be read by the following scripts: --tls-verify and --client-connect.
> >
> > The --tls-verify and --client-connect script have:
> > $(certificate_serial_number} -> ${tls_serial_hex_0} and ${daemon_pid},
> > in their environment and can guarantee to pick-up the correct data file.
> >
> > This is OK for one running server but when there are more than one server
> > instance running, using a PID file becomes messy and cumbersome.
> >
> > The "secure" in this sense is that, having openvpn provide the PID is much
> > more reliable than relying on multiple PID files.
> >
> > Also, while it is "trivial" for *nix to retrieve the Parent PID, under
> > Windows, programmatically doing this is not "trivial" at all:
> >
> > PID:
> > https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/finding-the-process-id
> > PPID:
> > https://stackoverflow.com/questions/7486717/finding-parent-process-id-on-windows
> >
> > Therefore, barring any known security reasons for not providing the openvpn PID
> > to all scripts which it executes, it makes more sense to have openvpn provide
> > daemon_pid. The only script currently missing this data is --tls-crypt-v2-verify
> > (And probably --learn-address but I have not tested that).
> >
>
> Due to the inordinate resistance this patch has received, consider this my official
> withdrawal.  I hereby NACK.

Resistance is a good thing -- it means people are considering your
patch seriously and are asking questions in earnest.

I've had patches that languished for  years and finally merged,
without batting an eye.. Except for an occasional gentle nudge (say
once a year), and some patience.


Selva
Kristof Provost via Openvpn-devel May 4, 2021, 10:36 a.m. UTC | #8
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Tuesday, 4 May 2021 20:41, Selva Nair <selva.nair@gmail.com> wrote:

> On Tue, May 4, 2021 at 3:04 PM tincantech via Openvpn-devel
> openvpn-devel@lists.sourceforge.net wrote:
>

<snip>

> > Due to the inordinate resistance this patch has received, consider this my official
> > withdrawal. I hereby NACK.
>
> Resistance is a good thing -- it means people are considering your
> patch seriously and are asking questions in earnest.
>
> I've had patches that languished for years and finally merged,
> without batting an eye.. Except for an occasional gentle nudge (say
> once a year), and some patience.

Selva, thanks for your guidance.

On this occasion I have been pushed to find a better way.
Thanks to Gert for reminding me about PPID and pushing my code a little further,
I now realise that I don't need daemon_pid from openvpn directly.

Withdrawing this patch seems the most appropriate way to minimize the time wasted.

Sometimes it's difficult to see your own mistakes but realisation is satisfying
if and when you do "break on through" ;-)

R


-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wsBzBAEBCAAGBQJgkbBPACEJEE+XnPZrkLidFiEECbw9RGejjXJ5xVVVT5ec
9muQuJ0MYwgAuFchojIT/cnf0yppB9o/WxJRZHdFaEn9jyRO+PgMYNrAJeet
QUqaCbUPZ0N5UdXnwuf5DTMYcStQpDyKnQyeB/dw0r23fmlSV69U1Vdx64+x
kjfbxl3h3miJ5yRu62YnmSCtqtiC/ErrJ1bz68RglI/aeGD4g6nPkpoHhZ/O
ix1zPxOpV+fnjEZtZfRCzNah+wa1vWyxF/UFpbIUe/pME6Y1pCGf4ZWGPFG8
qVdxSU/GwMMIaQn+Kz+iwoZDzhbkkprHGwS/yvJkEBIXOf8SspnlOOH0gJ6J
yFvxCBmaLeFUDoHhOy9JdL5toUN6hCtwu1wKPnUC3xN0IypeGdHtrQ==
=a9Jn
-----END PGP SIGNATURE-----
Kristof Provost via Openvpn-devel May 4, 2021, 11:49 a.m. UTC | #9
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256




Sent with ProtonMail Secure Email.

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Tuesday, 4 May 2021 21:36, tincantech via Openvpn-devel <openvpn-devel@lists.sourceforge.net> wrote:

> Hi,
>
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Tuesday, 4 May 2021 20:41, Selva Nair selva.nair@gmail.com wrote:
>
> > On Tue, May 4, 2021 at 3:04 PM tincantech via Openvpn-devel
> > openvpn-devel@lists.sourceforge.net wrote:
>
> <snip>
>
> > > Due to the inordinate resistance this patch has received, consider this my official
> > > withdrawal. I hereby NACK.
> >
> > Resistance is a good thing -- it means people are considering your
> > patch seriously and are asking questions in earnest.

<s>

And this resistance was indeed not futile.

I have discovered a flaw in my logic, which is so insidious, that it may
bring my entire house of cards down .. I just F000M'^~,.ed myself .. arse!

-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wsBzBAEBCAAGBQJgkcFnACEJEE+XnPZrkLidFiEECbw9RGejjXJ5xVVVT5ec
9muQuJ3msAgAsoR4DghKD6z/et3JYXabmsGny+5/hu48E1FMXGUH5cB/cWpM
5P+KL3Kr3D7MkemqbHvvapQQKn0DCA+Kt056fqQI8h9fc8vhJKLYAxFD4M8E
b60H8+/K5bSwUxVkH4X6jW8m/HJ16q8fBrTSRDbZeJ4x1u1u0uxTk84WVhW2
gjued3xLGhPlkBlufYayr6LytUXN5wDEJSKLgjeROl04NlvTlDc1VCu+QREw
KnqXh1JBg9Lqo5ctmNtV6QZ8R5nx9G3cNqJ0joRQfN329H1Bp30S6iyvkBMV
/7pgCyKQ1X38D5rXNcRL/4qP0YfcSkQ43zb2dur9LtQ/wFqepKwr4Q==
=IcL9
-----END PGP SIGNATURE-----
Arne Schwabe May 4, 2021, 9:51 p.m. UTC | #10
>> Could you explain why you need the process ID of the daemon? I am trying
>> to figure out why that is needed. I also don't understand the secure in
>> this context. What are you protecting yourself against? You are not
>> protecting your script being called from a malicious program as that
>> could lookup the PID of openvpn and just set the daemon_id variable.
> 
> 
> The reason I am using the process ID is as follows:
> 
> When --tls-crypt-v2-verify is executed, it saves a file named:
> $(certificate_serial_number}.${daemon_pid}
> with data from the TLS-Crypt-V2 key metadata field, which can then
> be read by the following scripts: --tls-verify and --client-connect.
>

I can get behind the need of needing something daemon specific when
running multiple daemon that scripts/plugins need something simple to
identify a specific daemon. With management and a persistent connection
that is easier to implicitly assign an ID but for scripts daemon_pid
seems to be a good fit.

So if we make that a bit clear in the commit message this gets an ACK
from me.

Arne
Kristof Provost via Openvpn-devel May 5, 2021, noon UTC | #11
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Wednesday, 5 May 2021 08:51, Arne Schwabe <arne@rfc2549.org> wrote:

> > > Could you explain why you need the process ID of the daemon? I am trying
> > > to figure out why that is needed. I also don't understand the secure in
> > > this context. What are you protecting yourself against? You are not
> > > protecting your script being called from a malicious program as that
> > > could lookup the PID of openvpn and just set the daemon_id variable.
> >
> > The reason I am using the process ID is as follows:
> > When --tls-crypt-v2-verify is executed, it saves a file named:
> > $(certificate_serial_number}.${daemon_pid}
> > with data from the TLS-Crypt-V2 key metadata field, which can then
> > be read by the following scripts: --tls-verify and --client-connect.
>
> I can get behind the need of needing something daemon specific when
> running multiple daemon that scripts/plugins need something simple to
> identify a specific daemon. With management and a persistent connection
> that is easier to implicitly assign an ID but for scripts daemon_pid
> seems to be a good fit.
>
> So if we make that a bit clear in the commit message this gets an ACK
> from me.
>

Arne,

thanks for the feedback, I can resubmit with an improved commit message
and corrected email if required.

Thanks
R
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wsBzBAEBCAAGBQJgkxVyACEJEE+XnPZrkLidFiEECbw9RGejjXJ5xVVVT5ec
9muQuJ0PGggAtMnaL8kv8Z2xGvqMkSr+TO7kHLWl2OoYP+o+S18NpUpQrLn1
1Yr0t2ZHjdho30l24iMsKGYAgtPwXfmNgNI+tjhb2p7URRNgkfaDDDUiTePL
hfnZnjLdmjlCIurKNnCqFsVKj92C2jQbicLcCH+504a0TeTLGmWaCYQ3/QiE
2I5CUJErNmjXrBRTeS5hB7FLSbYzbAs1AC5dU7uGSxjnhPrT1tx7An/GNAc6
HJMMBhF1if98jvPRntG6zXLTC4nIFPEM73m9oyWyrwXPql0lD8hZJ08OnpxI
YyfsH3KEFc3f6st2pmAen8o31zuDxARpYdhusqiLzqWW0WbCj1lt7A==
=ClM7
-----END PGP SIGNATURE-----
Gert Doering May 10, 2021, 7:29 a.m. UTC | #12
Hi,

On Wed, May 05, 2021 at 10:00:37PM +0000, tincantech via Openvpn-devel wrote:
> thanks for the feedback, I can resubmit with an improved commit message
> and corrected email if required.

This is how I understand Arne - he's happy with the code change, just
wants the commit message to explain a bit better why this is relevant.

So, there is an ACK-and-merge pending :-)

gert
Kristof Provost via Openvpn-devel May 10, 2021, 10:23 a.m. UTC | #13
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, 10 May 2021 18:29, Gert Doering <gert@greenie.muc.de> wrote:

> Hi,
>
> On Wed, May 05, 2021 at 10:00:37PM +0000, tincantech via Openvpn-devel wrote:
>
> > thanks for the feedback, I can resubmit with an improved commit message
> > and corrected email if required.
>
> This is how I understand Arne - he's happy with the code change, just
> wants the commit message to explain a bit better why this is relevant.
>
> So, there is an ACK-and-merge pending :-)
>

well, personally.. it's complicated..

1. Antonio's initial reluctance.

I do not believe it would be prudent to continue without his opinion.

2. Replacement method.

This is no longer required. (Thanks given to Gert for PPID)

3. Work

Rewriting the commit message to suit, plus resubmitting as a V2 .. and ..
then jumping through googles and gits hoops is not at the top of my list.

4. Having my user account _recently_ locked out of Trac.

I expect that I can even ''predict'' what that is with regard to.

...

For the time being, I suggest that TLS-Crypt-V2 code be left as-is,
because TLS-Crypt-V2 *has* more important issues to consider..
I would list the trac issue but #4

The impasse has been met,
R
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wsBzBAEBCAAGBQJgmZY5ACEJEE+XnPZrkLidFiEECbw9RGejjXJ5xVVVT5ec
9muQuJ1sMggAnsGf77yUfrERZRY67k3zTPqRuXuVegRKSzyAlumJXaMSyY81
JMD1m3s5vQSE2EyH/b+3U0jlZIlnKTRVvLE/YJYKiEEFNm0LuWy1dc7jpdwo
vWeGI3O442zngXzk4SRnHRNP1e11jwPtlh3zZevlMHMwgzpKE+xkpT+9ySIP
bGNyHO25odJy4lqwpvF54C2IL9Pokh5u3/Ij7vdESE/X+WLkS/I2nPJFMkLj
ls4Hdyxfhyh/ekiVPDEkyioAEG00FqVsVYvZrMpsbu2wmwP6eX8Jk1jJVZ1i
FRyEaUVJaxzmCr1pqt8Nzu46uL4Pt3xdenxOo6O09SZzCNaPqYcsIQ==
=dlZP
-----END PGP SIGNATURE-----
Kristof Provost via Openvpn-devel May 10, 2021, 12:01 p.m. UTC | #14
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,


> 4.  Having my user account recently locked out of Trac.
>
>     I expect that I can even ''predict'' what that is with regard to.
>
>     ...
>

Seems I typed my ludicrously long password incorrectly..

The rest still stands.

Sorry for the noise,
R
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wsBzBAEBCAAGBQJgma0dACEJEE+XnPZrkLidFiEECbw9RGejjXJ5xVVVT5ec
9muQuJ3aSggAs9IxRPp2KmNfmP8o16gHzMA1X7MpncTWmWHRsVsrAv7daAbg
LUX310MIqsz1tOb+dG7TqHiXjfhO7VH5L7DJfjm/zRdVIWyHBtYM3CBeleq+
zsHwOYF1k8pyRPMmOIc+mYPpXuk3hb9xvNBvLpdEBsJI7HYE9JTfaLAJNaOi
zoIWHUtcG6sc6pP0as0EDoT1kKhN0j0j/BWkxIvNMOENGCTPyHn4MX0aFmdp
Hpzva/0zHIbd5MFnDMH3v20thBOp3EhOaBxD0uXn3S4J0NxzNXeOtHaoQSFF
W9zefCIUEFxiP7yxae644Cw9FLqjSD+uRyAeJb9YkPwXVxNWJdVzrw==
=NgsE
-----END PGP SIGNATURE-----
Selva Nair May 10, 2021, 12:10 p.m. UTC | #15
On Mon, May 10, 2021 at 4:24 PM tincantech via Openvpn-devel
<openvpn-devel@lists.sourceforge.net> wrote:
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Hi,
>
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Monday, 10 May 2021 18:29, Gert Doering <gert@greenie.muc.de> wrote:
>
> > Hi,
> >
> > On Wed, May 05, 2021 at 10:00:37PM +0000, tincantech via Openvpn-devel wrote:
> >
> > > thanks for the feedback, I can resubmit with an improved commit message
> > > and corrected email if required.
> >
> > This is how I understand Arne - he's happy with the code change, just
> > wants the commit message to explain a bit better why this is relevant.
> >
> > So, there is an ACK-and-merge pending :-)
> >
>
> well, personally.. it's complicated..

I think it's a good thing to pass daemon_pid to all scripts uniformly.
Can't think of any downside. And, on Windows it's a pain to get the
parent pid from a batch file. Personally, I do not have a use case
though.


Selva
Kristof Provost via Openvpn-devel May 10, 2021, 12:22 p.m. UTC | #16
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, 10 May 2021 23:10, Selva Nair <selva.nair@gmail.com> wrote:

> On Mon, May 10, 2021 at 4:24 PM tincantech via Openvpn-devel
> openvpn-devel@lists.sourceforge.net wrote:
>
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA256
> > Hi,
> > ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> > On Monday, 10 May 2021 18:29, Gert Doering gert@greenie.muc.de wrote:
> >
> > > Hi,
> > > On Wed, May 05, 2021 at 10:00:37PM +0000, tincantech via Openvpn-devel wrote:
> > >
> > > > thanks for the feedback, I can resubmit with an improved commit message
> > > > and corrected email if required.
> > >
> > > This is how I understand Arne - he's happy with the code change, just
> > > wants the commit message to explain a bit better why this is relevant.
> > > So, there is an ACK-and-merge pending :-)
> >
> > well, personally.. it's complicated..
>
> I think it's a good thing to pass daemon_pid to all scripts uniformly.
> Can't think of any downside. And, on Windows it's a pain to get the
> parent pid from a batch file. Personally, I do not have a use case
> though.
>
> Selva

Trac in question: https://community.openvpn.net/openvpn/ticket/1310
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wsBzBAEBCAAGBQJgmbIfACEJEE+XnPZrkLidFiEECbw9RGejjXJ5xVVVT5ec
9muQuJ2QrQgAoH7L8LCDvs8Tp9mj/VVR/aolKox2hHQJqfEcZXsEfKySmFUC
8oDFY9bnlOiGg1LhEaLIITXkYlW2aTB11Sm1DE/hYy1MK/IxByzwRa1AJyCk
6TJtmoiMl7Inwxz6z/IOCpSDCdOR+/i+BXaXx8pJujn7omr9Vprgkku78I6s
2unDfIycBWwMD78pCULJvqnOPzCM5TkA82x6WdcpJykLaeOOX+do7CrkNmxC
s9Hfl7duiqGgSCLAZOZv71CwvyoJYorHpO6yhB+UxikhszFANXRCEU4AXoqR
jRL8yH7ouz92mR9vospC39lwAMJGthBQ85l8Sj5ngaiVrvBj3qfh6g==
=WQSw
-----END PGP SIGNATURE-----
Gert Doering May 10, 2021, 8:02 p.m. UTC | #17
Hi,

On Mon, May 10, 2021 at 06:10:33PM -0400, Selva Nair wrote:
> > > So, there is an ACK-and-merge pending :-)
[..]
> I think it's a good thing to pass daemon_pid to all scripts uniformly.
> Can't think of any downside. And, on Windows it's a pain to get the
> parent pid from a batch file. Personally, I do not have a use case
> though.

This was about the thoughts Arne and I had - if we have daemon_pid in
some places, we should have it in all places.  And, on Windows it's not
as trivial as on Unix...

(Out of interest: is there a way at all to get the ppid from a batch file?)

gert
Kristof Provost via Openvpn-devel May 11, 2021, 1:07 a.m. UTC | #18
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256




Sent with ProtonMail Secure Email.

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Tuesday, 11 May 2021 07:02, Gert Doering <gert@greenie.muc.de> wrote:

> Hi,
>
> On Mon, May 10, 2021 at 06:10:33PM -0400, Selva Nair wrote:
>
> > > > So, there is an ACK-and-merge pending :-)
>
> [..]
>
> > I think it's a good thing to pass daemon_pid to all scripts uniformly.
> > Can't think of any downside. And, on Windows it's a pain to get the
> > parent pid from a batch file. Personally, I do not have a use case
> > though.
>
> This was about the thoughts Arne and I had - if we have daemon_pid in
> some places, we should have it in all places. And, on Windows it's not
> as trivial as on Unix...
>
> (Out of interest: is there a way at all to get the ppid from a batch file?)
>

This is what I found for PID/PPID in batch:


PID:
https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/finding-the-process-id
PPID:
https://stackoverflow.com/questions/7486717/finding-parent-process-id-on-windows

Thanks
R
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wsBzBAEBCAAGBQJgmmV+ACEJEE+XnPZrkLidFiEECbw9RGejjXJ5xVVVT5ec
9muQuJ0BCgf9Ffzik7MJhQtRjPBc2L/ZCpcQxLJJTHSB9c//g0YetY2/hEyG
ReuEW9G7AnSoZTPep8Xt502rPJZtyxut3kmY79J9Pt/NTD7siV4+f4ZUg24V
lPDqWpsVhqD0EeeiPqWa/6OhZWmgT4qXMnYyPznCHdzYlcjYAZARJB4EWeE6
baf6RQFfJ1cjhNY07jaeMJi3SW72J5RjdlLFPfKITfrPgIuzFhFc6rvmyplU
Sz41k1Bd1QprZwIGE7JiZDLajOmYkmGUaqXQ6AoLWmTZJACNFDKyQZYXs7lY
wwoROF6u14vxLh2TeQ1btfuxnGUs2HhpqZX80TrxAm80EQyBrnJ5bA==
=17mP
-----END PGP SIGNATURE-----

Patch

diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 7b5016d3..23d93a6c 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -537,6 +537,7 @@  tls_crypt_v2_verify_metadata(const struct tls_wrap_ctx *ctx,
     setenv_str(es, "script_type", "tls-crypt-v2-verify");
     setenv_str(es, "metadata_type", metadata_type_str);
     setenv_str(es, "metadata_file", tmp_file);
+    setenv_int(es, "daemon_pid", platform_getpid());
 
     struct argv argv = argv_new();
     argv_parse_cmd(&argv, opt->tls_crypt_v2_verify_script);