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
Allow manual selection of IPsec IKE Pseudo-Random Function (PRF). Issue #9309 #4106
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks OK, but needs some slight wording changes, and testing.
@@ -944,6 +969,13 @@ function build_eal_list() { | |||
$pconfig['splitconn'] | |||
)); | |||
|
|||
$section->addInput(new Form_Checkbox( | |||
'prfselect_enable', | |||
'Allow select PRF', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would label this PRF Selection
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
$section->addInput(new Form_Checkbox( | ||
'prfselect_enable', | ||
'Allow select PRF', | ||
'Enable this to manually select pseudo-random function (PRF). In most cases this is not required.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use this instead: Enable manual Pseudo-Random Function (PRF) selection
And then add a setHelp()
with: Manual PRF selection is typically not required, but can be useful in combination with AEAD Encryption Algorithms such as AES-GCM
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
pfSense 2.5.0.a.20191203.0148
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This now has conflicts that must be resolved. Please review the significant changes made to IPsec for https://redmine.pfsense.org/issues/9603 before attempting to resolve the conflicts.
conflicts resolved |
src/etc/inc/ipsec.inc
Outdated
@@ -1895,7 +1899,7 @@ function ipsec_setup_proposal_entry(& $ph2ent, & $algo_arr, $ealg_id, $keylen) { | |||
/* If multiple hash algorithms are present, loop through and add them all. */ | |||
if (!empty($ph2ent['hash-algorithm-option']) && is_array($ph2ent['hash-algorithm-option'])) { | |||
foreach ($ph2ent['hash-algorithm-option'] as $halgo) { | |||
$proposal[] = ipsec_setup_proposal_algo($ealg_id, $keylen, $halgo, $ph2ent['pfsgroup']); | |||
$proposal[] = ipsec_setup_proposal_algo($ealg_id, $keylen, $halgo, False, $ph2ent['pfsgroup']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should either be false
or FALSE
, preferably false
since that's what is used elsewhere in the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/etc/inc/ipsec.inc
Outdated
@@ -2013,9 +2017,13 @@ function ipsec_setup_tunnels() { | |||
empty($p1enc['hash-algorithm'])) { | |||
continue; | |||
} | |||
if ($ph1ent['prfselect_enable'] != 'yes') { | |||
$p1enc['prf-algorithm'] = False; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should either be false
or FALSE
, preferably false
since that's what is used elsewhere in the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@vktg please rebase your fork to fix conflicts. While there, could you please squash all these commits in a single one to make the commit history cleaner? |
all done |
@vktg please rebase your fork and resolve conflicts again |
Successfully tested: |
default (current) behavior if checkbox disabled
pfSense <-> Cisco IOS tunnel tested