Re: [PATCH] OpenVPN: Introduce Negotiable Crypto Parameters for roadwarriors

[ummeegge <ummeegge@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 6619 bytes --]

Hi Michael,

Am Dienstag, den 07.08.2018, 14:10 +0100 schrieb Michael Tremer:
> 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.
Sure.

> 
> 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 from Fedora.
> > Please see explanations in https://fedoraproject.org/wiki/Changes/N
> > ew_default_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(-)
> > 
> > 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 {
> >      }	
> >      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-128-CBC:BF-CBC\n";
> > +    } else {
> > +        print CONF "ncp-disable\n";
> > +    }
> 
> Questions here:
> 
> 1) Why do we hard-code the cipher list?
There is also the possibility to set --ncp-ciphers only whereby OpenVPN
 uses then AES-256-GCM:AES-128-GCM per default. The linked Fedora
example uses in first place longer keys 256-GCM:256-CBC before 128-GCM
whereby i think longer keys (stronger encryption) where prefered to
128-GCM (192-GCM is also available and might be also an idea ?) .

The second point in there is if a client is 2.4 ready but uses older
OpenSSL libs from the system where no GCM is available, this --ncp-
ciphers list can nevertheless be used.

> 
> 2) Who would want to disable this as it should always peacefully co-
> exists with the "cipher" options.
The --ncp-ciphers option is currently completely disabled and not
available on IPFire and only --ciphers are used just before Core 120
and with OpenVPN-2.3.x .
I think this option is quiet neat if you have a lot´s of clients cause
an client update to >= 2.4 and clicking the checkbox on server side is
enough to change the encryption for very old configurations (BF-CBC old
default) to a strong cipher without the need to generate and distribute
new configuration files to each client.
Have often see that old unsecure configs has been used cause the hassle
to change all of them togehter was simply a lots of stress.

In my case, i won´t disable it cause as you said, it co-exists
peacfully beneath the old directives.

> 
> >      if ($sovpnsettings{'DAUTH'} eq '') {
> >          print CONF "";
> >      } else {
> > @@ -789,6 +794,7 @@ if ($cgiparams{'ACTION'} eq $Lang::tr{'save-
> > adv-options'}) {
> >      $vpnsettings{'ROUTES_PUSH'} = $cgiparams{'ROUTES_PUSH'};
> >      $vpnsettings{'DAUTH'} = $cgiparams{'DAUTH'};
> >      $vpnsettings{'TLSAUTH'} = $cgiparams{'TLSAUTH'};
> > +    $vpnsettings{'NCP'} = $cgiparams{'NCP'};
> >      my @temp=();
> >      
> >      if ($cgiparams{'FRAGMENT'} eq '') {
> > @@ -2685,6 +2691,9 @@ ADV_ERROR:
> >      $checked{'TLSAUTH'}{'off'} = '';
> >      $checked{'TLSAUTH'}{'on'} = '';
> >      $checked{'TLSAUTH'}{$cgiparams{'TLSAUTH'}} = 'CHECKED';
> > +    $checked{'NCP'}{'off'} = '';
> > +    $checked{'NCP'}{'on'} = '';
> > +    $checked{'NCP'}{$cgiparams{'NCP'}} = 'CHECKED';
> >     
> >      &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='100%'>
> > +    <tr>
> > +    <td width='20%'></td> <td width='15%'> </td><td width='15%'>
> > </td><td width='15%'></td><td width='35%'></td>
> > +    </tr>
> > +
> > +    <tr>
> > +         <td class='base'>$Lang::tr{'ovpn ncp'}</td>
> > +         <td><input type='checkbox' name='NCP'
> > $checked{'NCP'}{'on'} /></td>
> > +    </tr>
> > +
> > +    <tr>
> > +         <td class='base'>HMAC tls-auth</td>
> > +         <td><input type='checkbox' name='TLSAUTH'
> > $checked{'TLSAUTH'}{'on'} /></td>
> > +    </tr>
> > +
> >  	<tr>
> >  		<td width='20%'></td> <td width='30%'> </td><td
> > width='25%'> </td><td width='25%'></td>
> >      </tr>	
> > @@ -2833,17 +2858,8 @@ print <<END;
> >  		<td>$Lang::tr{'openvpn default'}: <span
> > class="base">SHA1 (160 $Lang::tr{'bit'})</span></td>
> >      </tr>
> >  </table>
> > +<hr size='1'>
> >  
> > -<table width='100%'>
> > -    <tr>
> > -	<td width='20%'></td> <td width='15%'> </td><td
> > width='15%'> </td><td width='15%'></td><td width='35%'></td>
> > -    </tr>
> > -
> > -    <tr>
> > -	<td class='base'>HMAC tls-auth</td>
> > -	<td><input type='checkbox' name='TLSAUTH'
> > $checked{'TLSAUTH'}{'on'} /></td>
> > -    </tr>
> > -    </table><hr>
> >  END
> >  
> >  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' => 'Deaktiviert',
> >  'ovpn mtu-disc with mssfix or fragment' => 'Path MTU Discovery
> > kann nicht gemeinsam mit mssfix oder fragment verwendet werden.',
> >  'ovpn mtu-disc yes' => 'Forciert',
> > +'ovpn ncp' => 'Verschlüsselung aushandeln',
> >  'ovpn no connections' => 'Keine aktiven OpenVPN Verbindungen',
> >  'ovpn on blue' => 'OpenVPN auf BLAU:',
> >  'ovpn on orange' => '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' => 'Disabled',
> >  'ovpn mtu-disc with mssfix or fragment' => 'Path MTU Discovery
> > cannot be used with mssfix or fragment.',
> >  'ovpn mtu-disc yes' => 'Forced',
> > +'ovpn ncp' => 'Negotiate encryption',
> 
> This doesn't fully explain to the user actually is being negotiated.
> The control channel? The data channel? TLS?
It is the data channel. The control channel uses since OpenVPN-2.4.x
 "TLSv1.2 ECDHE-RSA-AES256-GCM-SHA384, 2048 bit RSA" and can not be
configured.

If this patch might be interesting, i can also change the description.
If you have an good idea, let it me know.


Best,

Erik