From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH 1/3] ovpnmain.cgi: Updated fix for Bug#13137
Date: Sat, 10 Jun 2023 11:15:43 +0100
Message-ID: <9DB271BC-5F48-4DE2-824F-28644CA985E4@ipfire.org>
In-Reply-To: <20230607142150.18407-1-adolf.belka@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============3499847929094687912=="
List-Id: <development.lists.ipfire.org>

--===============3499847929094687912==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hello Adolf,

Since no comments (neither good or bad) arrived me, I merge this into all bra=
nches yesterday.

Once again, thank you very much for putting so much extra time into this pain=
ful problem.

I believe that we should be good for a release now - although I am bracing my=
self for the corner cases that we will discover over the next couple of month=
s or maybe even years. It is going to be fun!

-Michael

> On 7 Jun 2023, at 15:21, Adolf Belka <adolf.belka(a)ipfire.org> wrote:
>=20
> - This now only adds "providers legacy default" to the config files of conn=
ections that
>   have legacy certificates, both for n2n and roadwarrior.
> - This new approach also removes the requirement to have code in the update=
.sh script
>   or in backup.pl so those earlier modifications are removed in two additio=
nal patches
>   combined with this one in a set.
> - The -legacy option has been removed from the pkcs12 creation part of the =
code as
>   otherwise this creates a certificate in legacy format, which is not wante=
d. All new
>   connection certificates being created will be based on openssl-3.x
>=20
> Fixes: Bug#13137
> Suggested-by: Michael Tremer <michael.tremer(a)ipfire.org>
> Tested-by: Adolf Belka <adolf.belka(a)ipfire.org>
> Signed-off-by: Adolf Belka <adolf.belka(a)ipfire.org>
> ---
> html/cgi-bin/ovpnmain.cgi | 70 ++++++++++++++++++++++++++++++---------
> 1 file changed, 55 insertions(+), 15 deletions(-)
>=20
> diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
> index 88106251e..a210e0509 100755
> --- a/html/cgi-bin/ovpnmain.cgi
> +++ b/html/cgi-bin/ovpnmain.cgi
> @@ -138,6 +138,17 @@ unless (-e "$local_clientconf") {
> ###
> ### Useful functions
> ###
> +sub iscertlegacy
> +{
> + my $file=3D$_[0];
> + my @certinfo =3D &General::system_output("/usr/bin/openssl", "pkcs12", "-=
info", "-nodes",=20
> + "-in", "$file.p12", "-noout", "-passin", "pass:''");
> + if (index ($certinfo[0], "MAC: sha1") !=3D -1) {
> + return 0;
> + }
> + return 1;
> +}
> +
> sub haveOrangeNet
> {
> if ($netsettings{'CONFIG_TYPE'} =3D=3D 2) {return 1;}
> @@ -1115,7 +1126,9 @@ unless(-d "${General::swroot}/ovpn/n2nconf/$cgiparams=
{'NAME'}"){mkdir "${General
>   print CLIENTCONF "# Activate Management Interface and Port\n";
>   if ($cgiparams{'OVPN_MGMT'} eq '') {print CLIENTCONF "management localhos=
t $cgiparams{'DEST_PORT'}\n"}
>   else {print CLIENTCONF "management localhost $cgiparams{'OVPN_MGMT'}\n"};
> -  print CLIENTCONF "providers legacy default\n";
> +  if (&iscertlegacy("${General::swroot}/ovpn/certs/$cgiparams{'NAME'}")) {
> +     print CLIENTCONF "providers legacy default\n";
> +  }
>   close(CLIENTCONF);
>=20
> }
> @@ -1649,7 +1662,7 @@ END
> goto ROOTCERT_ERROR;
>    }
> } else { # child
> -    unless (exec ('/usr/bin/openssl', 'pkcs12', '-legacy', '-cacerts', '-n=
okeys',
> +    unless (exec ('/usr/bin/openssl', 'pkcs12', '-cacerts', '-nokeys',
>    '-in', $filename,
>    '-out', "$tempdir/cacert.pem")) {
> $errormessage =3D "$Lang::tr{'cant start openssl'}: $!";
> @@ -1672,7 +1685,7 @@ END
> goto ROOTCERT_ERROR;
>    }
> } else { # child
> -    unless (exec ('/usr/bin/openssl', 'pkcs12', '-legacy', '-clcerts', '-n=
okeys',
> +    unless (exec ('/usr/bin/openssl', 'pkcs12', '-clcerts', '-nokeys',
>    '-in', $filename,
>    '-out', "$tempdir/hostcert.pem")) {
> $errormessage =3D "$Lang::tr{'cant start openssl'}: $!";
> @@ -1695,7 +1708,7 @@ END
> goto ROOTCERT_ERROR;
>    }
> } else { # child
> -    unless (exec ('/usr/bin/openssl', 'pkcs12', '-legacy', '-nocerts',
> +    unless (exec ('/usr/bin/openssl', 'pkcs12', '-nocerts',
>    '-nodes',
>    '-in', $filename,
>    '-out', "$tempdir/serverkey.pem")) {
> @@ -2157,7 +2170,10 @@ if ($confighash{$cgiparams{'KEY'}}[3] eq 'net'){
>    if ($confighash{$cgiparams{'KEY'}}[22] eq '') {print CLIENTCONF "managem=
ent localhost $confighash{$cgiparams{'KEY'}}[29]\n"}
>     else {print CLIENTCONF "management localhost $confighash{$cgiparams{'KE=
Y'}}[22]\n"};
>    print CLIENTCONF "# remsub $confighash{$cgiparams{'KEY'}}[11]\n";
> -   print CLIENTCONF "providers legacy default\n";
> +  if (&iscertlegacy("${General::swroot}/ovpn/certs/$confighash{$cgiparams{=
'KEY'}}[1]")) {
> +     print CLIENTCONF "providers legacy default\n";
> +  }
> +
>=20
>=20
>     close(CLIENTCONF);
> @@ -2229,10 +2245,18 @@ else
>=20
> # Extract the certificate
> # This system call is safe, because all arguments are passed as an array.
> - system('/usr/bin/openssl', 'pkcs12', '-legacy', '-in', "${General::swroot=
}/ovpn/certs/$confighash{$cgiparams{'KEY'}}[1].p12",
> - '-clcerts', '-nokeys', '-nodes', '-out', "$file_crt" , '-passin', 'pass:'=
);
> - if ($?) {
> - die "openssl error: $?";
> + if (&iscertlegacy("${General::swroot}/ovpn/certs/$confighash{$cgiparams{'=
KEY'}}[1]")) {
> + system('/usr/bin/openssl', 'pkcs12', '-legacy', '-in', "${General::swroot=
}/ovpn/certs/$confighash{$cgiparams{'KEY'}}[1].p12",
> + '-clcerts', '-nokeys', '-nodes', '-out', "$file_crt" , '-passin', 'pass:'=
);
> + if ($?) {
> + die "openssl error: $?";
> + }
> + } else {
> + system('/usr/bin/openssl', 'pkcs12', '-in', "${General::swroot}/ovpn/cert=
s/$confighash{$cgiparams{'KEY'}}[1].p12",
> + '-clcerts', '-nokeys', '-nodes', '-out', "$file_crt" , '-passin', 'pass:'=
);
> + if ($?) {
> + die "openssl error: $?";
> + }
> }
>=20
> $zip->addFile("$file_crt", "$confighash{$cgiparams{'KEY'}}[1].pem") or die;
> @@ -2240,10 +2264,18 @@ else
>=20
> # Extract the key
> # This system call is safe, because all arguments are passed as an array.
> - system('/usr/bin/openssl', 'pkcs12', '-legacy', '-in', "${General::swroot=
}/ovpn/certs/$confighash{$cgiparams{'KEY'}}[1].p12",
> - '-nocerts', '-nodes', '-out', "$file_key", '-passin', 'pass:');
> - if ($?) {
> - die "openssl error: $?";
> + if (&iscertlegacy("${General::swroot}/ovpn/certs/$confighash{$cgiparams{'=
KEY'}}[1]")) {
> + system('/usr/bin/openssl', 'pkcs12', '-legacy', '-in', "${General::swroot=
}/ovpn/certs/$confighash{$cgiparams{'KEY'}}[1].p12",
> + '-nocerts', '-nodes', '-out', "$file_key", '-passin', 'pass:');
> + if ($?) {
> + die "openssl error: $?";
> + }
> + } else {
> + system('/usr/bin/openssl', 'pkcs12', '-in', "${General::swroot}/ovpn/cert=
s/$confighash{$cgiparams{'KEY'}}[1].p12",
> + '-nocerts', '-nodes', '-out', "$file_key", '-passin', 'pass:');
> + if ($?) {
> + die "openssl error: $?";
> + }
> }
>=20
> $zip->addFile("$file_key", "$confighash{$cgiparams{'KEY'}}[1].key") or die;
> @@ -2302,6 +2334,11 @@ else
>     # If the server is asking for TOTP this needs to happen interactively
>     print CLIENTCONF "auth-retry interact\r\n";
>=20
> +    # Add provider line if certificate is legacy type
> +    if (&iscertlegacy("${General::swroot}/ovpn/certs/$confighash{$cgiparam=
s{'KEY'}}[1]")) {
> + print CLIENTCONF "providers legacy default\r\n";
> +    }
> +
>     if ($include_certs) {
> print CLIENTCONF "\r\n";
>=20
> @@ -3298,7 +3335,10 @@ END
> print FILE "# Logfile\n";
> print FILE "status-version 1\n";
> print FILE "status /var/run/openvpn/$n2nname[0]-n2n 10\n";
> - print FILE "providers legacy default\n";
> + if (&iscertlegacy("${General::swroot}/ovpn/certs/$cgiparams{'n2nname'}"))=
 {
> +     print CLIENTCONF "providers legacy default\n";
> + }
> +
> close FILE;
>=20
> unless(move("$tempdir/$uplconffilename", "${General::swroot}/ovpn/n2nconf/$=
n2nname[0]/$uplconffilename2")) {
> @@ -4245,7 +4285,7 @@ if ($cgiparams{'TYPE'} eq 'net') {
>=20
>    # Create the pkcs12 file
>    # The system call is safe, because all arguments are passed as an array.
> -    system('/usr/bin/openssl', 'pkcs12', '-legacy', '-export',
> +    system('/usr/bin/openssl', 'pkcs12', '-export',
> '-inkey', "${General::swroot}/ovpn/certs/$cgiparams{'NAME'}key.pem",
> '-in', "${General::swroot}/ovpn/certs/$cgiparams{'NAME'}cert.pem",
> '-name', $cgiparams{'NAME'},
> --=20
> 2.40.1
>=20


--===============3499847929094687912==--