Core: system: rework encryption flow, headers and nextcloud optional password

[kevenwyld]

I just tried the nextcloud configuration backup option (really great feature by the way) however the file which is uploaded to nextcloud appears to be encrypted. A long block of letters, numbers, and symbols with lines broken at 77 characters. It is not the same as if I select "Download Configuration" which is cleartext.

Looking at the docs here there doesn't seem to be any mention of encryption for nextcloud, only for google drive. Also the nextcloud section of System: Configuration: Backups does not have any options regarding encryption.

I tried restoring the file in a vm but the restore failed.

Any suggestions, or am I missing something in the docs?

Thanks in advance.

EDIT:
Versions OPNsense 18.1.11-amd64
FreeBSD 11.1-RELEASE-p11
LibreSSL 2.6.5

Filename: config-<firewall_name>-2018-07-10_01:07:00.xml

[fabianfrz]

@fichtner I was able to decrypt the config uploaded to nextcloud with an additional parameter:
-md md5
It is very likely not a stable API or you missed something in the release notes. In that case it currently also breaks the Google Drive backups. Note: OPNsense is on LibreSSL while my Laptop where I decrypted the file is using OpenSSL 1.1.0

Found the solution here:
https://stackoverflow.com/questions/34304570/how-to-resolve-the-evp-decryptfinal-ex-bad-decrypt-during-file-decryption

[fabianfrz]

@fichtner this works on my machine:
cat /tmp/config-opn.sense-2018-07-09_19_18_06.xml | base64 -di | openssl enc -d -aes-256-cbc -md md5 -pass pass:PASSWORD

[kevenwyld]

Ohh, it's encrypted with the nextcloud client password. That's... not exactly clear in the docs. I'll see if I can come up with some good wording for that and submit a PR (unless either of you object).

The workaround @fabianfrz posted does work for me. Is it suppose to use the stored password to decrypt automatically when you try to restore? Or is the intended workflow to decrypt the file manually first and then restore with the resulting cleartext?

[fabianfrz]

In theory, you should be able to restore it in the coresponding dialog. For some reason the restore does not work correctly. The restore with the nextcloud password does not work because the restore is not aware of the Nextcloud backup implementation.

[StrikerTwo]

Can we make the encryption optional? I appreciate the "security first" approach, but my Nextcloud instance is already encrypted, so I'd like to keep it simple.

[tmsmr]

+1 for optional encryption.
IMO: It seems the implementation changes too often and there is no backward compatibility in the restore process!? This may lead to horrible results in desaster-recovery scenarios.

[fichtner]

IMO: It seems the implementation changes too often and there is no backward compatibility

Where? NextCloud or OPNsense?

[fabianfrz]

If anyone wants to try:
Here is a solution for a separate password: ec9c291

[tmsmr]

@fichtner Neither of them. I misinterpreted the comments in this issue and rushed ahead to complain :disappointed:.
My problem was (like from @fabianfrz noticed already), that the default md of my OpenSSL version (OpenSSL 1.1.0h) seems to differ from the one that is bundled with OPNsense (OpenSSL 1.0.2o).
When OPNSense updates the OpenSSL version, a backup made with the old version may be invalid/corrupt because currently there is no explicit selection of the md (https://github.com/opnsense/core/blob/master/src/opnsense/mvc/app/library/OPNsense/Backup/Base.php#L51).

[fichtner]

@opthomas-prime no problem, thanks for explaining. It's a bit problematic indeed. The old code requires MD5 so we may need to reengineer the wrapping to add a version tag or something so we can auto-set the parameter. But in any case, changing away from the old default will leave older versions worse off, although that seems unavoidable in the mid- and long-term.

[fabianfrz]

@fichtner we can add a small header like

Hash: SHA-256
Cipher: AES-256-CBC

and if it is missing, we can fall back to an older variant.

[fichtner]

@fabianfrz good idea. added preliminary header support to the static page export. backup factory doesn't do this at the moment as it was left out, but we will have to push it back and make all of it compatible. with all of it in place we can later change the default and still cope with most scenarios although older versions will still break with the newer format old backups will work on newer versions. see 94b2df5

[fabianfrz]

@fichtner If you do the changes in base, can you move that call to it?
https://github.com/opnsense/core/blob/master/src/opnsense/mvc/app/library/OPNsense/Backup/Nextcloud.php#L144

It is the function that creates the line feeds in the config file to prevent editor crashes/freezes (too long line), so the backup strategies only get the ready to upload file (so it also works for Google Drive backup).

[fichtner]

to finish this up we need a unit test and merge the static implementation with the MVC one (they are the same now)

[fabianfrz]

@fichtner this is probably not very easy here because a network API (WebDAV) and files are involved. Maybe it is possible to mock curl_request else we have to mock the Nextcloud API here and I don't want to create such a nginx or sinatra app just for one test. Same is valid for Google Drive.

[fichtner]

I just need to test encrypt() and decrypt() which will be callable from the static pages so unittest is not an issue. the static page is an half-automated unittest then anyway :D