Re: packets to bridged interfaces bypass input filter

2020-05-26 Thread Stephan Mending
On Tue, May 26, 2020 at 09:26:07PM +0200, Sven M. Hallberg wrote:
> hi all,
> 
> i sent the following question to misc@ on march 29th but received no
> response. i hope you don't mind me retrying on tech@.
> 
> while playing around with pf, i noticed that some connections that i
> thought should be blocked, were in fact not. here is my fairly standard
> bridge setup between a wlan interface and a wired ethernet.
> 
># cat /etc/hostname.em0
>group lan
>up
> 
># cat /etc/hostname.athn0
>mediaopt hostap
>nwid xxx wpakey abcd1234
>group lan
>up
> 
># cat /etc/hostname.vether0
>group lan
>inet 192.168.22.145/28
> 
># cat /etc/hostname.bridge0
>add em0 add athn0 add vether0
>blocknonip athn0
>up
> 
> i then set up pf rules that should let the (w)lan interfaces talk to
> the world.
> 
> # cat /etc/pf.conf
> set skip on lo
> block return
> pass out# ingress filter
> pass in on lan from lan:network to any
> 
> now this all works as intended. but, when i remove athn0 from the lan
> group, i would expect the above ruleset to block any incoming
> connections from the wlan.
> 
> # ifconfig athn0 -group lan
Did you flush states between your tests (pfctl -F states) ? -> After you 
removed athn0 from lan group ? 
> 
> and indeed, i could not ping a wired host from the wlan. however, i
> could still access the router (192.168.22.145) itself from the wireless.
> as far as i can tell, this is due to the following code in if_bridge.c,
> bridge_process() -- comments by me:
> 
> if (bridge_ourether(bif0->ifp, eh->ether_dhost)) {
> /* addressed to the iface it came in on */
> bif = bif0;
> } else {
> SMR_SLIST_FOREACH_LOCKED(bif, &sc->sc_iflist, bif_next) {
> if (bif->ifp == ifp)
> continue;
> if (bridge_ourether(bif->ifp, eh->ether_dhost))
> /* addressed to some other bridge member */
> break;
> }
> }
> if (bif != NULL) {
> /* ... */
> if (bridge_filterrule(&bif0->bif_brlin, eh, m) ==
> BRL_ACTION_BLOCK) {
> goto bad;
> }
> /* ... */
> }
> 
> a packet that comes in on a member of a bridge and is addressed to _any_
> bridge member interface, is processed as input to (only) the destination
> interface. if that destination interface differs from the one that the
> packet actually came in on, pf rules for the actual input interface are
> not evaluated.
> 
> this seems to contradict the following statement in the NOTES section of
> bridge(4) -- in any case, it certainly contradicted my intuition:
> 
>  Bridged packets pass through pf(4) filters once as input on the receiving
>  interface and once as output on all interfaces on which they are
>  forwarded.  In order to pass through the bridge packets must pass any in
>  rules on the input and any out rules on the output interface.  Packets
>  may be blocked either entering or leaving the bridge.
> 
> now, to be clear, i don't mind that the packet passes through 'in' rules for
> vether0 in my example, but i wonder if it should not also pass through
> the 'in' rules for 'athn0'.
> 
> i would appreciate enlightenment on this. note that (contrary to misc) i
> am subscribed to tech@, so no cc needed.
> 
> 
> thanks,
> pesco
> 
> PS: at the time, i was using OpenBSD 6.6, but the code in question
> appears unchanged in current.
> 



Re: iked(8): AES_GCM ciphers for IKE

2020-05-16 Thread Stephan Mending
On Fri, May 15, 2020 at 01:59:35AM +0200, Tobias Heider wrote:
> On Thu, May 14, 2020 at 10:47:52PM +0200, Tobias Heider wrote:
> > On Thu, May 14, 2020 at 10:07:30PM +0200, Tobias Heider wrote:
> > > Hi,
> > > 
> > > currently iked(8) supports AES-GCM only for ESP.
> > > The diff below adds the ENCR_AES_GCM_16 and ENCR_AES_GCM_12 variants for 
> > > IKE.
> > > (for more information see [1] and [2]).
> > > Both variants support the 128, 196, and 256 bit key lengths.
> > > 
> > > The new new ciphers can be configured with:
> > > - aes-128-gcm, aes-196-gcm and aes-256-gcm for ENCR_AES_GCM_16
> > > - aes-128-gcm-12, aes-196-gcm-12 and aes-256-gcm-12 for ENCR_AES_GCM_12
> 
> Small typo: it's 192, not 196.
> 
> > > 
> > > It would be nice if we could get some interop testing with different IKEv2
> > > implementations.  I have so far successfully tested strongswan <-> iked 
> > > and
> > > of course iked <-> iked.
> > > 
> > > Feedback welcome ;)
> > > 
> > > [1] https://tools.ietf.org/html/rfc5282
> > > [2] 
> > > https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xhtml#ikev2-parameters-5
> > > 
> > 
> > whoops, previous diff was broken.
> > 
> 
> Another update because it seems parse_xf matches substrings instead of the
> full transform type name, which means I had to change the order of ikeencxfs
> members or 'aes-128-gcm' will always match 'aes-128-gcm-12' ...
> 
> Index: crypto.c
> ===
> RCS file: /cvs/src/sbin/iked/crypto.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 crypto.c
> --- crypto.c  14 May 2020 15:08:30 -  1.27
> +++ crypto.c  14 May 2020 23:55:13 -
> @@ -92,7 +92,7 @@ hash_new(uint8_t type, uint16_t id)
>   struct iked_hash*hash;
>   const EVP_MD*md = NULL;
>   HMAC_CTX*ctx = NULL;
> - int  length = 0, fixedkey = 0, trunc = 0;
> + int  length = 0, fixedkey = 0, trunc = 0, isaead = 
> 0;
>  
>   switch (type) {
>   case IKEV2_XFORMTYPE_PRF:
> @@ -156,6 +156,14 @@ hash_new(uint8_t type, uint16_t id)
>   length = SHA512_DIGEST_LENGTH;
>   trunc = 32;
>   break;
> + case IKEV2_XFORMAUTH_AES_GCM_12:
> + length = 12;
> + isaead = 1;
> + break;
> + case IKEV2_XFORMAUTH_AES_GCM_16:
> + length = 16;
> + isaead = 1;
> + break;
>   case IKEV2_XFORMAUTH_NONE:
>   case IKEV2_XFORMAUTH_DES_MAC:
>   case IKEV2_XFORMAUTH_KPDK_MD5:
> @@ -177,7 +185,7 @@ hash_new(uint8_t type, uint16_t id)
>   print_map(id, ikev2_xformtype_map));
>   break;
>   }
> - if (md == NULL)
> + if (!isaead && md == NULL)
>   return (NULL);
>  
>   if ((hash = calloc(1, sizeof(*hash))) == NULL) {
> @@ -192,6 +200,10 @@ hash_new(uint8_t type, uint16_t id)
>   hash->hash_trunc = trunc;
>   hash->hash_length = length;
>   hash->hash_fixedkey = fixedkey;
> + hash->hash_isaead = isaead;
> +
> + if (isaead)
> + return (hash);
>  
>   if ((ctx = calloc(1, sizeof(*ctx))) == NULL) {
>   log_debug("%s: alloc hash ctx", __func__);
> @@ -276,6 +288,7 @@ cipher_new(uint8_t type, uint16_t id, ui
>   const EVP_CIPHER*cipher = NULL;
>   EVP_CIPHER_CTX  *ctx = NULL;
>   int  length = 0, fixedkey = 0, ivlength = 0;
> + int  saltlength = 0, authid = 0;
>  
>   switch (type) {
>   case IKEV2_XFORMTYPE_ENCR:
> @@ -309,6 +322,39 @@ cipher_new(uint8_t type, uint16_t id, ui
>   ivlength = EVP_CIPHER_iv_length(cipher);
>   fixedkey = EVP_CIPHER_key_length(cipher);
>   break;
> + case IKEV2_XFORMENCR_AES_GCM_16:
> + case IKEV2_XFORMENCR_AES_GCM_12:
> + switch (id_length) {
> + case 128:
> + cipher = EVP_aes_128_gcm();
> + break;
> + case 192:
> + cipher = EVP_aes_192_gcm();
> + break;
> + case 256:
> + cipher = EVP_aes_256_gcm();
> + break;
> + default:
> + log_debug("%s: invalid key length %d"
> + " for cipher %s", __func__, id_length,
> + print_map(id, ikev2_xformencr_map));
> + break;
> + }
> + if (cipher == NULL)
> + break;
> + switch(id) {
> + case IKEV2_XFORMENCR_AES_GCM_16:
> 

Re: iked(8): AES_GCM ciphers for IKE

2020-05-14 Thread Stephan Mending
On Thu, May 14, 2020 at 10:47:52PM +0200, Tobias Heider wrote:
> On Thu, May 14, 2020 at 10:07:30PM +0200, Tobias Heider wrote:
> > Hi,
> > 
> > currently iked(8) supports AES-GCM only for ESP.
> > The diff below adds the ENCR_AES_GCM_16 and ENCR_AES_GCM_12 variants for 
> > IKE.
> > (for more information see [1] and [2]).
> > Both variants support the 128, 196, and 256 bit key lengths.
> > 
> > The new new ciphers can be configured with:
> > - aes-128-gcm, aes-196-gcm and aes-256-gcm for ENCR_AES_GCM_16
> > - aes-128-gcm-12, aes-196-gcm-12 and aes-256-gcm-12 for ENCR_AES_GCM_12
> > 
> > It would be nice if we could get some interop testing with different IKEv2
> > implementations.  I have so far successfully tested strongswan <-> iked and
> > of course iked <-> iked.
> > 
> > Feedback welcome ;)
> > 
> > [1] https://tools.ietf.org/html/rfc5282
> > [2] 
> > https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xhtml#ikev2-parameters-5
> > 
> 
> whoops, previous diff was broken.
> 
> Index: crypto.c
> ===
> RCS file: /cvs/src/sbin/iked/crypto.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 crypto.c
> --- crypto.c  14 May 2020 15:08:30 -  1.27
> +++ crypto.c  14 May 2020 20:43:27 -
> @@ -92,7 +92,7 @@ hash_new(uint8_t type, uint16_t id)
>   struct iked_hash*hash;
>   const EVP_MD*md = NULL;
>   HMAC_CTX*ctx = NULL;
> - int  length = 0, fixedkey = 0, trunc = 0;
> + int  length = 0, fixedkey = 0, trunc = 0, isaead = 
> 0;
>  
>   switch (type) {
>   case IKEV2_XFORMTYPE_PRF:
> @@ -156,6 +156,14 @@ hash_new(uint8_t type, uint16_t id)
>   length = SHA512_DIGEST_LENGTH;
>   trunc = 32;
>   break;
> + case IKEV2_XFORMAUTH_AES_GCM_12:
> + length = 12;
> + isaead = 1;
> + break;
> + case IKEV2_XFORMAUTH_AES_GCM_16:
> + length = 16;
> + isaead = 1;
> + break;
>   case IKEV2_XFORMAUTH_NONE:
>   case IKEV2_XFORMAUTH_DES_MAC:
>   case IKEV2_XFORMAUTH_KPDK_MD5:
> @@ -177,7 +185,7 @@ hash_new(uint8_t type, uint16_t id)
>   print_map(id, ikev2_xformtype_map));
>   break;
>   }
> - if (md == NULL)
> + if (!isaead && md == NULL)
>   return (NULL);
>  
>   if ((hash = calloc(1, sizeof(*hash))) == NULL) {
> @@ -192,6 +200,10 @@ hash_new(uint8_t type, uint16_t id)
>   hash->hash_trunc = trunc;
>   hash->hash_length = length;
>   hash->hash_fixedkey = fixedkey;
> + hash->hash_isaead = isaead;
> +
> + if (isaead)
> + return (hash);
>  
>   if ((ctx = calloc(1, sizeof(*ctx))) == NULL) {
>   log_debug("%s: alloc hash ctx", __func__);
> @@ -276,6 +288,7 @@ cipher_new(uint8_t type, uint16_t id, ui
>   const EVP_CIPHER*cipher = NULL;
>   EVP_CIPHER_CTX  *ctx = NULL;
>   int  length = 0, fixedkey = 0, ivlength = 0;
> + int  saltlength = 0, authid = 0;
>  
>   switch (type) {
>   case IKEV2_XFORMTYPE_ENCR:
> @@ -309,6 +322,39 @@ cipher_new(uint8_t type, uint16_t id, ui
>   ivlength = EVP_CIPHER_iv_length(cipher);
>   fixedkey = EVP_CIPHER_key_length(cipher);
>   break;
> + case IKEV2_XFORMENCR_AES_GCM_16:
> + case IKEV2_XFORMENCR_AES_GCM_12:
> + switch (id_length) {
> + case 128:
> + cipher = EVP_aes_128_gcm();
> + break;
> + case 192:
> + cipher = EVP_aes_192_gcm();
> + break;
> + case 256:
> + cipher = EVP_aes_256_gcm();
> + break;
> + default:
> + log_debug("%s: invalid key length %d"
> + " for cipher %s", __func__, id_length,
> + print_map(id, ikev2_xformencr_map));
> + break;
> + }
> + if (cipher == NULL)
> + break;
> + switch(id) {
> + case IKEV2_XFORMENCR_AES_GCM_16:
> + authid = IKEV2_XFORMAUTH_AES_GCM_16;
> + break;
> + case IKEV2_XFORMENCR_AES_GCM_12:
> + authid = IKEV2_XFORMAUTH_AES_GCM_12;
> + break;
> + }
> + length = EVP_CIPHER_block_size(cipher);
> + ivleng

Dead peer detection in iked

2020-05-07 Thread Stephan Mending
Hi *, 
I was wondering why there is no dead peer detection implemented for iked ?
Is it just due to lack of time ? Or are there good reasons to dismiss directly 
implemented dpd in iked ? 

Because technically one has the option to just use ifstated. 

I'm just being curios here. 

Thanks for your time ! 

Best regards, 
Stephan 



Re: iked(8): Removing SHA1 from default transforms

2020-05-03 Thread Stephan Mending
I know Theo, Tobias told me a few mails back. I was joking... 

On Sat, May 02, 2020 at 07:32:43AM -0600, Theo de Raadt wrote:
> Stephan Mending  wrote:
> 
> > On 02/05/2020 02:58, Theo de Raadt wrote:
> > 
> > > Stephan Mending  wrote:
> > >
> > >> I don't get how this could be ?
> > > then go study.
> > >
> > I think I've struck a nerve right here. I'm sorry to have caused you
> > high blood pressure by sending this diff. I do not doubt the
> > competency of you or the other developers. And yes I did not know that
> > SHA-1 is still fine to be using as MAC. That's why my name is md5
> > collisions and not sha1 collisions, right?
> 
> MD5 HMACs are fine also.  HMACs are fine.
> 
> It's obvious you see yourself as a crusader.  I'm thinking more monty
> python, though.
> 
> 
> 



Re: iked(8): Removing SHA1 from default transforms

2020-05-02 Thread Stephan Mending

On 02/05/2020 02:58, Theo de Raadt wrote:


Stephan Mending  wrote:


I don't get how this could be ?

then go study.

I think I've struck a nerve right here. I'm sorry to have caused you 
high blood pressure by sending this diff. I do not doubt the competency 
of you or the other developers. And yes I did not know that SHA-1 is 
still fine to be using as MAC. That's why my name is md5 collisions and 
not sha1 collisions, right?




Re: iked(8): Removing SHA1 from default transforms

2020-05-02 Thread Stephan Mending

On 02/05/2020 01:45, Tobias Heider wrote:

Hi Stephan,


"Also: the mentioned sha1 transform is also a HMAC construction and technically
safe to use (for now), same as the PRF."

I don't get how this could be ? SHA-1 for signage and hash generation is
colliding. How can it be that this doesn't apply for iked ?

It is true that there have been several successful collission attacks against
SHA-1 in the last years and it is NOT safe to use plain sha-1 as a integrity
verification hash nowadays, but there are exceptions.

So what can an attacker do with these attacks?
Given a sha-1 hash 'h' of some arbitrary bytestring 'a', an attacker utilizing
the attack could in the worst case craft a second bytestring 'b' which would
generate the same sha-1 hash value 'h'.
For this to work, the attacker must be able to control the input to the hash
function, in this case 'b'.

Why is this not a problem for iked and esp?
ESP and IKEv2 don't use a plain sha-1 hash for the integrity verification but
a so called keyed-hash message authentication code (or HMAC) [1].
Given a message m, and our hash algorithm H we don't simply hash the message
to get H(m) as the integrity check value (ICV).

Instead we compute ICV = H(K XOR opad, H(K XOR ipad, m)), where K is a secret
key shared between the two peers (resulting from the previous Diffie-Hellman
exchange) and ipad and opad are constant values of:

ipad = the byte 0x36 repeated $SHA1-BLOCKSIZE times
opad = the byte 0x5C repeated $SHA1-BLOCKSIZE times.

This makes things a lot more complicated for the attacker because it requires
knowledge of the key K.  This kind of HMAC construction is generally assumed
to be resistant against chosen-prefix attacks as we have seen the for sha-1,
for a more detailed analysis see for example [2].
Indeed even today there is no known attack against HMAC-MD5 in this same
construction.

That being said, this doesn't mean it is a good idea to use HMAC-MD5 in new
applications and it is also a good idea to get rid of sha-1 hmacs wherever
we can, which is why your diff is actually perfectly valid.
But for today, there is no known attack and it is technically still considered
safe to use.  This means we actually have the time to make the migration as
painless as possible for our users which would require some proper testing and
possible a trial period.

- Tobias

[1] https://tools.ietf.org/html/rfc2104
[2] https://cseweb.ucsd.edu/~mihir/papers/hmac-new.html



Thank you so much for your detailed response.  I absolutely appreciate it.





On 02/05/2020 00:03, Tobias Heider wrote:

On Fri, May 01, 2020 at 11:35:23PM +0200, Stephan Mending wrote:

Hi *,

this diff removes SHA1 as default transform for integrity algorithms.

It's been broken long enough. Let's at least get rid of it in iked's
defaults.

SHA1 is officially broken since 2011 and there have been doubts about it
since 2005.

Though using SHA1 in combination with HAMC as pseudorandom function is
perfectly fine as of today.


OK?

Thank you for the diff.

It's not as if we haven't thought about removing this transform, but before
doing so we need to do some testing to ensure we don't break existing setups.
See this mail from yesterday:
https://marc.info/?l=openbsd-tech&m=158828278120230&w=2

Also: the mentioned sha1 transform is also a HMAC construction and technically
safe to use (for now), same as the PRF.


Index: parse.y
===
RCS file: /cvs/src/sbin/iked/parse.y,v
retrieving revision 1.99
diff -u -p -r1.99 parse.y
--- parse.y 30 Apr 2020 21:11:13 -  1.99
+++ parse.y 1 May 2020 21:19:41 -
@@ -144,7 +144,6 @@ struct iked_transform ikev2_default_ike_
      { IKEV2_XFORMTYPE_PRF,  IKEV2_XFORMPRF_HMAC_SHA2_256 },
      { IKEV2_XFORMTYPE_PRF,  IKEV2_XFORMPRF_HMAC_SHA1 },
      { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_256_128 },
-   { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA1_96 },
      { IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_CURVE25519 },
      { IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_ECP_521 },
      { IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_ECP_384 },
@@ -164,7 +163,6 @@ struct iked_transform ikev2_default_esp_
      { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 192 },
      { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 128 },
      { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_256_128 },
-   { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA1_96 },
      { IKEV2_XFORMTYPE_ESN,  IKEV2_XFORMESN_ESN },
      { IKEV2_XFORMTYPE_ESN,  IKEV2_XFORMESN_NONE },
      { 0 }

ndex: parse.y
===
RCS file: /cvs/src/sbin/iked/parse.y,v
retrieving revision 1.99
diff -u -p -r1.99 parse.y
--- parse.y 30 Apr 2020 21:11:13 -  1.99
+++ parse.y

Re: iked(8): Removing SHA1 from default transforms

2020-05-01 Thread Stephan Mending

On 02/05/2020 00:40, Stuart Henderson wrote:

On 2020/05/02 00:23, Stephan Mending wrote:

Hi,

I actually read your thread. By what I understood you're at the moment
trying to change a few defaults.

That was the reason I wanted to add SHA1 for removal. I just thought it
deserved a seperate thread.

I do understand that you're trying to be careful with removing or changing
defaults. From my point of view everybody that is (maybe implicitly) using
SHA1 right now is better off to be get this wakeup call the earlier the
better.

We aren't even removing SHA1 we're just not offering it as default. And for
those Windows boxes who require it, those people just have to add a line to
explicitly enable it. I would not see such big of a problem.

The things removed recently have a very low risk of affecting anyone.
sha1 (and modp1024) are high risk.

Removing from the default list may cause some people to be unable
to connect to their network after updating. This may mean that they
are then unable to connect back in to fix it.

If this change is made it needs to be done fairly early in the release
cycle, and preferably at a time when slightly fewer people are relying
on working remote access to get at their networks.



I dont't have much experience with such a big projekt like OpenBSD. How 
do you normally carry through with such significant changes ? Just the 
release notes and hoping somebody in snaps will complain ? Or is there 
more to it, which I didn't notice ?




Re: iked(8): Removing SHA1 from default transforms

2020-05-01 Thread Stephan Mending

Hi,

I actually read your thread. By what I understood you're at the moment 
trying to change a few defaults.


That was the reason I wanted to add SHA1 for removal. I just thought it 
deserved a seperate thread.


I do understand that you're trying to be careful with removing or 
changing defaults. From my point of view everybody that is (maybe 
implicitly) using SHA1 right now is better off to be get this wakeup 
call the earlier the better.


We aren't even removing SHA1 we're just not offering it as default. And 
for those Windows boxes who require it, those people just have to add a 
line to explicitly enable it. I would not see such big of a problem.



"Also: the mentioned sha1 transform is also a HMAC construction and technically
safe to use (for now), same as the PRF."

I don't get how this could be ? SHA-1 for signage and hash generation is 
colliding. How can it be that this doesn't apply for iked ?



g,

Stephan


On 02/05/2020 00:03, Tobias Heider wrote:

On Fri, May 01, 2020 at 11:35:23PM +0200, Stephan Mending wrote:

Hi *,

this diff removes SHA1 as default transform for integrity algorithms.

It's been broken long enough. Let's at least get rid of it in iked's
defaults.

SHA1 is officially broken since 2011 and there have been doubts about it
since 2005.

Though using SHA1 in combination with HAMC as pseudorandom function is
perfectly fine as of today.


OK?

Thank you for the diff.

It's not as if we haven't thought about removing this transform, but before
doing so we need to do some testing to ensure we don't break existing setups.
See this mail from yesterday:
https://marc.info/?l=openbsd-tech&m=158828278120230&w=2

Also: the mentioned sha1 transform is also a HMAC construction and technically
safe to use (for now), same as the PRF.



Index: parse.y
===
RCS file: /cvs/src/sbin/iked/parse.y,v
retrieving revision 1.99
diff -u -p -r1.99 parse.y
--- parse.y 30 Apr 2020 21:11:13 -  1.99
+++ parse.y 1 May 2020 21:19:41 -
@@ -144,7 +144,6 @@ struct iked_transform ikev2_default_ike_
     { IKEV2_XFORMTYPE_PRF,  IKEV2_XFORMPRF_HMAC_SHA2_256 },
     { IKEV2_XFORMTYPE_PRF,  IKEV2_XFORMPRF_HMAC_SHA1 },
     { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_256_128 },
-   { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA1_96 },
     { IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_CURVE25519 },
     { IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_ECP_521 },
     { IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_ECP_384 },
@@ -164,7 +163,6 @@ struct iked_transform ikev2_default_esp_
     { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 192 },
     { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 128 },
     { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_256_128 },
-   { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA1_96 },
     { IKEV2_XFORMTYPE_ESN,  IKEV2_XFORMESN_ESN },
     { IKEV2_XFORMTYPE_ESN,  IKEV2_XFORMESN_NONE },
     { 0 }

ndex: parse.y
===
RCS file: /cvs/src/sbin/iked/parse.y,v
retrieving revision 1.99
diff -u -p -r1.99 parse.y
--- parse.y 30 Apr 2020 21:11:13 -  1.99
+++ parse.y 1 May 2020 21:19:41 -
@@ -144,7 +144,6 @@ struct iked_transform ikev2_default_ike_
 { IKEV2_XFORMTYPE_PRF,  IKEV2_XFORMPRF_HMAC_SHA2_256 },
 { IKEV2_XFORMTYPE_PRF,  IKEV2_XFORMPRF_HMAC_SHA1 },
 { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_256_128 },
-   { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA1_96 },
 { IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_CURVE25519 },
 { IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_ECP_521 },
 { IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_ECP_384 },
@@ -164,7 +163,6 @@ struct iked_transform ikev2_default_esp_
 { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 192 },
 { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 128 },
 { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_256_128 },
-   { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA1_96 },
 { IKEV2_XFORMTYPE_ESN,  IKEV2_XFORMESN_ESN },
 { IKEV2_XFORMTYPE_ESN,  IKEV2_XFORMESN_NONE },
 { 0 }




iked(8): Removing SHA1 from default transforms

2020-05-01 Thread Stephan Mending

Hi *,

this diff removes SHA1 as default transform for integrity algorithms.

It's been broken long enough. Let's at least get rid of it in iked's 
defaults.


SHA1 is officially broken since 2011 and there have been doubts about it 
since 2005.


Though using SHA1 in combination with HAMC as pseudorandom function is 
perfectly fine as of today.



OK?


Index: parse.y
===
RCS file: /cvs/src/sbin/iked/parse.y,v
retrieving revision 1.99
diff -u -p -r1.99 parse.y
--- parse.y 30 Apr 2020 21:11:13 -  1.99
+++ parse.y 1 May 2020 21:19:41 -
@@ -144,7 +144,6 @@ struct iked_transform ikev2_default_ike_
    { IKEV2_XFORMTYPE_PRF,  IKEV2_XFORMPRF_HMAC_SHA2_256 },
    { IKEV2_XFORMTYPE_PRF,  IKEV2_XFORMPRF_HMAC_SHA1 },
    { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_256_128 },
-   { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA1_96 },
    { IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_CURVE25519 },
    { IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_ECP_521 },
    { IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_ECP_384 },
@@ -164,7 +163,6 @@ struct iked_transform ikev2_default_esp_
    { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 192 },
    { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 128 },
    { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_256_128 },
-   { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA1_96 },
    { IKEV2_XFORMTYPE_ESN,  IKEV2_XFORMESN_ESN },
    { IKEV2_XFORMTYPE_ESN,  IKEV2_XFORMESN_NONE },
    { 0 }

ndex: parse.y
===
RCS file: /cvs/src/sbin/iked/parse.y,v
retrieving revision 1.99
diff -u -p -r1.99 parse.y
--- parse.y 30 Apr 2020 21:11:13 -  1.99
+++ parse.y 1 May 2020 21:19:41 -
@@ -144,7 +144,6 @@ struct iked_transform ikev2_default_ike_
{ IKEV2_XFORMTYPE_PRF,  IKEV2_XFORMPRF_HMAC_SHA2_256 },
{ IKEV2_XFORMTYPE_PRF,  IKEV2_XFORMPRF_HMAC_SHA1 },
{ IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_256_128 },
-   { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA1_96 },
{ IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_CURVE25519 },
{ IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_ECP_521 },
{ IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_ECP_384 },
@@ -164,7 +163,6 @@ struct iked_transform ikev2_default_esp_
{ IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 192 },
{ IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 128 },
{ IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_256_128 },
-   { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA1_96 },
{ IKEV2_XFORMTYPE_ESN,  IKEV2_XFORMESN_ESN },
{ IKEV2_XFORMTYPE_ESN,  IKEV2_XFORMESN_NONE },
{ 0 }


Re: AEAD Suites in IKEX (iked) and Phase 1 (isakmpd)

2020-04-20 Thread Stephan Mending
Hi Tobias, *,

thanks for the heads up.
I'd see a security benefit by using those chiphers. (-> compared with CBC 
Ciphers)

Thanks for your time.

g,
Stephan


On Mon, Apr 20, 2020 at 01:36:47PM +0200, Tobias Heider wrote:
> Date: Mon, 20 Apr 2020 13:36:47 +0200
> From: Tobias Heider 
> Subject: Re: AEAD Suites in IKEX (iked) and Phase 1 (isakmpd)
> To: Stephan Mending 
> Cc: tech@openbsd.org
> 
> On Mon, Apr 20, 2020 at 12:52:24PM +0200, Stephan Mending wrote:
> > Hi,
> > I was wondering if there was a reason why there are no AEAD Suites 
> > implemented for initial IKEX in iked or phase 1 in isamkmpd ? Even though 
> > iked's childSAs
> > support it and Phase 2 in isakmpd does as well ? Is it just lack of time ? 
> > Because for example strongswan does exactly support that. Using GCM Suites 
> > and/or
> > Chacha20Poly1305. 
> > 
> > Thanks for your time. 
> > 
> > Best regards, 
> > Stephan
> > 
> 
> The quick answer would be: "No one bothered to implement it."
> 
> It is on my list of planned features for iked, but honestly having
> AEADs in the IKE SA is not really a priority.
> 
> The main reason to prefer AEADs is their better performance.
> The amount of data exchanged in phase 1 (or the IKE SA) is negligible
> because it is only used for encrypting the key exchange messages.
> The Child SAs (which are used for ESP) are where performance and
> throughput actually matter.
> 
> - Tobias
> 



AEAD Suites in IKEX (iked) and Phase 1 (isakmpd)

2020-04-20 Thread Stephan Mending
Hi,
I was wondering if there was a reason why there are no AEAD Suites implemented 
for initial IKEX in iked or phase 1 in isamkmpd ? Even though iked's childSAs
support it and Phase 2 in isakmpd does as well ? Is it just lack of time ? 
Because for example strongswan does exactly support that. Using GCM Suites 
and/or
Chacha20Poly1305. 

Thanks for your time. 

Best regards, 
Stephan