diff mbox series

[v2] OpenVPN: Introduce new AES-GCM cipher for N2N and RW

Message ID 1518619253-22278-1-git-send-email-erik.kapfer@ipfire.org
State Dropped
Headers
Series [v2] OpenVPN: Introduce new AES-GCM cipher for N2N and RW |

Commit Message

Erik Kapfer Feb. 15, 2018, 1:40 a.m. UTC 
  AES-GCM 128, 196 and 256 bit has been added to Net-to-Net and Roadwarrior section.

Cipher menu description has been changed for N2N and RW since AES-GCM uses own authentication encryption (GMAC).
    More information can be found in here https://tools.ietf.org/html/rfc5288 .
Added java script snipped to disable HMAC selection for N2N if AES-GCM has been selected.
    'auth *' line in N2N.conf won´t be deleted even if AES-GCM is used so possible individual '--tls-auth' configurations won´t broke.
    'auth *' line in N2N.conf will also be ignored if AES-GCM is used and no '--tls-auth' are configured.
Left HMAC selection menu for Roadwarriors as it was since the WUI do provides '--tls-auth' which uses the configuered HMAC even AES-GCM has been applied.

Signed-off-by: Erik Kapfer <erik.kapfer@ipfire.org>
---
 html/cgi-bin/ovpnmain.cgi | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

Comments

ummeegge Feb. 15, 2018, 6:11 a.m. UTC | #1 
As a version 3 idea,
or might it be possibly a better idea to delete the '--auth *' directive in N2N.conf
if AES-GCM has been chosen ? i think it might also be better to integrate
'--tls-crypt' --> https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg12357.html 
instead of '--tls-auth' to N2N connections which uses a static AES-256-CTR whereby
a HMAC can not be selected ?

But also it might be time to delete SHA1 complete from Net-to-Net HMAC
selection since this won´t harm old connections but brings a little
more security per default ?

Sorry for the back and forth but the way is the goal :D .

Some feedback might be nevertheless nice and important.

Greetings,

Erik


Am Mittwoch, den 14.02.2018, 15:40 +0100 schrieb Erik Kapfer:
> AES-GCM 128, 196 and 256 bit has been added to Net-to-Net and
> Roadwarrior section.
> 
> Cipher menu description has been changed for N2N and RW since AES-GCM 
> uses own authentication encryption (GMAC).
>     More information can be found in here https://tools.ietf.org/html
> /rfc5288 .
> Added java script snipped to disable HMAC selection for N2N if AES-
> GCM has been selected.
>     'auth *' line in N2N.conf won´t be deleted even if AES-GCM is
> used so possible individual '--tls-auth' configurations won´t broke.
>     'auth *' line in N2N.conf will also be ignored if AES-GCM is used
> and no '--tls-auth' are configured.
> Left HMAC selection menu for Roadwarriors as it was since the WUI do
> provides '--tls-auth' which uses the configuered HMAC even AES-GCM
> has been applied.
> 
> Signed-off-by: Erik Kapfer <erik.kapfer@ipfire.org>
> ---
>  html/cgi-bin/ovpnmain.cgi | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
> index 9f5e682..0a18ec7 100644
> --- a/html/cgi-bin/ovpnmain.cgi
> +++ b/html/cgi-bin/ovpnmain.cgi
> @@ -4543,6 +4543,9 @@ if ($cgiparams{'TYPE'} eq 'net') {
>      }
>      $checked{'PMTU_DISCOVERY'}{$cgiparams{'PMTU_DISCOVERY'}} =
> 'checked=\'checked\'';
>  
> +    $selected{'DCIPHER'}{'AES-256-GCM'} = '';
> +    $selected{'DCIPHER'}{'AES-192-GCM'} = '';
> +    $selected{'DCIPHER'}{'AES-128-GCM'} = '';
>      $selected{'DCIPHER'}{'CAMELLIA-256-CBC'} = '';
>      $selected{'DCIPHER'}{'CAMELLIA-192-CBC'} = '';
>      $selected{'DCIPHER'}{'CAMELLIA-128-CBC'} = '';
> @@ -4706,7 +4709,10 @@ if ($cgiparams{'TYPE'} eq 'net') {
>  	</tr>
>  
>  	<tr><td class='boldbase'>$Lang::tr{'cipher'}</td>
> -		<td><select name='DCIPHER'>
> +		<td><select name='DCIPHER'  id="n2ncipher" required>
> +				<option value='AES-256-GCM'		
> $selected{'DCIPHER'}{'AES-256-GCM'}>AES-GCM (256 $Lang::tr{'bit'})
> with SHA384</option>
> +				<option value='AES-192-GCM'		
> $selected{'DCIPHER'}{'AES-192-GCM'}>AES-GCM (192 $Lang::tr{'bit'})
> with SHA256</option>
> +				<option value='AES-128-GCM'		
> $selected{'DCIPHER'}{'AES-128-GCM'}>AES-GCM (128 $Lang::tr{'bit'})
> with SHA256</option>
>  				<option value='CAMELLIA-256-CBC'	
> $selected{'DCIPHER'}{'CAMELLIA-256-CBC'}>CAMELLIA-CBC (256
> $Lang::tr{'bit'})</option>
>  				<option value='CAMELLIA-192-CBC'	
> $selected{'DCIPHER'}{'CAMELLIA-192-CBC'}>CAMELLIA-CBC (192
> $Lang::tr{'bit'})</option>
>  				<option value='CAMELLIA-128-CBC'	
> $selected{'DCIPHER'}{'CAMELLIA-128-CBC'}>CAMELLIA-CBC (128
> $Lang::tr{'bit'})</option>
> @@ -4723,7 +4729,7 @@ if ($cgiparams{'TYPE'} eq 'net') {
>  		</td>
>  
>  		<td class='boldbase'>$Lang::tr{'ovpn ha'}:</td>
> -		<td><select name='DAUTH'>
> +		<td><select name='DAUTH' id="n2nhmac">
>  				<option value='whirlpool'		
> $selected{'DAUTH'}{'whirlpool'}>Whirlpool (512
> $Lang::tr{'bit'})</option>
>  				<option value='SHA512'		
> 	$selected{'DAUTH'}{'SHA512'}>SHA2 (512
> $Lang::tr{'bit'})</option>
>  				<option value='SHA384'		
> 	$selected{'DAUTH'}{'SHA384'}>SHA2 (384
> $Lang::tr{'bit'})</option>
> @@ -4737,6 +4743,22 @@ if ($cgiparams{'TYPE'} eq 'net') {
>  END
>  ;
>  	}
> +
> +#### JAVA SCRIPT ####
> +# Validate N2N cipher. If GCM is used, disable HMAC menu
> +print<<END;
> +	<script>
> +		var disable_options = false;
> +		document.getElementById('n2ncipher').onchange =
> function () {
> +			if((this.value == "AES-256-GCM"||this.value
> == "AES-192-GCM"||this.value == "AES-128-GCM")) {
> +				document.getElementById('n2nhmac').s
> etAttribute('disabled', true);
> +			} else {
> +				document.getElementById('n2nhmac').r
> emoveAttribute('disabled');
> +			}
> +		}
> +	</script>
> +END
> +
>  #jumper
>  	print "<tr><td class='boldbase'>$Lang::tr{'remark
> title'}</td>";
>  	print "<td colspan='3'><input type='text' name='REMARK'
> value='$cgiparams{'REMARK'}' size='55' maxlength='50'
> /></td></tr></table>";
> @@ -5108,6 +5130,9 @@ END
>      $selected{'DPROTOCOL'}{'tcp'} = '';
>      $selected{'DPROTOCOL'}{$cgiparams{'DPROTOCOL'}} = 'SELECTED';
>  
> +    $selected{'DCIPHER'}{'AES-256-GCM'} = '';
> +    $selected{'DCIPHER'}{'AES-192-GCM'} = '';
> +    $selected{'DCIPHER'}{'AES-128-GCM'} = '';
>      $selected{'DCIPHER'}{'CAMELLIA-256-CBC'} = '';
>      $selected{'DCIPHER'}{'CAMELLIA-192-CBC'} = '';
>      $selected{'DCIPHER'}{'CAMELLIA-128-CBC'} = '';
> @@ -5204,6 +5229,9 @@ END
>  
>  		<td class='boldbase'
> nowrap='nowrap'>$Lang::tr{'cipher'}</td>
>  		<td><select name='DCIPHER'>
> +				<option value='AES-256-GCM'
> $selected{'DCIPHER'}{'AES-256-GCM'}>AES-GCM (256 $Lang::tr{'bit'})
> with SHA384</option>
> +				<option value='AES-192-GCM'
> $selected{'DCIPHER'}{'AES-192-GCM'}>AES-GCM (192 $Lang::tr{'bit'})
> with SHA256</option>
> +				<option value='AES-128-GCM'
> $selected{'DCIPHER'}{'AES-128-GCM'}>AES-GCM (128 $Lang::tr{'bit'})
> with SHA256</option>
>  				<option value='CAMELLIA-256-CBC'
> $selected{'DCIPHER'}{'CAMELLIA-256-CBC'}>CAMELLIA-CBC (256
> $Lang::tr{'bit'})</option>
>  				<option value='CAMELLIA-192-CBC'
> $selected{'DCIPHER'}{'CAMELLIA-192-CBC'}>CAMELLIA-CBC (192
> $Lang::tr{'bit'})</option>
>  				<option value='CAMELLIA-128-CBC'
> $selected{'DCIPHER'}{'CAMELLIA-128-CBC'}>CAMELLIA-CBC (128
> $Lang::tr{'bit'})</option>
Michael Tremer Feb. 15, 2018, 7:20 a.m. UTC | #2 
Hi,

this patch is actually quite big and introduces a new feature by adding AES-GCM. 
It would have been better to get the necessary stuff done first.

On Wed, 2018-02-14 at 15:40 +0100, Erik Kapfer wrote:
> AES-GCM 128, 196 and 256 bit has been added to Net-to-Net and Roadwarrior
> section.
> 
> Cipher menu description has been changed for N2N and RW since AES-GCM uses own
> authentication encryption (GMAC).
>     More information can be found in here https://tools.ietf.org/html/rfc5288
> .
> Added java script snipped to disable HMAC selection for N2N if AES-GCM has
> been selected.
>     'auth *' line in N2N.conf won´t be deleted even if AES-GCM is used so
> possible individual '--tls-auth' configurations won´t broke.
>     'auth *' line in N2N.conf will also be ignored if AES-GCM is used and no
> '--tls-auth' are configured.
> Left HMAC selection menu for Roadwarriors as it was since the WUI do provides
> '--tls-auth' which uses the configuered HMAC even AES-GCM has been applied.
> 
> Signed-off-by: Erik Kapfer <erik.kapfer@ipfire.org>
> ---
>  html/cgi-bin/ovpnmain.cgi | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
> index 9f5e682..0a18ec7 100644
> --- a/html/cgi-bin/ovpnmain.cgi
> +++ b/html/cgi-bin/ovpnmain.cgi
> @@ -4543,6 +4543,9 @@ if ($cgiparams{'TYPE'} eq 'net') {
>      }
>      $checked{'PMTU_DISCOVERY'}{$cgiparams{'PMTU_DISCOVERY'}} =
> 'checked=\'checked\'';
>  
> +    $selected{'DCIPHER'}{'AES-256-GCM'} = '';
> +    $selected{'DCIPHER'}{'AES-192-GCM'} = '';
> +    $selected{'DCIPHER'}{'AES-128-GCM'} = '';
>      $selected{'DCIPHER'}{'CAMELLIA-256-CBC'} = '';
>      $selected{'DCIPHER'}{'CAMELLIA-192-CBC'} = '';
>      $selected{'DCIPHER'}{'CAMELLIA-128-CBC'} = '';
> @@ -4706,7 +4709,10 @@ if ($cgiparams{'TYPE'} eq 'net') {
>  	</tr>
>  
>  	<tr><td class='boldbase'>$Lang::tr{'cipher'}</td>
> -		<td><select name='DCIPHER'>
> +		<td><select name='DCIPHER'  id="n2ncipher" required>
> +				<option value='AES-256-GCM'		$s
> elected{'DCIPHER'}{'AES-256-GCM'}>AES-GCM (256 $Lang::tr{'bit'}) with
> SHA384</option>
> +				<option value='AES-192-GCM'		$s
> elected{'DCIPHER'}{'AES-192-GCM'}>AES-GCM (192 $Lang::tr{'bit'}) with
> SHA256</option>
> +				<option value='AES-128-GCM'		$s
> elected{'DCIPHER'}{'AES-128-GCM'}>AES-GCM (128 $Lang::tr{'bit'}) with
> SHA256</option>

This has nothing to do with SHA* and SHA is not being used at all. The message
authentication is in GCM and only AES is being use as a cipher in counter mode.

So it would only be AES-GCM (X bit).

Also "with" was not translated.

>  				<option value='CAMELLIA-256-CBC'	$sele
> cted{'DCIPHER'}{'CAMELLIA-256-CBC'}>CAMELLIA-CBC (256
> $Lang::tr{'bit'})</option>
>  				<option value='CAMELLIA-192-CBC'	$sele
> cted{'DCIPHER'}{'CAMELLIA-192-CBC'}>CAMELLIA-CBC (192
> $Lang::tr{'bit'})</option>
>  				<option value='CAMELLIA-128-CBC'	$sele
> cted{'DCIPHER'}{'CAMELLIA-128-CBC'}>CAMELLIA-CBC (128
> $Lang::tr{'bit'})</option>
> @@ -4723,7 +4729,7 @@ if ($cgiparams{'TYPE'} eq 'net') {
>  		</td>
>  
>  		<td class='boldbase'>$Lang::tr{'ovpn ha'}:</td>
> -		<td><select name='DAUTH'>
> +		<td><select name='DAUTH' id="n2nhmac">
>  				<option value='whirlpool'		$sel
> ected{'DAUTH'}{'whirlpool'}>Whirlpool (512 $Lang::tr{'bit'})</option>
>  				<option value='SHA512'			
> $selected{'DAUTH'}{'SHA512'}>SHA2 (512 $Lang::tr{'bit'})</option>
>  				<option value='SHA384'			
> $selected{'DAUTH'}{'SHA384'}>SHA2 (384 $Lang::tr{'bit'})</option>
> @@ -4737,6 +4743,22 @@ if ($cgiparams{'TYPE'} eq 'net') {
>  END
>  ;
>  	}
> +
> +#### JAVA SCRIPT ####
> +# Validate N2N cipher. If GCM is used, disable HMAC menu
> +print<<END;
> +	<script>
> +		var disable_options = false;
> +		document.getElementById('n2ncipher').onchange = function () {
> +			if((this.value == "AES-256-GCM"||this.value == "AES-
> 192-GCM"||this.value == "AES-128-GCM")) {
> +				document.getElementById('n2nhmac').setAttribu
> te('disabled', true);
> +			} else {
> +				document.getElementById('n2nhmac').removeAttr
> ibute('disabled');
> +			}
> +		}
> +	</script>
> +END
> +
>  #jumper
>  	print "<tr><td class='boldbase'>$Lang::tr{'remark title'}</td>";
>  	print "<td colspan='3'><input type='text' name='REMARK'
> value='$cgiparams{'REMARK'}' size='55' maxlength='50' /></td></tr></table>";
> @@ -5108,6 +5130,9 @@ END
>      $selected{'DPROTOCOL'}{'tcp'} = '';
>      $selected{'DPROTOCOL'}{$cgiparams{'DPROTOCOL'}} = 'SELECTED';
>  
> +    $selected{'DCIPHER'}{'AES-256-GCM'} = '';
> +    $selected{'DCIPHER'}{'AES-192-GCM'} = '';
> +    $selected{'DCIPHER'}{'AES-128-GCM'} = '';
>      $selected{'DCIPHER'}{'CAMELLIA-256-CBC'} = '';
>      $selected{'DCIPHER'}{'CAMELLIA-192-CBC'} = '';
>      $selected{'DCIPHER'}{'CAMELLIA-128-CBC'} = '';
> @@ -5204,6 +5229,9 @@ END
>  
>  		<td class='boldbase' nowrap='nowrap'>$Lang::tr{'cipher'}</td>
>  		<td><select name='DCIPHER'>
> +				<option value='AES-256-GCM'
> $selected{'DCIPHER'}{'AES-256-GCM'}>AES-GCM (256 $Lang::tr{'bit'}) with
> SHA384</option>
> +				<option value='AES-192-GCM'
> $selected{'DCIPHER'}{'AES-192-GCM'}>AES-GCM (192 $Lang::tr{'bit'}) with
> SHA256</option>
> +				<option value='AES-128-GCM'
> $selected{'DCIPHER'}{'AES-128-GCM'}>AES-GCM (128 $Lang::tr{'bit'}) with
> SHA256</option>

Same as above.

>  				<option value='CAMELLIA-256-CBC'
> $selected{'DCIPHER'}{'CAMELLIA-256-CBC'}>CAMELLIA-CBC (256
> $Lang::tr{'bit'})</option>
>  				<option value='CAMELLIA-192-CBC'
> $selected{'DCIPHER'}{'CAMELLIA-192-CBC'}>CAMELLIA-CBC (192
> $Lang::tr{'bit'})</option>
>  				<option value='CAMELLIA-128-CBC'
> $selected{'DCIPHER'}{'CAMELLIA-128-CBC'}>CAMELLIA-CBC (128
> $Lang::tr{'bit'})</option>

-Michael
Michael Tremer Feb. 15, 2018, 7:23 a.m. UTC | #3 
Hi,

On Wed, 2018-02-14 at 20:11 +0100, ummeegge wrote:
> As a version 3 idea,
> or might it be possibly a better idea to delete the '--auth *' directive in
> N2N.conf
> if AES-GCM has been chosen ? i think it might also be better to integrate
> '--tls-crypt' --> https://www.mail-archive.com/openvpn-
> devel@lists.sourceforge.net/msg12357.html 

I do not get any of those arguments in that email. I find that highly useless
for a legitimate use of VPNs.

> instead of '--tls-auth' to N2N connections which uses a static AES-256-CTR
> whereby
> a HMAC can not be selected ?

The counter mode does not provide authentication like GCM does.

> 
> But also it might be time to delete SHA1 complete from Net-to-Net HMAC
> selection since this won´t harm old connections but brings a little
> more security per default ?

SHA1 is fine when used as a HMAC. Even MD5 is considered secure in that context.

> Sorry for the back and forth but the way is the goal :D .
> 
> Some feedback might be nevertheless nice and important.
> 
> Greetings,
> 
> Erik
> 
> 
> Am Mittwoch, den 14.02.2018, 15:40 +0100 schrieb Erik Kapfer:
> > AES-GCM 128, 196 and 256 bit has been added to Net-to-Net and
> > Roadwarrior section.
> > 
> > Cipher menu description has been changed for N2N and RW since AES-GCM 
> > uses own authentication encryption (GMAC).
> >     More information can be found in here https://tools.ietf.org/html
> > /rfc5288 .
> > Added java script snipped to disable HMAC selection for N2N if AES-
> > GCM has been selected.
> >     'auth *' line in N2N.conf won´t be deleted even if AES-GCM is
> > used so possible individual '--tls-auth' configurations won´t broke.
> >     'auth *' line in N2N.conf will also be ignored if AES-GCM is used
> > and no '--tls-auth' are configured.
> > Left HMAC selection menu for Roadwarriors as it was since the WUI do
> > provides '--tls-auth' which uses the configuered HMAC even AES-GCM
> > has been applied.
> > 
> > Signed-off-by: Erik Kapfer <erik.kapfer@ipfire.org>
> > ---
> >  html/cgi-bin/ovpnmain.cgi | 32 ++++++++++++++++++++++++++++++--
> >  1 file changed, 30 insertions(+), 2 deletions(-)
> > 
> > diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
> > index 9f5e682..0a18ec7 100644
> > --- a/html/cgi-bin/ovpnmain.cgi
> > +++ b/html/cgi-bin/ovpnmain.cgi
> > @@ -4543,6 +4543,9 @@ if ($cgiparams{'TYPE'} eq 'net') {
> >      }
> >      $checked{'PMTU_DISCOVERY'}{$cgiparams{'PMTU_DISCOVERY'}} =
> > 'checked=\'checked\'';
> >  
> > +    $selected{'DCIPHER'}{'AES-256-GCM'} = '';
> > +    $selected{'DCIPHER'}{'AES-192-GCM'} = '';
> > +    $selected{'DCIPHER'}{'AES-128-GCM'} = '';
> >      $selected{'DCIPHER'}{'CAMELLIA-256-CBC'} = '';
> >      $selected{'DCIPHER'}{'CAMELLIA-192-CBC'} = '';
> >      $selected{'DCIPHER'}{'CAMELLIA-128-CBC'} = '';
> > @@ -4706,7 +4709,10 @@ if ($cgiparams{'TYPE'} eq 'net') {
> >  	</tr>
> >  
> >  	<tr><td class='boldbase'>$Lang::tr{'cipher'}</td>
> > -		<td><select name='DCIPHER'>
> > +		<td><select name='DCIPHER'  id="n2ncipher" required>
> > +				<option value='AES-256-GCM'		
> > $selected{'DCIPHER'}{'AES-256-GCM'}>AES-GCM (256 $Lang::tr{'bit'})
> > with SHA384</option>
> > +				<option value='AES-192-GCM'		
> > $selected{'DCIPHER'}{'AES-192-GCM'}>AES-GCM (192 $Lang::tr{'bit'})
> > with SHA256</option>
> > +				<option value='AES-128-GCM'		
> > $selected{'DCIPHER'}{'AES-128-GCM'}>AES-GCM (128 $Lang::tr{'bit'})
> > with SHA256</option>
> >  				<option value='CAMELLIA-256-CBC'	
> > $selected{'DCIPHER'}{'CAMELLIA-256-CBC'}>CAMELLIA-CBC (256
> > $Lang::tr{'bit'})</option>
> >  				<option value='CAMELLIA-192-CBC'	
> > $selected{'DCIPHER'}{'CAMELLIA-192-CBC'}>CAMELLIA-CBC (192
> > $Lang::tr{'bit'})</option>
> >  				<option value='CAMELLIA-128-CBC'	
> > $selected{'DCIPHER'}{'CAMELLIA-128-CBC'}>CAMELLIA-CBC (128
> > $Lang::tr{'bit'})</option>
> > @@ -4723,7 +4729,7 @@ if ($cgiparams{'TYPE'} eq 'net') {
> >  		</td>
> >  
> >  		<td class='boldbase'>$Lang::tr{'ovpn ha'}:</td>
> > -		<td><select name='DAUTH'>
> > +		<td><select name='DAUTH' id="n2nhmac">
> >  				<option value='whirlpool'		
> > $selected{'DAUTH'}{'whirlpool'}>Whirlpool (512
> > $Lang::tr{'bit'})</option>
> >  				<option value='SHA512'		
> > 	$selected{'DAUTH'}{'SHA512'}>SHA2 (512
> > $Lang::tr{'bit'})</option>
> >  				<option value='SHA384'		
> > 	$selected{'DAUTH'}{'SHA384'}>SHA2 (384
> > $Lang::tr{'bit'})</option>
> > @@ -4737,6 +4743,22 @@ if ($cgiparams{'TYPE'} eq 'net') {
> >  END
> >  ;
> >  	}
> > +
> > +#### JAVA SCRIPT ####
> > +# Validate N2N cipher. If GCM is used, disable HMAC menu
> > +print<<END;
> > +	<script>
> > +		var disable_options = false;
> > +		document.getElementById('n2ncipher').onchange =
> > function () {
> > +			if((this.value == "AES-256-GCM"||this.value
> > == "AES-192-GCM"||this.value == "AES-128-GCM")) {
> > +				document.getElementById('n2nhmac').s
> > etAttribute('disabled', true);
> > +			} else {
> > +				document.getElementById('n2nhmac').r
> > emoveAttribute('disabled');
> > +			}
> > +		}
> > +	</script>
> > +END
> > +
> >  #jumper
> >  	print "<tr><td class='boldbase'>$Lang::tr{'remark
> > title'}</td>";
> >  	print "<td colspan='3'><input type='text' name='REMARK'
> > value='$cgiparams{'REMARK'}' size='55' maxlength='50'
> > /></td></tr></table>";
> > @@ -5108,6 +5130,9 @@ END
> >      $selected{'DPROTOCOL'}{'tcp'} = '';
> >      $selected{'DPROTOCOL'}{$cgiparams{'DPROTOCOL'}} = 'SELECTED';
> >  
> > +    $selected{'DCIPHER'}{'AES-256-GCM'} = '';
> > +    $selected{'DCIPHER'}{'AES-192-GCM'} = '';
> > +    $selected{'DCIPHER'}{'AES-128-GCM'} = '';
> >      $selected{'DCIPHER'}{'CAMELLIA-256-CBC'} = '';
> >      $selected{'DCIPHER'}{'CAMELLIA-192-CBC'} = '';
> >      $selected{'DCIPHER'}{'CAMELLIA-128-CBC'} = '';
> > @@ -5204,6 +5229,9 @@ END
> >  
> >  		<td class='boldbase'
> > nowrap='nowrap'>$Lang::tr{'cipher'}</td>
> >  		<td><select name='DCIPHER'>
> > +				<option value='AES-256-GCM'
> > $selected{'DCIPHER'}{'AES-256-GCM'}>AES-GCM (256 $Lang::tr{'bit'})
> > with SHA384</option>
> > +				<option value='AES-192-GCM'
> > $selected{'DCIPHER'}{'AES-192-GCM'}>AES-GCM (192 $Lang::tr{'bit'})
> > with SHA256</option>
> > +				<option value='AES-128-GCM'
> > $selected{'DCIPHER'}{'AES-128-GCM'}>AES-GCM (128 $Lang::tr{'bit'})
> > with SHA256</option>
> >  				<option value='CAMELLIA-256-CBC'
> > $selected{'DCIPHER'}{'CAMELLIA-256-CBC'}>CAMELLIA-CBC (256
> > $Lang::tr{'bit'})</option>
> >  				<option value='CAMELLIA-192-CBC'
> > $selected{'DCIPHER'}{'CAMELLIA-192-CBC'}>CAMELLIA-CBC (192
> > $Lang::tr{'bit'})</option>
> >  				<option value='CAMELLIA-128-CBC'
> > $selected{'DCIPHER'}{'CAMELLIA-128-CBC'}>CAMELLIA-CBC (128
> > $Lang::tr{'bit'})</option>
ummeegge Feb. 15, 2018, 4:02 p.m. UTC | #4 
Hello,

Am Mittwoch, den 14.02.2018, 20:20 +0000 schrieb Michael Tremer:
> Hi,
> 
> this patch is actually quite big and introduces a new feature by
> adding AES-GCM. 
> It would have been better to get the necessary stuff done first.

Should i split the java stuff (if it is in general usefull) in a
separate patch ? Can also split N2N from the Roadwarrior patch but trhe
changes are pretty equal and straight forward ?

We can leave this patch behind, most important for me was feedback with
this. Have send the most necessary stuff already.

Greetings,

Erik
ummeegge Feb. 15, 2018, 5:09 p.m. UTC | #5 
Hi,
and thanks for your feedback.

Am Mittwoch, den 14.02.2018, 20:23 +0000 schrieb Michael Tremer:
> Hi,
> 
> On Wed, 2018-02-14 at 20:11 +0100, ummeegge wrote:
> > 
> > As a version 3 idea,
> > or might it be possibly a better idea to delete the '--auth *'
> > directive in
> > N2N.conf
> > if AES-GCM has been chosen ? i think it might also be better to
> > integrate
> > '--tls-crypt' --> https://www.mail-archive.com/openvpn-
> > devel@lists.sourceforge.net/msg12357.html 
> I do not get any of those arguments in that email. I find that highly
> useless
> for a legitimate use of VPNs.
> 
Not sure what you exactly mean with 'useless' ?

Just to clarify, --auth HMAC is also used by --tls-auth which serves a
separate layer of authentication protection for the control channel (to
mitigate DoS attacks and attacks on the TLS stack).

--tls-crypt is a new feature in v2.4 which not only authenticates (like
--tls-auth do), but also encrypts the TLS control channel (more
privacy) but uses AES-256-CTR instead of the --auth HMAC (also called
"poor-man's" post-quantum security).

Both options are currently not available for N2N but may in the future.
So i thought it might be better to delete the '--auth HMAC' directive
in N2N.conf if GCM has been selected.

> > 
> > instead of '--tls-auth' to N2N connections which uses a static AES-
> > 256-CTR
> > whereby
> > a HMAC can not be selected ?
> The counter mode does not provide authentication like GCM does.
> 

Sure CTR is different to GCM but according to OpenVPN-2.4 manpage 
--> https://community.openvpn.net/openvpn/wiki/Openvpn24ManPage ( under '--tls-crypt keyfile' ) 
it encrypts but also authenticates.
Logs from testings with --tls-crypt, AES-GCM for N2N looked like this:

Apr  7 16:59:58 ipfire UE2n2n[1530]: disabling NCP mode (--ncp-disable) because not in P2MP client or server mode
Apr  7 16:59:58 ipfire UE2n2n[1530]: OpenVPN 2.4.1 i586-pc-linux-gnu [SSL (OpenSSL)] [LZO] [LZ4] [EPOLL] [MH/PKTINFO] [AEAD] built on Apr  5 2017

...

Apr  7 16:59:58 ipfire UE2n2n[1531]: Outgoing Control Channel Encryption: Cipher 'AES-256-CTR' initialized with 256 bit key
Apr  7 16:59:58 ipfire UE2n2n[1531]: Outgoing Control Channel Encryption: Using 256 bit message hash 'SHA256' for HMAC authentication
Apr  7 16:59:58 ipfire UE2n2n[1531]: Incoming Control Channel Encryption: Cipher 'AES-256-CTR' initialized with 256 bit key
Apr  7 16:59:58 ipfire UE2n2n[1531]: Incoming Control Channel Encryption: Using 256 bit message hash 'SHA256' for HMAC authentication

...

Apr  7 17:00:04 ipfire UE2n2n[1531]: Data Channel Encrypt: Cipher 'AES-256-GCM' initialized with 256 bit key
Apr  7 17:00:04 ipfire UE2n2n[1531]: Data Channel Decrypt: Cipher 'AES-256-GCM' initialized with 256 bit key
Apr  7 17:00:04 ipfire UE2n2n[1531]: Control Channel: TLSv1.2, cipher TLSv1/SSLv3 ECDHE-RSA-AES256-GCM-SHA384, 8192 bit RSA
Apr  7 17:00:04 ipfire UE2n2n[1531]: [xxx.xxx-gateway.de] Peer Connection Initiated with [AF_INET]91.192.xxx.xxx:61000
Apr  7 17:00:05 ipfire UE2n2n[1531]: Initialization Sequence Completed


So i would a kind of prepare this a little for a potential future
(deleting --auth from N2N.conf if GCM is used) but if there is a
decision in the future to use --tls-auth, the HMAC selection makes
sense even we use GCM. But since --tls-crypt uses only AES-256-CTR the
HMAC selection is useless if GCM has been chosen.

Sorry for the longer term thinking and possible confusions.

Greetings,

Erik
Michael Tremer Feb. 15, 2018, 9:42 p.m. UTC | #6 
Hi,

On Thu, 2018-02-15 at 06:02 +0100, ummeegge wrote:
> Hello,
> 
> Am Mittwoch, den 14.02.2018, 20:20 +0000 schrieb Michael Tremer:
> > Hi,
> > 
> > this patch is actually quite big and introduces a new feature by
> > adding AES-GCM. 
> > It would have been better to get the necessary stuff done first.
> 
> Should i split the java stuff (if it is in general usefull) in a
> separate patch ? Can also split N2N from the Roadwarrior patch but trhe
> changes are pretty equal and straight forward ?

No, leave this in there. That just creates some extra work. But consider that JS
is a not a strict requirement in the webUI. And we do have jQuery if you want to
use that.

> We can leave this patch behind, most important for me was feedback with
> this. Have send the most necessary stuff already.
> 
> Greetings,
> 
> Erik
Michael Tremer Feb. 15, 2018, 9:59 p.m. UTC | #7 
Hi,

On Thu, 2018-02-15 at 07:09 +0100, ummeegge wrote:
> Hi,
> and thanks for your feedback.
> 
> Am Mittwoch, den 14.02.2018, 20:23 +0000 schrieb Michael Tremer:
> > Hi,
> > 
> > On Wed, 2018-02-14 at 20:11 +0100, ummeegge wrote:
> > > 
> > > As a version 3 idea,
> > > or might it be possibly a better idea to delete the '--auth *'
> > > directive in
> > > N2N.conf
> > > if AES-GCM has been chosen ? i think it might also be better to
> > > integrate
> > > '--tls-crypt' --> https://www.mail-archive.com/openvpn-
> > > devel@lists.sourceforge.net/msg12357.html 
> > 
> > I do not get any of those arguments in that email. I find that highly
> > useless
> > for a legitimate use of VPNs.
> > 
> 
> Not sure what you exactly mean with 'useless' ?

I thought some of that is a bit esoteric cryptography.

Hiding the TLS connection makes sense when you are in China behind the big
state-run firewall, but that is about it.

I mean I am not against it, but this is pretty useless and probably only creates
many confusing configuration options for the average user.

> Just to clarify, --auth HMAC is also used by --tls-auth which serves a
> separate layer of authentication protection for the control channel (to
> mitigate DoS attacks and attacks on the TLS stack).
>
> --tls-crypt is a new feature in v2.4 which not only authenticates (like
> --tls-auth do), but also encrypts the TLS control channel (more
> privacy) but uses AES-256-CTR instead of the --auth HMAC (also called
> "poor-man's" post-quantum security).

I am never a fan of non-standard cryptography. Has this been properly peer-
reviewed?

> Both options are currently not available for N2N but may in the future.
> So i thought it might be better to delete the '--auth HMAC' directive
> in N2N.conf if GCM has been selected.

GCM already has the authentication built in.

> 
> > > 
> > > instead of '--tls-auth' to N2N connections which uses a static AES-
> > > 256-CTR
> > > whereby
> > > a HMAC can not be selected ?
> > 
> > The counter mode does not provide authentication like GCM does.
> > 
> 
> Sure CTR is different to GCM but according to OpenVPN-2.4 manpage 
> --> https://community.openvpn.net/openvpn/wiki/Openvpn24ManPage ( under '
> --tls-crypt keyfile' ) 
> it encrypts but also authenticates.

So this is basically using a static key and then running the TLS connection
through it? Usually there will be a DH key exchange and a classic TLS
connection.

And who wants to use CTR mode when you can have GCM? This can only be to speed
things up a bit because messages are now being encrypted twice.

> Logs from testings with --tls-crypt, AES-GCM for N2N looked like this:
> 
> Apr  7 16:59:58 ipfire UE2n2n[1530]: disabling NCP mode (--ncp-disable)
> because not in P2MP client or server mode
> Apr  7 16:59:58 ipfire UE2n2n[1530]: OpenVPN 2.4.1 i586-pc-linux-gnu [SSL
> (OpenSSL)] [LZO] [LZ4] [EPOLL] [MH/PKTINFO] [AEAD] built on Apr  5 2017
> 
> ...
> 
> Apr  7 16:59:58 ipfire UE2n2n[1531]: Outgoing Control Channel Encryption:
> Cipher 'AES-256-CTR' initialized with 256 bit key
> Apr  7 16:59:58 ipfire UE2n2n[1531]: Outgoing Control Channel Encryption:
> Using 256 bit message hash 'SHA256' for HMAC authentication
> Apr  7 16:59:58 ipfire UE2n2n[1531]: Incoming Control Channel Encryption:
> Cipher 'AES-256-CTR' initialized with 256 bit key
> Apr  7 16:59:58 ipfire UE2n2n[1531]: Incoming Control Channel Encryption:
> Using 256 bit message hash 'SHA256' for HMAC authentication
> 
> ...
> 
> Apr  7 17:00:04 ipfire UE2n2n[1531]: Data Channel Encrypt: Cipher 'AES-256-
> GCM' initialized with 256 bit key
> Apr  7 17:00:04 ipfire UE2n2n[1531]: Data Channel Decrypt: Cipher 'AES-256-
> GCM' initialized with 256 bit key
> Apr  7 17:00:04 ipfire UE2n2n[1531]: Control Channel: TLSv1.2, cipher
> TLSv1/SSLv3 ECDHE-RSA-AES256-GCM-SHA384, 8192 bit RSA
> Apr  7 17:00:04 ipfire UE2n2n[1531]: [xxx.xxx-gateway.de] Peer Connection
> Initiated with [AF_INET]91.192.xxx.xxx:61000
> Apr  7 17:00:05 ipfire UE2n2n[1531]: Initialization Sequence Completed
> 
> 
> So i would a kind of prepare this a little for a potential future
> (deleting --auth from N2N.conf if GCM is used) but if there is a
> decision in the future to use --tls-auth, the HMAC selection makes
> sense even we use GCM. But since --tls-crypt uses only AES-256-CTR the
> HMAC selection is useless if GCM has been chosen.

Let's focus on things that are useful for the average user first. I think --tls-
auth does not add anything extra when using GCM, but it doesn't harm anyone
either.

The --tls-crypt is something that should never be enabled by default. But if you
want to have it, add it.

> 
> Sorry for the longer term thinking and possible confusions.
> 
> Greetings,
> 
> Erik
> 
>
ummeegge Feb. 16, 2018, 12:30 a.m. UTC | #8 
Hello,
first of all. May it is better to wait with the introduction of AES-GCM 
until OpenSSL-1.1.0g + OpenVPN-2.4.4 has been released, or what do you
think ?

> > > 
> > > On Wed, 2018-02-14 at 20:11 +0100, ummeegge wrote:
> > > > 
> > > > 
> > > > As a version 3 idea,
> > > > or might it be possibly a better idea to delete the '--auth *'
> > > > directive in
> > > > N2N.conf
> > > > if AES-GCM has been chosen ? i think it might also be better to
> > > > integrate
> > > > '--tls-crypt' --> https://www.mail-archive.com/openvpn-
> > > > devel@lists.sourceforge.net/msg12357.html 
> > > I do not get any of those arguments in that email. I find that
> > > highly
> > > useless
> > > for a legitimate use of VPNs.
> > > 
> > Not sure what you exactly mean with 'useless' ?
> I thought some of that is a bit esoteric cryptography.

:D i see, you are also right this is a kind of esoteric in the true
sense of the word (designed for or understood by the specially
initiated alone ;) .
> 
> Hiding the TLS connection makes sense when you are in China behind
> the big
> state-run firewall, but that is about it.

Not only, to some extend the Heartbleed vulnerability for example was not exploitable
with an active --tls-auth (--tls-crypt serves the same mechanism)
--> https://community.openvpn.net/openvpn/wiki/heartbleed but OpenVPN do also
strongly encourage to use such protections 
--> https://community.openvpn.net/openvpn/wiki/Hardening#Useof--tls-auth .

> 
> I mean I am not against it, but this is pretty useless and probably
> only creates
> many confusing configuration options for the average user.

Have integrated it some months ago in my environment (works here
without problems) and it can be activated via one checkbox 
https://people.ipfire.org/~ummeegge/screenshoots/OpenVPN-2.4_beta2/N2N_tls-crypt.png
same like --tls-auth which IPFire serves for Roadwarriors since 2 or 3
years meanwhile.

> 
> > 
> > Just to clarify, --auth HMAC is also used by --tls-auth which
> > serves a
> > separate layer of authentication protection for the control channel
> > (to
> > mitigate DoS attacks and attacks on the TLS stack).
> > 
> > --tls-crypt is a new feature in v2.4 which not only authenticates
> > (like
> > --tls-auth do), but also encrypts the TLS control channel (more
> > privacy) but uses AES-256-CTR instead of the --auth HMAC (also
> > called
> > "poor-man's" post-quantum security).
> I am never a fan of non-standard cryptography. Has this been properly
> peer-
> reviewed?

I think it has also been reviewed while the v2.4 security evaluation
from Quarkslabs and PrivateInternetAccess 
https://blog.quarkslab.com/resources/2017-05-11-security-assessment-of-openvpn/17-03-284-REP-openvpn-sec-assessment.pdf
take a look into the 'Recommendations' section under '2. Executive Summary' .
But it is also meanwhile widely used on other distros e.g. https://redmine.pfsense.org/issues/7071 
but also by some VPN providers i think.

> 
> > 
> > Both options are currently not available for N2N but may in the
> > future.
> > So i thought it might be better to delete the '--auth HMAC'
> > directive
> > in N2N.conf if GCM has been selected.
> GCM already has the authentication built in.

This are two different layers of security in my opinion whereby both
directives do offers a 2nd line of defense if a future flaw is
discovered in a particular TLS cipher-suite or implementation, whereby
--tls-crypt encrypts also the control channel.
A little deeper explanation can also be found in the hardening wiki or
in here 
http://archive.openvpn.net/pipermail/openvpn-devel/2016-July/024892.html
for a little more info causing --tls-crypt .

> 
> The --tls-crypt is something that should never be enabled by default.
> But if you
> want to have it, add it.

Think so and i haven´t it enabled by default, integrated it in the same
way as --tls-auth is already integrated, ticking a checkbox and ready.

But as mentioned this is a future sound of music and i would wait with
this since there are more important things i think (--ncp-cipher, AES-
GCM integration, deprecated directives such as comp-lzo, ...).


Most important for me was to come to a decision for the AES-GCM patch
if i should delete the 'auth' directive (needed only for --tls-auth
since it use the same HMAC then the old ciphers) if a GCM cipher has
been chosen and i think i will do this to keep the house clean so to
say ;-).


Greetings,

Erik
ummeegge Feb. 16, 2018, 12:35 a.m. UTC | #9 
Hello,

#Am Donnerstag, den 15.02.2018, 10:42 +0000 schrieb Michael Tremer:
> Hi,
> 
> On Thu, 2018-02-15 at 06:02 +0100, ummeegge wrote:
> > 
> > Hello,
> > 
> > Am Mittwoch, den 14.02.2018, 20:20 +0000 schrieb Michael Tremer:
> > > 
> > > Hi,
> > > 
> > > this patch is actually quite big and introduces a new feature by
> > > adding AES-GCM. 
> > > It would have been better to get the necessary stuff done first.
> > Should i split the java stuff (if it is in general usefull) in a
> > separate patch ? Can also split N2N from the Roadwarrior patch but
> > trhe
> > changes are pretty equal and straight forward ?
> No, leave this in there. That just creates some extra work.
OK.

>  But consider that JS
> is a not a strict requirement in the webUI. And we do have jQuery if
> you want to
> use that.
Good to know will give it a try but i need to take a deeper look into
the jQuery thing then.

Have also already added your other suggestions for the cipher list
description.


Thanks for the feedback.

Greetings,

Erik
diff mbox series

Patch

diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
index 9f5e682..0a18ec7 100644
--- a/html/cgi-bin/ovpnmain.cgi
+++ b/html/cgi-bin/ovpnmain.cgi
@@ -4543,6 +4543,9 @@  if ($cgiparams{'TYPE'} eq 'net') {
     }
     $checked{'PMTU_DISCOVERY'}{$cgiparams{'PMTU_DISCOVERY'}} = 'checked=\'checked\'';
 
+    $selected{'DCIPHER'}{'AES-256-GCM'} = '';
+    $selected{'DCIPHER'}{'AES-192-GCM'} = '';
+    $selected{'DCIPHER'}{'AES-128-GCM'} = '';
     $selected{'DCIPHER'}{'CAMELLIA-256-CBC'} = '';
     $selected{'DCIPHER'}{'CAMELLIA-192-CBC'} = '';
     $selected{'DCIPHER'}{'CAMELLIA-128-CBC'} = '';
@@ -4706,7 +4709,10 @@  if ($cgiparams{'TYPE'} eq 'net') {
 	</tr>
 
 	<tr><td class='boldbase'>$Lang::tr{'cipher'}</td>
-		<td><select name='DCIPHER'>
+		<td><select name='DCIPHER'  id="n2ncipher" required>
+				<option value='AES-256-GCM'		$selected{'DCIPHER'}{'AES-256-GCM'}>AES-GCM (256 $Lang::tr{'bit'}) with SHA384</option>
+				<option value='AES-192-GCM'		$selected{'DCIPHER'}{'AES-192-GCM'}>AES-GCM (192 $Lang::tr{'bit'}) with SHA256</option>
+				<option value='AES-128-GCM'		$selected{'DCIPHER'}{'AES-128-GCM'}>AES-GCM (128 $Lang::tr{'bit'}) with SHA256</option>
 				<option value='CAMELLIA-256-CBC'	$selected{'DCIPHER'}{'CAMELLIA-256-CBC'}>CAMELLIA-CBC (256 $Lang::tr{'bit'})</option>
 				<option value='CAMELLIA-192-CBC'	$selected{'DCIPHER'}{'CAMELLIA-192-CBC'}>CAMELLIA-CBC (192 $Lang::tr{'bit'})</option>
 				<option value='CAMELLIA-128-CBC'	$selected{'DCIPHER'}{'CAMELLIA-128-CBC'}>CAMELLIA-CBC (128 $Lang::tr{'bit'})</option>
@@ -4723,7 +4729,7 @@  if ($cgiparams{'TYPE'} eq 'net') {
 		</td>
 
 		<td class='boldbase'>$Lang::tr{'ovpn ha'}:</td>
-		<td><select name='DAUTH'>
+		<td><select name='DAUTH' id="n2nhmac">
 				<option value='whirlpool'		$selected{'DAUTH'}{'whirlpool'}>Whirlpool (512 $Lang::tr{'bit'})</option>
 				<option value='SHA512'			$selected{'DAUTH'}{'SHA512'}>SHA2 (512 $Lang::tr{'bit'})</option>
 				<option value='SHA384'			$selected{'DAUTH'}{'SHA384'}>SHA2 (384 $Lang::tr{'bit'})</option>
@@ -4737,6 +4743,22 @@  if ($cgiparams{'TYPE'} eq 'net') {
 END
 ;
 	}
+
+#### JAVA SCRIPT ####
+# Validate N2N cipher. If GCM is used, disable HMAC menu
+print<<END;
+	<script>
+		var disable_options = false;
+		document.getElementById('n2ncipher').onchange = function () {
+			if((this.value == "AES-256-GCM"||this.value == "AES-192-GCM"||this.value == "AES-128-GCM")) {
+				document.getElementById('n2nhmac').setAttribute('disabled', true);
+			} else {
+				document.getElementById('n2nhmac').removeAttribute('disabled');
+			}
+		}
+	</script>
+END
+
 #jumper
 	print "<tr><td class='boldbase'>$Lang::tr{'remark title'}</td>";
 	print "<td colspan='3'><input type='text' name='REMARK' value='$cgiparams{'REMARK'}' size='55' maxlength='50' /></td></tr></table>";
@@ -5108,6 +5130,9 @@  END
     $selected{'DPROTOCOL'}{'tcp'} = '';
     $selected{'DPROTOCOL'}{$cgiparams{'DPROTOCOL'}} = 'SELECTED';
 
+    $selected{'DCIPHER'}{'AES-256-GCM'} = '';
+    $selected{'DCIPHER'}{'AES-192-GCM'} = '';
+    $selected{'DCIPHER'}{'AES-128-GCM'} = '';
     $selected{'DCIPHER'}{'CAMELLIA-256-CBC'} = '';
     $selected{'DCIPHER'}{'CAMELLIA-192-CBC'} = '';
     $selected{'DCIPHER'}{'CAMELLIA-128-CBC'} = '';
@@ -5204,6 +5229,9 @@  END
 
 		<td class='boldbase' nowrap='nowrap'>$Lang::tr{'cipher'}</td>
 		<td><select name='DCIPHER'>
+				<option value='AES-256-GCM' $selected{'DCIPHER'}{'AES-256-GCM'}>AES-GCM (256 $Lang::tr{'bit'}) with SHA384</option>
+				<option value='AES-192-GCM' $selected{'DCIPHER'}{'AES-192-GCM'}>AES-GCM (192 $Lang::tr{'bit'}) with SHA256</option>
+				<option value='AES-128-GCM' $selected{'DCIPHER'}{'AES-128-GCM'}>AES-GCM (128 $Lang::tr{'bit'}) with SHA256</option>
 				<option value='CAMELLIA-256-CBC' $selected{'DCIPHER'}{'CAMELLIA-256-CBC'}>CAMELLIA-CBC (256 $Lang::tr{'bit'})</option>
 				<option value='CAMELLIA-192-CBC' $selected{'DCIPHER'}{'CAMELLIA-192-CBC'}>CAMELLIA-CBC (192 $Lang::tr{'bit'})</option>
 				<option value='CAMELLIA-128-CBC' $selected{'DCIPHER'}{'CAMELLIA-128-CBC'}>CAMELLIA-CBC (128 $Lang::tr{'bit'})</option>