Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ipsec: support multiple phase 1 proposals #1852

Closed
fichtner opened this issue Sep 28, 2017 · 18 comments

Comments

Projects
None yet
3 participants
@fichtner
Copy link
Member

commented Sep 28, 2017

Goal is to have single phase 1 compatibility with multiple mobile platforms.

  • Hash algos as selectpickers with multiple values phase1/2
  • Dh groups as selectpickers with multiple values phase1
  • Disable strict policy in phase1/2 under advanced (only if strictly necessary)

@fichtner fichtner added the feature label Sep 28, 2017

@fichtner fichtner added this to the 18.1 milestone Sep 28, 2017

@fichtner fichtner self-assigned this Sep 28, 2017

@mimugmail

This comment has been minimized.

Copy link
Member

commented Sep 30, 2017

This would be very nice feature since it guarantuees compatibility to older and newer clients.
For the beginning, a checkbox with a warning for unsetting the "!" in ike and/or esp would be enough.

Then older clients would negotiate lower encryption:

              Defaults  to  aes128-sha256.   The  daemon  adds  its  extensive
              default proposal to this default or the  configured  value.   To
              restrict  it  to the configured proposal an exclamation mark (!)
              can be added at the end.

              Note: As a responder, the daemon defaults to selecting the first
              configured  proposal that's also supported by the peer. This may
              be changed via strongswan.conf(5) to selecting the first accept-
              able  proposal  sent by the peer instead. In order to restrict a
              responder to only accept specific cipher suites, the strict flag
              (!,  exclamation mark) can be used, e.g: aes256-sha512-modp4096!

@mimugmail

This comment has been minimized.

Copy link
Member

commented Dec 5, 2017

https://forum.opnsense.org/index.php?topic=6544.0

This could fit the thing with "!" .. a button "Disable strict policy" would be nice.

@mimugmail

This comment has been minimized.

Copy link
Member

commented Dec 7, 2017

For me it seems if the other endpoint has multiple offers and the first one doesn't match, the connections fails (see newest reply in forum). If the "!" is removed it tries the next one and so one.

So I'd still suggest a new Checkbox "Disable strict policy" to remove it (default unchecked).

It's a compatibilty enhancement for devices supporting multiple values for P1 and P2 like ASA.

@fichtner fichtner modified the milestones: 18.1, 18.7 Dec 27, 2017

@fichtner fichtner removed this from the 18.7 milestone Feb 27, 2018

@fichtner fichtner added this to the 19.1 milestone Jul 15, 2018

@GurliGebis

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2018

This would be nice, since I have to configure both Android 8 and Windows 10 roadwarrior clients, which is not possible, since there is no configuration that both support.

@mimugmail

This comment has been minimized.

Copy link
Member

commented Aug 29, 2018

I'm also fighting ATM since Android 7 only support 1024 PFS in P1 and Mac requires 2048.
Trying to build a support matrix which client supports which VPN types .. but right now it doesn't make sense. Perhaps also multiple DHs would be nice.

Some months ago I opened an issue to add a checkbox which would just remove the ! from the ike and esp .. perhaps calling it Disable strict cipher selection

@fichtner

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2018

@mimugmail first part in 037a92f -- it only works in IKEv2, IKEv1 only takes the first and maybe we should warn users in the GUI about this. The enc algo modelling is less flexible and we need glue to bring it all together but we'll have a solution for 19.1 in any case. :)

Please test!

@fichtner

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2018

@mimugmail 36cde51 job done? :D

@mimugmail

This comment has been minimized.

Copy link
Member

commented Sep 12, 2018

This looks great!! Thanks! :)
Do you think we ca reverse the sort order?
ATM it's sha1 and 1024 first and sha512 and 2048 at last ...

@fichtner

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2018

Hmm, we could reverse the write order, assuming that the values are filled from smallest to highest, but it's somewhat dependent on the browser.

@mimugmail

This comment has been minimized.

Copy link
Member

commented Sep 13, 2018

Legacy code doesn't support ordering so we could order from config.xml to ipsec.conf?

@fichtner

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2018

The issue is that in the current modelling it supported only one entry, so there is no order by intention... now that we can choose multiple values we're left to guess what the best order is. 2 hashes and 2 dh groups are 4 combinations and even if we order by both hashes and dh groups from best to worst we have a middle gap where order is subject to interpretation due to the inner and outer loop. I'll add a final reverse, but that's as far as we can go before rewriting the entire handling of ike and esp entries.

fichtner added a commit that referenced this issue Sep 13, 2018

@fichtner

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2018

@mimugmail see c1c27c3 -- do we still want overrides for "!" or close as is? I'd add them only if strictly needed.

@mimugmail

This comment has been minimized.

Copy link
Member

commented Sep 13, 2018

Thanks, I'll test. But for this I'd left P2 untouched (which also has wrong ordering regarding hash) since it could break existing setups

@fichtner

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2018

I haven't changed P2 other than changing the hash checkboxes to a selectpicker for unified style.

@mimugmail

This comment has been minimized.

Copy link
Member

commented Sep 13, 2018

Ok, works perfect. Tested with Andoid, before the patch both devices were DH1024, now Android is DH1024 and iPAD is at DH2048, so, working as expected with highest security available 👍

@fichtner

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2018

Ok, close?

@mimugmail

This comment has been minimized.

Copy link
Member

commented Sep 13, 2018

Close, thanks 🥇

@fichtner

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2018

Pew pew!

@fichtner fichtner closed this Sep 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.