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] OpenVPN: Introduce Negotiable Crypto Parameters for roadwarriors
Date: Tue, 07 Aug 2018 14:10:04 +0100
Message-ID: <1d03e9d4b66fd0d6476ace67f309914d8d0378da.camel@ipfire.org>
In-Reply-To: <1533540354-4387-1-git-send-email-erik.kapfer@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============3485985018463196947=="
List-Id: <development.lists.ipfire.org>

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

Hello,

hmm, I am not sure if I agree with the patch.

Could you answer some questions so that I understand better what the
implications are.

On Mon, 2018-08-06 at 09:25 +0200, Erik Kapfer wrote:
> The ncp-ciphers differs to the OpenVPN default value and has been adapted f=
rom Fedora.
> Please see explanations in https://fedoraproject.org/wiki/Changes/New_defau=
lt_cipher_in_OpenVPN .
> ---
>  html/cgi-bin/ovpnmain.cgi | 38 +++++++++++++++++++++++++++-----------
>  langs/de/cgi-bin/de.pl    |  1 +
>  langs/en/cgi-bin/en.pl    |  1 +
>  3 files changed, 29 insertions(+), 11 deletions(-)
>=20
> diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
> index 976300f..dc22ba5 100644
> --- a/html/cgi-bin/ovpnmain.cgi
> +++ b/html/cgi-bin/ovpnmain.cgi
> @@ -321,8 +321,13 @@ sub writeserverconf {
>      }=09
>      print CONF "status-version 1\n";
>      print CONF "status /var/run/ovpnserver.log 30\n";
> -    print CONF "ncp-disable\n";
>      print CONF "cipher $sovpnsettings{DCIPHER}\n";
> +    # Enable Negotiable Crypto Parameters
> +    if ($sovpnsettings{'NCP'} eq 'on') {
> +         print CONF "ncp-ciphers AES-256-GCM:AES-256-CBC:AES-128-GCM:AES-1=
28-CBC:BF-CBC\n";
> +    } else {
> +        print CONF "ncp-disable\n";
> +    }

Questions here:

1) Why do we hard-code the cipher list?

2) Who would want to disable this as it should always peacefully co-
exists with the "cipher" options.

>      if ($sovpnsettings{'DAUTH'} eq '') {
>          print CONF "";
>      } else {
> @@ -789,6 +794,7 @@ if ($cgiparams{'ACTION'} eq $Lang::tr{'save-adv-options=
'}) {
>      $vpnsettings{'ROUTES_PUSH'} =3D $cgiparams{'ROUTES_PUSH'};
>      $vpnsettings{'DAUTH'} =3D $cgiparams{'DAUTH'};
>      $vpnsettings{'TLSAUTH'} =3D $cgiparams{'TLSAUTH'};
> +    $vpnsettings{'NCP'} =3D $cgiparams{'NCP'};
>      my @temp=3D();
>     =20
>      if ($cgiparams{'FRAGMENT'} eq '') {
> @@ -2685,6 +2691,9 @@ ADV_ERROR:
>      $checked{'TLSAUTH'}{'off'} =3D '';
>      $checked{'TLSAUTH'}{'on'} =3D '';
>      $checked{'TLSAUTH'}{$cgiparams{'TLSAUTH'}} =3D 'CHECKED';
> +    $checked{'NCP'}{'off'} =3D '';
> +    $checked{'NCP'}{'on'} =3D '';
> +    $checked{'NCP'}{$cgiparams{'NCP'}} =3D 'CHECKED';
>    =20
>      &Header::showhttpheaders();
>      &Header::openpage($Lang::tr{'status ovpn'}, 1, '');
> @@ -2818,6 +2827,22 @@ print <<END;
>      <tr>
>  		<td class'base'><b>$Lang::tr{'ovpn crypt options'}</b></td>
>  	</tr>
> +
> +<table width=3D'100%'>
> +    <tr>
> +    <td width=3D'20%'></td> <td width=3D'15%'> </td><td width=3D'15%'> </t=
d><td width=3D'15%'></td><td width=3D'35%'></td>
> +    </tr>
> +
> +    <tr>
> +         <td class=3D'base'>$Lang::tr{'ovpn ncp'}</td>
> +         <td><input type=3D'checkbox' name=3D'NCP' $checked{'NCP'}{'on'} /=
></td>
> +    </tr>
> +
> +    <tr>
> +         <td class=3D'base'>HMAC tls-auth</td>
> +         <td><input type=3D'checkbox' name=3D'TLSAUTH' $checked{'TLSAUTH'}=
{'on'} /></td>
> +    </tr>
> +
>  	<tr>
>  		<td width=3D'20%'></td> <td width=3D'30%'> </td><td width=3D'25%'> </td>=
<td width=3D'25%'></td>
>      </tr>=09
> @@ -2833,17 +2858,8 @@ print <<END;
>  		<td>$Lang::tr{'openvpn default'}: <span class=3D"base">SHA1 (160 $Lang::=
tr{'bit'})</span></td>
>      </tr>
>  </table>
> +<hr size=3D'1'>
> =20
> -<table width=3D'100%'>
> -    <tr>
> -	<td width=3D'20%'></td> <td width=3D'15%'> </td><td width=3D'15%'> </td><=
td width=3D'15%'></td><td width=3D'35%'></td>
> -    </tr>
> -
> -    <tr>
> -	<td class=3D'base'>HMAC tls-auth</td>
> -	<td><input type=3D'checkbox' name=3D'TLSAUTH' $checked{'TLSAUTH'}{'on'} /=
></td>
> -    </tr>
> -    </table><hr>
>  END
> =20
>  if ( -e "/var/run/openvpn.pid"){
> diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl
> index 6e3dba4..9f0de6b 100644
> --- a/langs/de/cgi-bin/de.pl
> +++ b/langs/de/cgi-bin/de.pl
> @@ -1833,6 +1833,7 @@
>  'ovpn mtu-disc off' =3D> 'Deaktiviert',
>  'ovpn mtu-disc with mssfix or fragment' =3D> 'Path MTU Discovery kann nich=
t gemeinsam mit mssfix oder fragment verwendet werden.',
>  'ovpn mtu-disc yes' =3D> 'Forciert',
> +'ovpn ncp' =3D> 'Verschl=C3=BCsselung aushandeln',
>  'ovpn no connections' =3D> 'Keine aktiven OpenVPN Verbindungen',
>  'ovpn on blue' =3D> 'OpenVPN auf BLAU:',
>  'ovpn on orange' =3D> 'OpenVPN auf ORANGE:',
> diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
> index 3ec5af5..5cd47b1 100644
> --- a/langs/en/cgi-bin/en.pl
> +++ b/langs/en/cgi-bin/en.pl
> @@ -1866,6 +1866,7 @@
>  'ovpn mtu-disc off' =3D> 'Disabled',
>  'ovpn mtu-disc with mssfix or fragment' =3D> 'Path MTU Discovery cannot be=
 used with mssfix or fragment.',
>  'ovpn mtu-disc yes' =3D> 'Forced',
> +'ovpn ncp' =3D> 'Negotiate encryption',

This doesn't fully explain to the user actually is being negotiated.
The control channel? The data channel? TLS?

>  'ovpn no connections' =3D> 'No active OpenVPN connections',
>  'ovpn on blue' =3D> 'OpenVPN on BLUE:',
>  'ovpn on orange' =3D> 'OpenVPN on ORANGE:',

Best,
-Michael


--===============3485985018463196947==--