Skip to content

Commit 958914f

Browse files
Ma27Ma27
committed
nextcloud: drop password regeneration behavior
While updating to 23.0.9/24.0.5[1], it was discovered that Nextcloud silently changes db passwords in some cases (in case of MySQL in **all** now). This is inherently incompatible with our module. Further context is provided in the patch file attached to this commit. [1] #190646, see comments below
1 parent 1801529 commit 958914f

2 files changed

Lines changed: 137 additions & 0 deletions

File tree

‎pkgs/servers/nextcloud/0001-Setup-remove-custom-dbuser-creation-behavior.patch‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
From 045f33745f863ba20acfc3fe335c575d9cd87884 Mon Sep 17 00:00:00 2001
2+
From: Maximilian Bosch <maximilian@mbosch.me>
3+
Date: Sat, 10 Sep 2022 15:18:05 +0200
4+
Subject: [PATCH] Setup: remove custom dbuser creation behavior
5+
6+
Both PostgreSQL and MySQL can be authenticated against from Nextcloud by
7+
supplying a database password. Now, during setup the following things
8+
happen:
9+
10+
* When using postgres and the db user has elevated permissions, a new
11+
unprivileged db user is created and the settings `dbuser`/`dbpass` are
12+
altered in `config.php`.
13+
14+
* When using MySQL, the password is **always** regenerated since
15+
24.0.5/23.0.9[1].
16+
17+
I consider both cases problematic: the reason why people do configuration
18+
management is to have it as single source of truth! So, IMHO any
19+
application that silently alters config and thus causes deployed
20+
nodes to diverge from the configuration is harmful for that.
21+
22+
I guess it was sheer luck that it worked for so long in NixOS because
23+
nobody has apparently used password authentication with a privileged
24+
user to operate Nextcloud (which is a good thing in fact).
25+
26+
[1] https://github.com/nextcloud/server/pull/33513
27+
---
28+
lib/private/Setup/MySQL.php | 53 --------------------------------
29+
lib/private/Setup/PostgreSQL.php | 26 ----------------
30+
2 files changed, 79 deletions(-)
31+
32+
diff --git a/lib/private/Setup/MySQL.php b/lib/private/Setup/MySQL.php
33+
index 2c16cac3d2..9b2265091f 100644
34+
--- a/lib/private/Setup/MySQL.php
35+
+++ b/lib/private/Setup/MySQL.php
36+
@@ -142,59 +142,6 @@ class MySQL extends AbstractDatabase {
37+
$rootUser = $this->dbUser;
38+
$rootPassword = $this->dbPassword;
39+
40+
- //create a random password so we don't need to store the admin password in the config file
41+
- $saveSymbols = str_replace(['\"', '\\', '\'', '`'], '', ISecureRandom::CHAR_SYMBOLS);
42+
- $password = $this->random->generate(22, ISecureRandom::CHAR_ALPHANUMERIC . $saveSymbols)
43+
- . $this->random->generate(2, ISecureRandom::CHAR_UPPER)
44+
- . $this->random->generate(2, ISecureRandom::CHAR_LOWER)
45+
- . $this->random->generate(2, ISecureRandom::CHAR_DIGITS)
46+
- . $this->random->generate(2, $saveSymbols)
47+
- ;
48+
- $this->dbPassword = str_shuffle($password);
49+
-
50+
- try {
51+
- //user already specified in config
52+
- $oldUser = $this->config->getValue('dbuser', false);
53+
-
54+
- //we don't have a dbuser specified in config
55+
- if ($this->dbUser !== $oldUser) {
56+
- //add prefix to the admin username to prevent collisions
57+
- $adminUser = substr('oc_' . $username, 0, 16);
58+
-
59+
- $i = 1;
60+
- while (true) {
61+
- //this should be enough to check for admin rights in mysql
62+
- $query = 'SELECT user FROM mysql.user WHERE user=?';
63+
- $result = $connection->executeQuery($query, [$adminUser]);
64+
-
65+
- //current dbuser has admin rights
66+
- $data = $result->fetchAll();
67+
- $result->closeCursor();
68+
- //new dbuser does not exist
69+
- if (count($data) === 0) {
70+
- //use the admin login data for the new database user
71+
- $this->dbUser = $adminUser;
72+
- $this->createDBUser($connection);
73+
-
74+
- break;
75+
- } else {
76+
- //repeat with different username
77+
- $length = strlen((string)$i);
78+
- $adminUser = substr('oc_' . $username, 0, 16 - $length) . $i;
79+
- $i++;
80+
- }
81+
- }
82+
- }
83+
- } catch (\Exception $ex) {
84+
- $this->logger->info('Can not create a new MySQL user, will continue with the provided user.', [
85+
- 'exception' => $ex,
86+
- 'app' => 'mysql.setup',
87+
- ]);
88+
- // Restore the original credentials
89+
- $this->dbUser = $rootUser;
90+
- $this->dbPassword = $rootPassword;
91+
- }
92+
-
93+
$this->config->setValues([
94+
'dbuser' => $this->dbUser,
95+
'dbpassword' => $this->dbPassword,
96+
diff --git a/lib/private/Setup/PostgreSQL.php b/lib/private/Setup/PostgreSQL.php
97+
index bc24909dc3..e49e5508e1 100644
98+
--- a/lib/private/Setup/PostgreSQL.php
99+
+++ b/lib/private/Setup/PostgreSQL.php
100+
@@ -45,32 +45,6 @@ class PostgreSQL extends AbstractDatabase {
101+
$connection = $this->connect([
102+
'dbname' => 'postgres'
103+
]);
104+
- //check for roles creation rights in postgresql
105+
- $builder = $connection->getQueryBuilder();
106+
- $builder->automaticTablePrefix(false);
107+
- $query = $builder
108+
- ->select('rolname')
109+
- ->from('pg_roles')
110+
- ->where($builder->expr()->eq('rolcreaterole', new Literal('TRUE')))
111+
- ->andWhere($builder->expr()->eq('rolname', $builder->createNamedParameter($this->dbUser)));
112+
-
113+
- try {
114+
- $result = $query->execute();
115+
- $canCreateRoles = $result->rowCount() > 0;
116+
- } catch (DatabaseException $e) {
117+
- $canCreateRoles = false;
118+
- }
119+
-
120+
- if ($canCreateRoles) {
121+
- //use the admin login data for the new database user
122+
-
123+
- //add prefix to the postgresql user name to prevent collisions
124+
- $this->dbUser = 'oc_' . strtolower($username);
125+
- //create a new password so we don't need to store the admin config in the config file
126+
- $this->dbPassword = \OC::$server->getSecureRandom()->generate(30, ISecureRandom::CHAR_ALPHANUMERIC);
127+
-
128+
- $this->createDBUser($connection);
129+
- }
130+
131+
$this->config->setValues([
132+
'dbuser' => $this->dbUser,
133+
--
134+
2.36.2
135+

‎pkgs/servers/nextcloud/default.nix‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ let
1313
inherit sha256;
1414
};
1515

16+
patches = [ ./0001-Setup-remove-custom-dbuser-creation-behavior.patch ];
17+
1618
passthru.tests = nixosTests.nextcloud;
1719

1820
installPhase = ''

0 commit comments

Comments
 (0)