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

Fix #5344 Add the ability to provide and use a CA file for mysql servers with require_secure_transport=on #5418

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion server/database.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,10 +184,18 @@

/**
* @typedef {string|undefined} envString
* @param {{type: "sqlite"} | {type:envString, hostname:envString, port:envString, database:envString, username:envString, password:envString}} dbConfig the database configuration that should be written
* @param {{type: "sqlite"} | {type:envString, hostname:envString, port:envString, database:envString, username:envString, password:envString, caFilePath:envString}} dbConfig the database configuration that should be written
* @returns {void}
*/
static writeDBConfig(dbConfig) {
// Move CA file to the data directory
if (dbConfig.caFilePath) {
const dataCaFilePath = path.join(Database.dataDir, "mariadb-ca.pem");
fs.renameSync(dbConfig.caFilePath, dataCaFilePath);

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternate approach shall be , using a generic CA bundle : Eg: Mozilla. Then provide an env variable to override it, either as a path or the ca bundle content itself.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My first attempt at implementing this was to manage this entirely through environment variables but then I read from the contibuting.md the following:

I personally do not like something that requires a lot of configuration before you can finally start the app. The goal is to make the Uptime Kuma installation as easy as installing a mobile app.

And

Settings should be configurable in the frontend. Environment variables are discouraged, unless it is related to startup such as DATA_DIR

I believe this is something that should be configured through environment variables but if we want to respect the whishes of the maintainer, the user upload has to be there with all the complications it implies.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again just opinion :)

If we look at various feature requests for external Db, I believe it was not implemented early to keep the simplicity you quoted. A user base who may use a dedicated DB would be certainly having the knowledge and willingness to get it done, by reading docs and Configure As Code , even if there are efforts involved.

In that context, if we add a UI element, theoretically it is cluttering AND/OR complicating the experience of the user base who needs just out of the box experience.

On the other hand, if we do it via configuration those who need it can implement in a CAC fashion, hopefully it is efficient for those user base.

In a nutshell , if we do it via config, we may able to stretch the functionality without affecting the core slogan of simplicity.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@louislam Would it be possible to have your input on this as I do agree with @vsmanu proposal.

Changes are minimal and simpler with an environment variable only configuration but this would only be possible if Quote: Settings should be configurable in the frontend. is not an hard requirement.

dbConfig.caFilePath = dataCaFilePath;
dbConfig.ssl = undefined;
dbConfig.caFile = undefined;
}
fs.writeFileSync(path.join(Database.dataDir, "db-config.json"), JSON.stringify(dbConfig, null, 4));
}

Expand Down Expand Up @@ -259,11 +267,22 @@
throw Error("Invalid database name. A database name can only consist of letters, numbers and underscores");
}

let sslConfig = null;
let serverCa = undefined;
if (dbConfig.caFilePath) {
serverCa = [ fs.readFileSync(dbConfig.caFilePath, "utf8") ];
sslConfig = {
rejectUnauthorized: true,
ca: serverCa
};
}

const connection = await mysql.createConnection({
host: dbConfig.hostname,
port: dbConfig.port,
user: dbConfig.username,
password: dbConfig.password,
ssl: sslConfig
});

await connection.execute("CREATE DATABASE IF NOT EXISTS " + dbConfig.dbName + " CHARACTER SET utf8mb4");
Expand All @@ -277,7 +296,9 @@
user: dbConfig.username,
password: dbConfig.password,
database: dbConfig.dbName,
ssl: sslConfig,
timezone: "Z",
//ssl: sslConfig,
typeCast: function (field, next) {
if (field.type === "DATETIME") {
// Do not perform timezone conversion
Expand Down
31 changes: 31 additions & 0 deletions server/setup-database.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ class SetupDatabase {
dbConfig.dbName = process.env.UPTIME_KUMA_DB_NAME;
dbConfig.username = process.env.UPTIME_KUMA_DB_USERNAME;
dbConfig.password = process.env.UPTIME_KUMA_DB_PASSWORD;
dbConfig.caFilePath = process.env.UPTIME_KUMA_DB_CA_CERT;
Database.writeDBConfig(dbConfig);
}

Expand Down Expand Up @@ -206,13 +207,43 @@ class SetupDatabase {
return;
}

// Prevent someone from injecting a CA file path not generated by the code below
if (dbConfig.caFilePath) {
dbConfig.caFilePath = undefined;
}

if (dbConfig.caFile) {
const base64Data = dbConfig.caFile.replace(/^data:application\/octet-stream;base64,/, "");
const binaryData = Buffer.from(base64Data, "base64").toString("binary");
const tempCaDirectory = fs.mkdtempSync("kuma-ca-");
dbConfig.caFilePath = path.join(tempCaDirectory, "ca.pem");
try {
fs.writeFileSync(dbConfig.caFilePath, binaryData, "binary");
} catch (err) {

response.status(400).json("Cannot write CA file: " + err.message);
this.runningSetup = false;
return;
}
dbConfig.ssl = {
rejectUnauthorized: true,
ca: [ fs.readFileSync(dbConfig.caFilePath) ]
};
}

// Test connection
try {
let sslConfig = null;
if (dbConfig.ssl) {
sslConfig = dbConfig.ssl;
}

const connection = await mysql.createConnection({
host: dbConfig.hostname,
port: dbConfig.port,
user: dbConfig.username,
password: dbConfig.password,
ssl: sslConfig
});
await connection.execute("SELECT 1");
connection.end();
Expand Down
1 change: 1 addition & 0 deletions src/lang/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"setupDatabaseEmbeddedMariaDB": "You don't need to set anything. This docker image has embedded and configured MariaDB for you automatically. Uptime Kuma will connect to this database via unix socket.",
"setupDatabaseMariaDB": "Connect to an external MariaDB database. You need to set the database connection information.",
"setupDatabaseSQLite": "A simple database file, recommended for small-scale deployments. Prior to v2.0.0, Uptime Kuma used SQLite as the default database.",
"configureMariaCaFile": "You will sometimes need to provide a CA certificate to connect to database with 'require-secure-transport' on. Such as when using Azure MySql flexible servers. You can upload the CA file that will be used to enable a secure connection.",
"settingUpDatabaseMSG": "Setting up the database. It may take a while, please be patient.",
"dbName": "Database Name",
"Settings": "Settings",
Expand Down
32 changes: 31 additions & 1 deletion src/pages/SetupDatabase.vue
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,12 @@
<input id="floatingInput" v-model="dbConfig.dbName" type="text" class="form-control" required>
<label for="floatingInput">{{ $t("dbName") }}</label>
</div>
</template>

<div class="mb2 mt-3 short">
<p class="mb-2">{{ $t("configureMariaCaFile") }}</p>
<input id="ca-input" type="file" accept="application/x-pem-file, .pem" class="form-control" @change="onCaFileChange">
</div>
</template>
<button class="btn btn-primary mt-4 short" type="submit" :disabled="disabledButton">
{{ $t("Next") }}
</button>
Expand All @@ -117,6 +121,7 @@ export default {
username: "",
password: "",
dbName: "kuma",
caFile: ""
},
info: {
needSetup: false,
Expand Down Expand Up @@ -178,6 +183,15 @@ export default {
}
},

onCaFileChange(e) {
const fileReader = new FileReader();
fileReader.onload = () => {
this.dbConfig.caFile = fileReader.result;
console.log(this.dbConfig.caFile);
};
fileReader.readAsDataURL(e.target.files[0]);
},

test() {
this.$root.toastError("not implemented");
}
Expand All @@ -186,6 +200,22 @@ export default {
</script>

<style lang="scss" scoped>
@import "../assets/vars.scss";

.dark {
#ca-input {
&::file-selector-button {
color: $primary;
background-color: $dark-bg;
}

&:hover:not(:disabled):not([readonly])::file-selector-button {
color: $dark-font-color2;
background-color: $primary;
}
}
}

.form-container {
display: flex;
align-items: center;
Expand Down
Loading