make dh group 31 default, support 22-24+31#12764
make dh group 31 default, support 22-24+31#12764DaanHoogland wants to merge 4 commits intoapache:4.22from
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #12764 +/- ##
=========================================
Coverage 17.60% 17.61%
- Complexity 15659 15663 +4
=========================================
Files 5917 5917
Lines 531394 531402 +8
Branches 64970 64971 +1
=========================================
+ Hits 93575 93593 +18
+ Misses 427269 427255 -14
- Partials 10550 10554 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates Site-to-Site VPN Customer Gateway defaults/options to support additional IKE DH groups (including group 31) and validates them server-side, aligning with newer strongSwan capabilities.
Changes:
- Expands server-side IKE/ESP policy DH-group validation to include groups 22–24 and 31.
- Extends the UI DH group dropdown to include groups 22–24 and 31, and changes the default DH group to 31.
- Updates UI default IKE/ESP algorithm selections (aes256/sha256 defaults).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| utils/src/main/java/com/cloud/utils/net/NetUtils.java | Extends DH-group allowlist for S2S VPN policy validation. |
| ui/src/views/network/CreateVpnCustomerGateway.vue | Adds DH groups 22–24 and 31 to the dropdown and changes default selections. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ikeDh: 'Group 31(curve 25519)', | ||
| espEncryption: 'aes256', |
There was a problem hiding this comment.
The default ikeDh is set to a label-like string (Group 31(...) / curve 25519), but the select stores the raw DH token (e.g., curve25519). Initialize form.ikeDh to the raw value and adjust the submit logic accordingly (it currently assumes parentheses parsing for the default), otherwise the default won’t be selected and ikepolicy may be built with an invalid DH group.
|
Strongswan 5.9.8 in ACS 4.22 (Debian Bookworm) does seem to support GROUP 31, so I guess it's ok to make it so and also add more options in the Customer Gateway for this and other versions. This is what I am seeing in the shell, so groups 1, 2, 5 , 14, 15, 16, 17, 18, 22, 23, 24, 31 can be the options, also keeping PR #12193 in mind:
|
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 'Group 31': 'curve25519' | ||
| }, | ||
| ikeDhGroupInitialValue: 'Group 5(modp1536)', | ||
| ikeDhGroupInitialValue: 'Group 31(curve25519)', |
There was a problem hiding this comment.
ikeDhGroupInitialValue is set to a label-like string (Group 31(curve25519)), but the <a-select> for IKE DH uses :value="DHGroups[group]" (e.g. curve25519). This mismatch makes the initial-value comparison in handleSubmit unreliable and contributes to generating invalid ikepolicy values. Set ikeDhGroupInitialValue to the actual option value (e.g. curve25519) or remove the special-casing logic entirely and always append values.ikeDh.
| ikeDhGroupInitialValue: 'Group 31(curve25519)', | |
| ikeDhGroupInitialValue: 'curve25519', |
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 17067 |
|
@blueorangutan package |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17069 |
abh1sar
left a comment
There was a problem hiding this comment.
Code Looks good.
New algorithms need to be tested that they are working correctly.

Description
This PR...
Fixes: #12505
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?