Copilot commented on code in PR #12764:
URL: https://github.com/apache/cloudstack/pull/12764#discussion_r2904984446
##########
utils/src/main/java/com/cloud/utils/net/NetUtils.java:
##########
@@ -1265,7 +1265,7 @@ public static boolean isValidS2SVpnPolicy(final String
policyType, final String
if (group == null && policyType.toLowerCase().matches("ike")) {
return false; // StrongSwan requires a DH group for the IKE
policy
}
- if (group != null &&
!group.matches("modp1024|modp1536|modp2048|modp3072|modp4096|modp6144|modp8192"))
{
+ if (group != null &&
!group.matches("modp1024|modp1536|modp2048|modp3072|modp4096|modp6144|modp8192|modp1024s160|modp2048s224|modp2048s256|CURVE_25519"))
{
Review Comment:
New DH groups were added to the validation allowlist, but the existing
`NetUtilsTest#testIsValidS2SVpnPolicy` doesn’t cover them. Please extend the
unit test to assert that `modp1024s160`, `modp2048s224`, `modp2048s256` and the
group-31 token are accepted (and that invalid variants still fail).
##########
ui/src/views/network/CreateVpnCustomerGateway.vue:
##########
@@ -275,12 +279,12 @@ export default {
initForm () {
this.formRef = ref()
this.form = reactive({
- ikeEncryption: 'aes128',
+ ikeEncryption: 'aes256',
ikeHash: 'sha1',
ikeversion: 'ike',
- ikeDh: 'Group 5(modp1536)',
- espEncryption: 'aes128',
- espHash: 'sha1',
+ ikeDh: 'Group 31(curve 25519)',
+ espEncryption: 'aes256',
Review 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.
##########
ui/src/views/network/CreateVpnCustomerGateway.vue:
##########
@@ -258,9 +258,13 @@ export default {
'Group 15': 'modp3072',
'Group 16': 'modp4096',
'Group 17': 'modp6144',
- 'Group 18': 'modp8192'
+ 'Group 18': 'modp8192',
+ 'Group 22': 'modp1024s160',
+ 'Group 23': 'modp2048s224',
+ 'Group 24': 'modp2048s256',
+ 'Group 31': 'curve25519'
},
- ikeDhGroupInitialValue: 'Group 5(modp1536)',
+ ikeDhGroupInitialValue: 'Group 31(curve25519)',
Review Comment:
`ikeDhGroupInitialValue` should match the `<a-select>` option `value` (e.g.,
`curve25519`), not the rendered label (`Group 31(curve25519)`); otherwise the
initial selection/submit logic becomes inconsistent.
```suggestion
ikeDhGroupInitialValue: 'curve25519',
```
##########
utils/src/main/java/com/cloud/utils/net/NetUtils.java:
##########
@@ -1265,7 +1265,7 @@ public static boolean isValidS2SVpnPolicy(final String
policyType, final String
if (group == null && policyType.toLowerCase().matches("ike")) {
return false; // StrongSwan requires a DH group for the IKE
policy
}
- if (group != null &&
!group.matches("modp1024|modp1536|modp2048|modp3072|modp4096|modp6144|modp8192"))
{
+ if (group != null &&
!group.matches("modp1024|modp1536|modp2048|modp3072|modp4096|modp6144|modp8192|modp1024s160|modp2048s224|modp2048s256|CURVE_25519"))
{
Review Comment:
`isValidS2SVpnPolicy` now allows `CURVE_25519`, but the UI submits
`curve25519` for DH group 31; this mismatch will cause valid policies to be
rejected. Align the accepted token(s) with what the UI/strongSwan use (e.g.,
accept `curve25519`, or make the match case-insensitive).
```suggestion
if (group != null &&
!group.toLowerCase().matches("modp1024|modp1536|modp2048|modp3072|modp4096|modp6144|modp8192|modp1024s160|modp2048s224|modp2048s256|curve25519"))
{
```
##########
ui/src/views/network/CreateVpnCustomerGateway.vue:
##########
@@ -275,12 +279,12 @@ export default {
initForm () {
this.formRef = ref()
this.form = reactive({
- ikeEncryption: 'aes128',
+ ikeEncryption: 'aes256',
Review Comment:
This hunk also changes the default IKE/ESP algorithms (e.g., aes128→aes256,
sha1→sha256). The PR title/description focus on DH group defaults; please
confirm these additional behavior changes are intentional and document them in
the PR description/linked issue if so.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
