Re: [PATCH] Mark recommended ciphers/algorithms

[Michael Tremer <michael.tremer@ipfire.org>]

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

On Sun, 2015-12-13 at 16:10 +0100, IT Superhack wrote:
> Hello Michael,
> 
> Michael Tremer:
> > Hello,
> > 
> > this patch was line-wrapped and cannot be merged, but nevertheless,
> > here are my thoughts:
> I am unable to submit patches at the moment, since git send-email
> keeps
> crashing on every machine I own - sometimes starttls issues,
> sometimes
> segfault - and TB seems to line-wrap.

Weird that that happens. If you are using git on IPFire and
experiencing these issues, please open a bug report.

> > 
> > On Mon, 2015-12-07 at 17:35 +0100, IT Superhack wrote:
> > > Signed-off-by: Timmothy Wilson <itsuperhack(a)web.de>
> > > ---
> > > diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi
> > > -bin/ovpnmain.cgi
> > > index 62af54e..15385f1 100644
> > > --- a/html/cgi-bin/ovpnmain.cgi
> > > +++ b/html/cgi-bin/ovpnmain.cgi
> > > @@ -1316,7 +1316,7 @@ END
> > >  				<option value='1024'
> > > $selected{'DHLENGHT'}{'1024'}>1024
> > > $Lang::tr{'bit'}</option>
> > >  				<option value='2048'
> > > $selected{'DHLENGHT'}{'2048'}>2048
> > > $Lang::tr{'bit'}</option>
> > >  				<option value='3072'
> > > $selected{'DHLENGHT'}{'3072'}>3072
> > > $Lang::tr{'bit'}</option>
> > > -				<option value='4096'
> > > $selected{'DHLENGHT'}{'4096'}>4096
> > > $Lang::tr{'bit'}</option>
> > > +				<option value='4096'
> > > $selected{'DHLENGHT'}{'4096'}>4096
> > > $Lang::tr{'bit'} ($Lang::tr{'recommended'})</option>
> > >  			</select>
> > >  		</td>
> > >  	</tr>
> > 
> > I agree, that it is desirable to use longer keys. However, I am not
> > sure if it is a good idea to go all the way for 4096 bit and not
> > only
> > for e.g. 2048 bit. Why not 8192 even?
> > 
> Since the SSLTest server page treats 2048 DH primes as "weak", I
> guess
> 3072 or better is suitable here.
> > I would like to read some justification for the values that are
> > picked.
> Here is one, for example:
> https://netzpolitik.org/2015/kryptographie-open-source-und-gesellscha
> ft/
> (german, please see "8."). In this article is also mentioned that 512
> bit hash algorithms should be used.
> > 
> > Furthermore, I think that we the upper bound should be something
> > that
> > the average IPFire box is able to handle.
> I agree with that. Maybe 3072 bits is a good deal between speed and
> security, what do you think?

That depends entirely on the hardware. We cannot know what people are
using. That makes it rather complicated to decide.

> > 
> > > @@ -4687,7 +4687,7 @@ if ($cgiparams{'TYPE'} eq 'net') {
> > >  				<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>
> > > -				<option value='AES-256-CBC' 	
> > > $selected{'DCIPHER'}{'AES-256-CBC'}>AES-CBC (256
> > > $Lang::tr{'bit'},
> > > $Lang::tr{'default'})</option>
> > > +				<option value='AES-256-CBC' 	
> > > $selected{'DCIPHER'}{'AES-256-CBC'}>AES-CBC (256
> > > $Lang::tr{'bit'},
> > > $Lang::tr{'default'}, $Lang::tr{'recommended'})</option>
> > >  				<option value='AES-192-CBC' 	
> > > $selected{'DCIPHER'}{'AES-192-CBC'}>AES-CBC (192
> > > $Lang::tr{'bit'})</option>
> > >  				<option value='AES-128-CBC' 	
> > > $selected{'DCIPHER'}{'AES-128-CBC'}>AES-CBC (128
> > > $Lang::tr{'bit'})</option>
> > >  				<option value='DES-EDE3-CBC'	
> > > $selected{'DCIPHER'}{'DES-EDE3-CBC'}>DES-EDE3-CBC (192
> > > $Lang::tr{'bit'})</option>
> > 
> > I can agree with that since it is already selected by default. This
> > makes it just more explicit.
> > 
> > I would have merged this if this was an independent patch in a
> > patch
> > set.
> Thanks, but at the moment, i cannot hand in a patch without wrapped
> lines.

For now you could push the git branch somewhere and I can pull that.
Sending comments is difficult though, hence we do this on this list
with patches.

> > 
> > > @@ -4702,7 +4702,7 @@ if ($cgiparams{'TYPE'} eq 'net') {
> > >  		<td class='boldbase'>$Lang::tr{'ovpn ha'}:</td>
> > >  		<td><select name='DAUTH'>
> > >  				<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='SHA512'		
> > > 	$selected{'DAUTH'}{'SHA512'}>SHA2 (512
> > > $Lang::tr{'bit'}, $Lang::tr{'recommended'})</option>
> > >  				<option value='SHA384'		
> > > 	$selected{'DAUTH'}{'SHA384'}>SHA2 (384
> > > $Lang::tr{'bit'})</option>
> > >  				<option value='SHA256'		
> > > 	$selected{'DAUTH'}{'SHA256'}>SHA2 (256
> > > $Lang::tr{'bit'})</option>
> > >  				<option value='SHA1'		
> > > 	
> > > $selected{'DAUTH'}{'SHA1'}>SHA1 (160
> > > $Lang::tr{'bit'} Default)</option>
> > 
> > Same as above. SHA2 is considered to be "secure enough for now".
> > Why is
> > 256 bit not enough? This has also a rather huge performance impact.
> > Things like these should also be mentioned in the commit message.
> > 
> > > diff --git a/html/cgi-bin/vpnmain.cgi b/html/cgi-bin/vpnmain.cgi
> > > index f1cffb8..9aa50f5 100644
> > > --- a/html/cgi-bin/vpnmain.cgi
> > > +++ b/html/cgi-bin/vpnmain.cgi
> > > @@ -2424,7 +2424,7 @@ if(($cgiparams{'ACTION'} eq
> > > $Lang::tr{'advanced'}) ||
> > >  			<td>$Lang::tr{'vpn keyexchange'}:</td>
> > >  			<td>
> > >  				<select name='IKE_VERSION'>
> > > -					<option value='ikev2'
> > > $selected{'IKE_VERSION'}{'ikev2'}>IKEv2</option>
> > > +					<option value='ikev2'
> > > $selected{'IKE_VERSION'}{'ikev2'}>IKEv2
> > > ($Lang::tr{'recommended'})</option>
> > >  					<option value='ikev1'
> > > $selected{'IKE_VERSION'}{'ikev1'}>IKEv1</option>
> > >  				</select>
> > >  			</td>
> > 
> > Why should IKEv2 be recommended? AFAIK there are no known design
> > issues
> > with IKEv1. Some algorithms might not be available, but this is not
> > an
> > issue for now since AES, SHA2, (AKA the strong ones) are supported.
> > 
> > > @@ -2434,7 +2434,7 @@ if(($cgiparams{'ACTION'} eq
> > > $Lang::tr{'advanced'}) ||
> > >  			<td class='boldbase'
> > > width="15%">$Lang::tr{'encryption'}</td>
> > >  			<td class='boldbase'>
> > >  				<select name='IKE_ENCRYPTION'
> > > multiple='multiple' size='6'
> > > style='width: 100%'>
> > > -					<option
> > > value='aes256gcm128'
> > > $checked{'IKE_ENCRYPTION'}{'aes256gcm128'}>256 bit AES-GCM/128
> > > bit
> > > ICV</option>
> > > +					<option
> > > value='aes256gcm128'
> > > $checked{'IKE_ENCRYPTION'}{'aes256gcm128'}>256 bit AES-GCM/128
> > > bit
> > > ICV
> > > ($Lang::tr{'recommended'})</option>
> > >  					<option
> > > value='aes256gcm96'
> > > $checked{'IKE_ENCRYPTION'}{'aes256gcm96'}>256 bit AES-GCM/96 bit
> > > ICV</option>
> > >  					<option
> > > value='aes256gcm64'
> > > $checked{'IKE_ENCRYPTION'}{'aes256gcm64'}>256 bit AES-GCM/64 bit
> > > ICV</option>
> > >  					<option value='aes256'
> > > $checked{'IKE_ENCRYPTION'}{'aes256'}>256
> > > bit AES-CBC</option>
> > > @@ -2454,7 +2454,7 @@ if(($cgiparams{'ACTION'} eq
> > > $Lang::tr{'advanced'}) ||
> > >  			</td>
> > >  			<td class='boldbase'>
> > >  				<select name='ESP_ENCRYPTION'
> > > multiple='multiple' size='6'
> > > style='width: 100%'>
> > > -					<option
> > > value='aes256gcm128'
> > > $checked{'ESP_ENCRYPTION'}{'aes256gcm128'}>256 bit AES-GCM/128
> > > bit
> > > ICV</option>
> > > +					<option
> > > value='aes256gcm128'
> > > $checked{'ESP_ENCRYPTION'}{'aes256gcm128'}>256 bit AES-GCM/128
> > > bit
> > > ICV
> > > ($Lang::tr{'recommended'})</option>
> > >  					<option
> > > value='aes256gcm96'
> > > $checked{'ESP_ENCRYPTION'}{'aes256gcm96'}>256 bit AES-GCM/96 bit
> > > ICV</option>
> > >  					<option
> > > value='aes256gcm64'
> > > $checked{'ESP_ENCRYPTION'}{'aes256gcm64'}>256 bit AES-GCM/64 bit
> > > ICV</option>
> > >  					<option value='aes256'
> > > $checked{'ESP_ENCRYPTION'}{'aes256'}>256
> > > bit AES-CBC</option>
> > 
> > Why are the AES-GCM cipher suites with smaller IVs not recommended?
> > 
> > > @@ -2478,7 +2478,7 @@ if(($cgiparams{'ACTION'} eq
> > > $Lang::tr{'advanced'}) ||
> > >  			<td class='boldbase'
> > > width="15%">$Lang::tr{'integrity'}</td>
> > >  			<td class='boldbase'>
> > >  				<select name='IKE_INTEGRITY'
> > > multiple='multiple' size='6'
> > > style='width: 100%'>
> > > -					<option value='sha2_512'
> > > $checked{'IKE_INTEGRITY'}{'sha2_512'}>SHA2 512 bit</option>
> > > +					<option value='sha2_512'
> > > $checked{'IKE_INTEGRITY'}{'sha2_512'}>SHA2 512 bit
> > > ($Lang::tr{'recommended'})</option>
> > >  					<option value='sha2_384'
> > > $checked{'IKE_INTEGRITY'}{'sha2_384'}>SHA2 384 bit</option>
> > >  					<option value='sha2_256'
> > > $checked{'IKE_INTEGRITY'}{'sha2_256'}>SHA2 256 bit</option>
> > >  					<option value='aesxcbc'
> > > $checked{'IKE_INTEGRITY'}{'aesxcbc'}>AES
> > > XCBC</option>
> > 
> > Same as above.
> > 
> > > @@ -2488,7 +2488,7 @@ if(($cgiparams{'ACTION'} eq
> > > $Lang::tr{'advanced'}) ||
> > >  			</td>
> > >  			<td class='boldbase'>
> > >  				<select name='ESP_INTEGRITY'
> > > multiple='multiple' size='6'
> > > style='width: 100%'>
> > > -					<option value='sha2_512'
> > > $checked{'ESP_INTEGRITY'}{'sha2_512'}>SHA2 512 bit</option>
> > > +					<option value='sha2_512'
> > > $checked{'ESP_INTEGRITY'}{'sha2_512'}>SHA2 512 bit
> > > ($Lang::tr{'recommended'})</option>
> > >  					<option value='sha2_384'
> > > $checked{'ESP_INTEGRITY'}{'sha2_384'}>SHA2 384 bit</option>
> > >  					<option value='sha2_256'
> > > $checked{'ESP_INTEGRITY'}{'sha2_256'}>SHA2 256 bit</option>
> > >  					<option value='aesxcbc'
> > > $checked{'ESP_INTEGRITY'}{'aesxcbc'}>AES
> > > XCBC</option>
> > 
> > Same again.
> There seems to be a problem with the word "recommended". In the
> patches
> submitted, I recommended always the most strongest cipher. However,
> as
> you said, some of them are simply one step too much. Should then both
> be
> recommended?

I am not sure. Can anyone come up with a more fitting expression? If we
mark everything as "recommended" that is strong enough for now after
our consideration, we will have most of them tagged with that word. In
that case it would make more sense to mark the weak stuff as such to
keep readability. Maybe that is the way to go. But does the average Joe
know what is meant by "weak"?

> In my opinion, this has to be clarified, but since it is a very
> subjective thing, it might be difficult.

It is not really just subjective. We can say for sure that some ciphers
and hashes are broken. We can also say that some are weak and will be
considered broken soon. That is pretty much objective since we have
sources for that. If only enough people agree that X is broken or weak,
that is pretty much a fact.

The recommendation though is more complicated because I would like to
take into account how feasible it is to use a certain cipher/hash/etc.
RSA with 8192 bits long keys is quite nice. We can assume that it is
more secure than RSA with a 4096 bits long key if it was generated from
true random numbers. However handshakes will take longer. Generating
the key will take weeks on some systems. I want this to be reflected by
this change. Please feel free to disagree with me on this :)

Can we get more people to send their thoughts on this?

> > 
> > > diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl
> > > index 2bca854..b18cace 100644
> > > --- a/langs/de/cgi-bin/de.pl
> > > +++ b/langs/de/cgi-bin/de.pl
> > > @@ -1914,6 +1914,7 @@
> > >  'rebooting ipfire' => 'Starte IPFire neu',
> > >  'reconnect' => 'Neu Verbinden',
> > >  'reconnection' => 'Wiederverbindung',
> > > +'recommended' => 'empfohlen',
> > >  'red' => 'Internet',
> > >  'red1' => 'ROT',
> > >  'references' => 'Referenzen',
> > > 
> > > 
> > 
> > The English translation is missing.
> Oh, sorry, I forgot.
> > 
> > Best,
> > -Michael
> > 
> Best regards,
> Timmothy Wilson
> 

-Michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]