This problem was introduced in bug 13029. When I create a N2N IPsec connection that uses a PSK and edit the connection, after hitting the Save button, the PSK is being replaced by a base64-encoded version of itself which in return breaks the connection. Could you have a look at this? I think there is a line missing that decodes the entered data again. Maybe we should not even send it back to the user to not leak the secret? Let me know if you need my help.
Addendum: the PSK is base64 encoded with every save. But its not decoded when being sent to the webif - when you show the password-input field you see, it's not base64 decoded. For me this is required to solve it: --- vpnmain.cgi_o 2025-03-31 19:16:25.654451280 +0200 +++ vpnmain.cgi 2025-04-27 16:01:14.218525212 +0200 @@ -1678,7 +1678,11 @@ $cgiparams{'NAME'} = $confighash{$cgiparams{'KEY'}}[1]; $cgiparams{'TYPE'} = $confighash{$cgiparams{'KEY'}}[3]; $cgiparams{'AUTH'} = $confighash{$cgiparams{'KEY'}}[4]; - $cgiparams{'PSK'} = $confighash{$cgiparams{'KEY'}}[5]; + if ($confighash{$cgiparams{'KEY'}}[40] eq 'YES') { + $cgiparams{'PSK'} = MIME::Base64::decode_base64($confighash{$cgiparams{'KEY'}}[5]); + } else { + $cgiparams{'PSK'} = $confighash{$cgiparams{'KEY'}}[5]; + } $cgiparams{'LOCAL'} = $confighash{$cgiparams{'KEY'}}[6]; $cgiparams{'LOCAL_ID'} = $confighash{$cgiparams{'KEY'}}[7]; my @local_subnets = split(",", $confighash{$cgiparams{'KEY'}}[8]); @@ -2326,6 +2330,7 @@ $confighash{$key}[37] = $cgiparams{'INTERFACE_ADDRESS'}; $confighash{$key}[38] = $cgiparams{'INTERFACE_MTU'}; $confighash{$key}[39] = join("|", split(",", $cgiparams{'DNS_SERVERS'})); + $confighash{$key}[40] = $cgiparams{'BASE_64'}; # free unused fields! $confighash{$key}[15] = 'off';
Patch with fix has been submitted. https://lists.ipfire.org/development/20250427185851.25437-1-adolf.belka@ipfire.org/T/#u https://patchwork.ipfire.org/project/ipfire/patch/20250427185851.25437-1-adolf.belka@ipfire.org/ The patch has been tested out with new PSK generation, pressing Save on the Edit screen, even without changing the PSK and with restored IPSec PSK connections that had the PSK without any Base64 encoding. In the latter case the PSK stays unencoded until the Save button in the Edit screen is pressed, even if the PSK has not been changed. Going in to the Edit screen multiple times and pressing Save without changing the PSK now leaves the PSK base64 encoded the once. The problem in the original patch for this problem in the previous bug report was that it did the base64 encoding when the BASE_64 parameter was set to "YES" rather than when it was set to "". When the BASE_64 parameter was set to "YES" then the PSK was base64 encoded and the BASE_64 parameter was left at "YES", which meant that the next time it was edited the base64 encoded PSK was base64 encoded again. The base64 encoding should have been carried out when the BASE_64 parameter was set to "" and after encoding the BASE_64 parameter changed to "YES" so that it was not further base64 encoded.
If I use this patch (comment 2) on my Core193 installation I have this problem: when setting a new psk it's not saved as it should. I think the password-input-field should contain the original/real psk-value - whether base64 is set or not. (Since I'm missing the core194 changes the patch offsets about 20 lines, but it seems the core194-changes did not address the base64-thing).
Hello boys, great conversation here! I have tested both of your solutions and you both seem to have been very close. I agree with Christian that we would have to send the PSK decoded back to the browser because we otherwise don't know whether it is already encoded or not. This is where Adolf's patch had the problem that if you edit a connection that is encoding the PSK, the new key will be assumed to be already encoded and therefore the connection won't work. Christian's patch has a very similar issue. It stores the key correctly, but then resets the flag that it is encoded. I would like to propose a third patch. Please tell me if I broke something else: > https://lists.ipfire.org/development/20250428094551.704156-1-michael.tremer@ipfire.org/T/#u
I missed to test out changing the PSK for an existing PSK connection. Confirm, with my patch that fails to work properly. Have just patched my vm setup with @Michael's patch and have tested it. Tried with an existing non base64 encoded PSK restored from a backup. So the PSK in the config file and the ipsec.secrets file are both unencoded. Going into the edit mode for this connection and pressing Save without changing the PSK resulted in the previous PSK now being unencoded in the ipsec.secrets file but base64 encoded in the config file. Correct. Going into the edit screen and pressing Save multiple times did not change the PSK values in either file. Correct. Then I tested modifying the PSK in the Edit screen and pressing Save after the modification. This gave an updated PSK unencoded in the ipsec.secrets file and a correctly encoded version in the config file. Correct. I then tried a completely new PSK connection. Unfortunately this comes up with the Error Message Error messages Name must only contain characters. This message occurs, even if the PSK I use has only lowercase letters. Will have a look at the code to see what is causing this. This might be a hangover from the fix from bug13029 as there was a change in that to the check for characters but that should allow all characters except a single quote as that is used in the ipsec.secrets file to define the start and end of the PSK. I think we are very close now. Just one more push and we will be there.
Ignore the last part of my Comment 5. I had put a space into the name of the connection. It was nothing to do with the PSK. I didn't read the Error Message closely enough. So a completely fresh PSK connection also gives an unencoded PSK in ipsec.secrets and the encoded version in the config file. Correct. So @Michael's patch is working in all the testing I have now done.
Also just confirmed that putting in a single quote into the PSK gets flagged up as an error, which it should be, but otherwise a whole range of special characters can be used in the PSK without any problem, including the very original issue from the previous bug of using a comma successfully. Will be good to hear if everything also works well for @Christian.
Patch looks plausible and my small set of tests (productive system) didn't show a problem. Thx
Thank you everyone to work on this and test the changes. I merged the patch into master: > https://git.ipfire.org/?p=ipfire-2.x.git;a=commitdiff;h=f9f02b4c244fea3025245348678bb08bbfbd48a8
I have tested out the final patch on my CU194 Testing with build master/262809b8 Going into the edit screen for the PSK and pressing Save several times, leaves the PSK as it was defined. The PSK can also be changed in the screen and after Saving the files hold the correct PSK (base64 encoded in the config file). I have also tested out doing a restore of a backup with a PSK that had the value base64 encoded in the config file. The restore worked fine and config file had the base64 encoded value and the ipsec.secrets file had the unencoded version. I then did a restore of a backup with a PSK before the base64 encoding, so the value in the backup had the PSK in the config file and the ipsec.secrets file both not encoded by base64. After doing the restore the PSK connection worked with that PSK. However going into the edit window and pressing the Save button then resulted in a wrong PSK in the ipsec.secrets file. I suspect what was done is that the code then took the value in the config file and base64 decoded it and placed that in the ipsec.secrets file. However it happened the result was the wrong values in both the config file and the ipsec.secrets file. I will have a look at the restore section of the backup.pl. We probably need a section that checks if the PSK in the ipsec.secrets file is the same as the PSK in the config file and if yes then to change the value in the config file to the base64 encoded version.
Ignore the end part of my comment 10. I didn't have an existing backup with the PSK entries prior to the base64 encoding change, so I created by changing the PSK in the config file to the unencoded version but I forgot to clear the BASE64 [40] key value of YES. I just repeated my test with my generated old backup version, which now had the BASE64 [40] key value left blank (missing) from the config file. This now meant that when the save button was pressed, it was treated as unencoded but then converted to the base64 encoded version. So my testing has found all of the variations I have tested to work without problems, including restoring a base64 encoded backup version and restoring a backup before any base64 encoding was being applied.
https://www.ipfire.org/blog/ipfire-2-29-core-update-194-released