Skip to content
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

DietPi-Software | Some enhancement for Baikal #3789

Merged
merged 17 commits into from
Sep 25, 2020
Merged

DietPi-Software | Some enhancement for Baikal #3789

merged 17 commits into from
Sep 25, 2020

Conversation

Joulinar
Copy link
Collaborator

Status: ready

According BaiKal installation guide, webserver directory would need to be owned by www-data:www-data

According BaiKal upgrade guide, whole Specific as well as config directories, would need to be protected to be overwritten.

Commit list/description:

  • DietPi-Software | Some enhancements for BaiKal to have correct user permission after installation and to protect folders during re-installation.

MichaIng and others added 5 commits August 27, 2020 15:27
According BaiKal installation guide, webserver directory would need to be owned by www-data:www-data
- https://sabre.io/baikal/install/

According BaiKal upgrade guide, whole Specific as well as config directories, would need to be protected to be overwritten.
- https://sabre.io/baikal/upgrade/
@Joulinar Joulinar changed the title Baikal DietPi-Software | Some enhancement for Baikal Sep 24, 2020
@Joulinar
Copy link
Collaborator Author

looks like I've done something wrong as my PR contains some of your older commits 🤔

@MichaIng
Copy link
Owner

Your PR is based on master branch it seems, but not a big issue since no code is affected that has been changed in dev since last release and the additional commits are just the merge commits for beta and stable releases. I anyway merge them back into dev by times to resolve: "This branch is 138 commits ahead, 4 commits behind master."

@MichaIng MichaIng added this to the v6.33 milestone Sep 25, 2020
@Joulinar
Copy link
Collaborator Author

yes indeed, I created it against master in first place but tried to correct it by switching to dev later on

@MichaIng
Copy link
Owner

MichaIng commented Sep 25, 2020

Switching the base branch does not update the files in the head branch. But as said it's okay, only the CodeFactor warnings are annoying as this has been all gone through and fixed just in dev 😄.

Okay lets go into content:

  • Good point in preserving the config dir on reinstall 👍.
  • In our case, the Specific dir does not contain the database since MariaDB is setup for this. Only the SQLite database file would be stored there. But just in case users change the setup, it makes sense to block direct access. At best we simply create new webserver config files for that. I wonder why their Apache2 example config does not block access to Specific but explicitly disables .htaccess and grants access, while Nginx config makes me assume that there is a .htaccess... Contradicting 😄
  • chown -R www-data:root /var/www/baikal/Specific is what we should do to grant only the required permissions... ay excluding the .htaccess file.

There is a lot more we need to do:

@Joulinar
Copy link
Collaborator Author

ok than we would need to grand access to Specific as well as config as this was required during initial setup process by Baikal.

@MichaIng
Copy link
Owner

MichaIng commented Sep 25, 2020

I'll do installs both ways (source build with composer compared to pre-build release) and compare the result.

Reminder: https://dietpi.com/phpbb/viewtopic.php?p=1502#p1502

  • Update as the config will now for sure be in new config dir.
  • Recheck if MariaDB restart is required, as I don't think so.

@Joulinar
Copy link
Collaborator Author

Hi,

I need to say, i should have checked it better before raising the PR

I did one more installation test.

  • folder Specific is already owned by user www-data ✔️
  • folder config would need to be changed to www-data
  • Restart of MariaDB was not needed to finish initial setup

+ DietPi-Software | Baikal: Further install consistency and updates
@MichaIng
Copy link
Owner

Yes I also found the chown in config step already. What was really necessary is the chown of the config dir since this is new. In the past the config was in the Specific dir as well.

+ DietPi-Software | Baikal: Syntax
@MichaIng
Copy link
Owner

Going with the release archive seems to be the slim/cleaned up option that is more suitable for end users and follows the official install instructions:

2020-09-25 14:36:25 root@VM-Buster:/var/www# l baikal/vendor/ baikal_bak/vendor/
baikal/vendor/:
total 28K
-rw-r--r-- 1 root root  178 Jun 12 23:36 autoload.php
drwxr-xr-x 2 root root 4.0K Jun 12 23:36 bin
drwxr-xr-x 2 root root 4.0K Jun 12 23:36 composer
drwxr-xr-x 3 root root 4.0K Jun 12 23:36 psr
drwxr-xr-x 8 root root 4.0K Jun 12 23:36 sabre
drwxr-xr-x 5 root root 4.0K Jun 12 23:36 symfony
drwxr-xr-x 3 root root 4.0K Jun 12 23:36 twig

baikal_bak/vendor/:
total 44K
-rw-r--r--  1 root root  178 Sep 25 13:56 autoload.php
drwxr-xr-x  2 root root 4.0K Sep 25 13:56 bin
drwxr-xr-x  4 root root 4.0K Sep 25 13:56 composer
drwxr-xr-x  4 root root 4.0K Sep 25 13:56 doctrine
drwxr-xr-x  3 root root 4.0K Sep 25 13:56 friendsofphp
drwxr-xr-x  3 root root 4.0K Sep 25 13:56 paragonie
drwxr-xr-x  3 root root 4.0K Sep 25 13:56 php-cs-fixer
drwxr-xr-x  5 root root 4.0K Sep 25 13:56 psr
drwxr-xr-x  8 root root 4.0K Sep 25 13:56 sabre
drwxr-xr-x 22 root root 4.0K Sep 25 13:56 symfony
drwxr-xr-x  3 root root 4.0K Sep 25 13:56 twig
2020-09-25 14:36:38 root@VM-Buster:/var/www# du -sm baikal baikal_bak
14      baikal
19      baikal_bak

We'll go with that 👍.

+ DietPi-Software | Baikal: Move to pre-build release install, skipping composer usage
@MichaIng
Copy link
Owner

MichaIng commented Sep 25, 2020

Okay, last but not least the webserver config to harden access restrictions.

Testing

🈯 VM Stretch Lighttpd
🈯 VM Buster Lighttpd
🈯 VM Bullseye Lighttpd
🈯 VM Buster Apache2
🈯 VM Buster Nginx

+ DietPi-Software | Baikal: Create general Apache config for access hardening
+ DietPi-Software | Baikal: Create general Nginx config for access permission hardening
+ DietPi-Software | Baikal: Add general Lighttpd config for access permission hardenings
+ DietPi-Software | Baikal: Failsafe Lighttpd config
+ DietPi-Software | Baikal: Block access to all .git* and .ht* files on Nginx as well
+ DietPi-Software | Baikal: Install webserver configs to harden web access permissions
+ DietPi-Software | Baikal: Fix due to different top-level directory in release archive
+ CHANGELOG | Baikal: Update (re)install procedure to cover the new config directory and use the pre-packed release archives instead of raw source and composer. Additionally webserver configs have been added to harden access permissions.
@MichaIng
Copy link
Owner

Ready from my side, I'll merge. Great to have it given some general updates and enhancement.

@MichaIng MichaIng merged commit 26eea15 into dev Sep 25, 2020
@MichaIng MichaIng deleted the baikal branch September 25, 2020 15:47
@Joulinar
Copy link
Collaborator Author

I thought it would be an easy enhancement but turned into a full redevelopment. 🙄

Well done @MichaIng

@MichaIng
Copy link
Owner

I thought it would be an easy enhancement but turned into a full redevelopment.

Adding config/ to chown and reinstall preservation would have been enough to restore full compatibility indeed, but when already touching Baikal after a long time, it's good to review everything against current official install instructions/recommendation 🙂.

@Joulinar
Copy link
Collaborator Author

Joulinar commented Sep 25, 2020

ok probably we would need to perform some more adjustements. I tried the upgrade path from 0.6.1 to 0.7.1 using the new dietpi-software script. If I'm not mistaken, there are some major changes between version 0.6 and 0.7. This include the config directory as well. Because it's not yet existing, and therefore can't be saved 😉
cp: cannot stat '/var/www/baikal/config/.': No such file or directory

Ok I bypassed this and finished installation. Opening the website after a reboot will bring back the Baikal installer, which is expected due to the changes from 0.6 to 0.7. I followed the small installer but ended up with following

The constant PROJECT_DB_MYSQL_HOST, containing the MySQL host name, is not set.
You should set it in config/baikal.yaml

And indeed, whole DB information are missing within config/baikal.yaml

database:
    sqlite_file: /var/www/baikal/Specific/db/db.sqlite
    mysql: true
    mysql_host: ''
    mysql_dbname: ''
    mysql_username: ''
    mysql_password: ''
    encryption_key: removed
    configured_version: ''

Will run some more tests on the upgrade path 0.6 > 0.7.

@MichaIng
Copy link
Owner

Ah makes sense to not attempt to migrate a directory that does not exist.

Yes not only the new config dir has been added but also the config file name and type has changed. It was php before, now it is yaml, no chance to migrate from our end. I would have hoped for an automated migration by Baikal itself, e.g. it detects the old config file in Specific/ and then migrates the information from there into the new config file.

Opening the website without new config file does not trigger the initial setup when you open the web UI right? There is a flag file in Specific/ which needs to be removed, AFAIK. Probably we can implement the migration ourself, at least, when the old config file is found and the new not, moving the old one to some backup location for review and removing the install flag file, then print an info that Baikal needs to be reconfigured via web UI.
I hope the database itself is migrated successfully?

@MichaIng
Copy link
Owner

https://github.com/sabre-io/Baikal/releases/tag/0.7.0
Actually that sounds like migration is done mostly automated 🤔.

Fix for not attempting to migrate nonexistent directories: 1181e46

@Joulinar
Copy link
Collaborator Author

Joulinar commented Sep 25, 2020

Yes it will be done mostly automatically while opening admin panel first time after update. However my new configuration file was empty afterwards. Will run some more test.

@Joulinar
Copy link
Collaborator Author

Joulinar commented Sep 25, 2020

ok I did one more test on upgrading from 0.6 to 0.7 but it's resulting on the same empty config/baikal.yaml. Even if during the upgrade process all data are shown correctly, the config file is not going to be updated with database information.

WebGui picture attached. Click to expand!

image

baikal.yaml attached. Click to expand!
root@DietPi3:/var/www/baikal/config# cat baikal.yaml
system:
    configured_version: 0.6.1
    timezone: Europe/Berlin
    card_enabled: true
    cal_enabled: true
    dav_auth_type: Digest
    admin_passwordhash: removed
    auth_realm: BaikalDAV
    base_uri: ''
    invite_from: noreply@192.168.0.12
database:
    sqlite_file: /var/www/baikal/Specific/db/db.sqlite
    mysql: true
    mysql_host: ''
    mysql_dbname: ''
    mysql_username: ''
    mysql_password: ''
    encryption_key: removed
    configured_version: ''
root@DietPi3:/var/www/baikal/config#

Maybe an issue on Baikal update process. 🤔

But good news, your fix for nonexistent config directory is working well.

EDIT: Yap indeed an issue on Baikal side. There are a couple of reports on their GitHub. Some of them reporting same issue with empty baikal.yaml after upgrading from 0.6.1 > 0.7.1

@MichaIng
Copy link
Owner

Many thanks for testing and research. Maybe we can help a bid fixing that at Baikal. I am wondering what the differences are between this web UI setup on fresh install an on migration. I'd have suggested that in both cases, when hitting "Save changes" exactly the same is done, but obviously that is not the case. And that makes me worry that similar issues could happen when you want to change database settings on an already setup instance just via web UI settings.

@MichaIng
Copy link
Owner

MichaIng commented Sep 26, 2020

Okay, I found (as fast as I was able to follow the code) two migration issues which basically match those two reports:

  • Pre-0.5.1 migration: Error updating from 0.5.0 to 0.7 sabre-io/Baikal#942
    • For me it looks like the database connection settings are read from the new baikal.yaml, but those are empty since the old config.system.php has not yet been parsed/migrated.
    • From 0.5.1 on this is not an issue anymore, since no database (content) migration is done from that version on.
  • Pre-0.7.0 migration: Baikal does not migrate mysql details on upgrade sabre-io/Baikal#946
    • The just parsed config.system.php database settings are overwritten by parsing again the empty settings from baikal.yaml afterwards, which was introduced with 0.7.1, hence pre-0.7.0 => 0.7.0 => 0.7.1 works.

Both issues seem to be fixed with: sabre-io/Baikal#979

@Joulinar
Copy link
Collaborator Author

let's hope it will reach master branch soon, to have this fixed before releasing DietPi 6.33

@MichaIng
Copy link
Owner

Fix has been merged upstream: sabre-io/Baikal#979

@Joulinar
Copy link
Collaborator Author

Oki let me do some testing for the entire upgrade path.

@Joulinar
Copy link
Collaborator Author

Joulinar commented Oct 18, 2020

still empty config/baikal.yaml even if it was predefined on upgrade gui. But it's still 0.7.1. Should the version number not increasing?

database:
    sqlite_file: /var/www/baikal/Specific/db/db.sqlite
    mysql: true
    mysql_host: ''
    mysql_dbname: ''
    mysql_username: ''
    mysql_password: ''

I guess we would need to wait for a new release to be published.

@MichaIng
Copy link
Owner

Ah that fix has not yet been released: https://github.com/sabre-io/Baikal/releases
So currently one would need to install the master branch code manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants