From 85b5c9485827085085dab41be9a77fe28c24776c Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 22 Oct 2023 08:44:10 +0200 Subject: [PATCH 1/4] Add save/restore of API sessions to the database to avoid a forced logout on FTL restarts. This updated the database to version 15 Signed-off-by: DL6ER --- src/FTL.h | 3 - src/api/CMakeLists.txt | 1 + src/api/api.h | 1 + src/api/auth.c | 74 +++---- src/api/auth.h | 65 +++++++ src/database/CMakeLists.txt | 2 + src/database/common.c | 19 +- src/database/common.h | 3 + src/database/session-table.c | 368 +++++++++++++++++++++++++++++++++++ src/database/session-table.h | 24 +++ src/webserver/webserver.c | 3 + 11 files changed, 510 insertions(+), 53 deletions(-) create mode 100644 src/api/auth.h create mode 100644 src/database/session-table.c create mode 100644 src/database/session-table.h diff --git a/src/FTL.h b/src/FTL.h index 7c4151b9..c71d39c5 100644 --- a/src/FTL.h +++ b/src/FTL.h @@ -107,9 +107,6 @@ // Default: 300 (five minutes) #define API_SESSION_EXPIRE 300u -// How many authenticated API clients are allowed simultaneously? [.] -#define API_MAX_CLIENTS 16 - // After how many seconds do we check again if a client can be identified by other means? // (e.g., interface, MAC address, hostname) // Default: 60 (after one minutee) diff --git a/src/api/CMakeLists.txt b/src/api/CMakeLists.txt index c7d75625..e42a2505 100644 --- a/src/api/CMakeLists.txt +++ b/src/api/CMakeLists.txt @@ -15,6 +15,7 @@ set(sources api.h api.c auth.c + auth.h config.c dhcp.c dns.c diff --git a/src/api/api.h b/src/api/api.h index b98450db..3595499f 100644 --- a/src/api/api.h +++ b/src/api/api.h @@ -82,6 +82,7 @@ int api_list(struct ftl_conn *api); int api_group(struct ftl_conn *api); // Auth method +void init_api(void); int check_client_auth(struct ftl_conn *api, const bool is_api); int api_auth(struct ftl_conn *api); void delete_all_sessions(void); diff --git a/src/api/auth.c b/src/api/auth.c index 8516a1dc..86ce17ac 100644 --- a/src/api/auth.c +++ b/src/api/auth.c @@ -9,6 +9,7 @@ * Please see LICENSE file for your rights under this license. */ #include "FTL.h" +#include "api/auth.h" #include "webserver/http-common.h" #include "webserver/json_macros.h" #include "api/api.h" @@ -22,54 +23,11 @@ #include "daemon.h" // sha256_raw_to_hex() #include "config/password.h" +// database session functions +#include "database/session-table.h" -// crypto library -#include -#include -#include -// On 2017-08-27 (after v3.3, before v3.4), nettle changed the type of -// destination from uint_8t* to char* in all base64 and base16 functions -// (armor-signedness branch). This is a breaking change as this is a change in -// signedness causing issues when compiling FTL against older versions of -// nettle. We create this constant here to have a conversion if necessary. -// See https://github.com/gnutls/nettle/commit/f2da403135e2b2f641cf0f8219ad5b72083b7dfd -#if NETTLE_VERSION_MAJOR == 3 && NETTLE_VERSION_MINOR < 4 -#define NETTLE_SIGN (uint8_t*) -#else -#define NETTLE_SIGN -#endif - -// How many bits should the SID and CSRF token use? -#define SID_BITSIZE 128 -#define SID_SIZE BASE64_ENCODE_RAW_LENGTH(SID_BITSIZE/8) - -// SameSite=Strict: Defense against some classes of cross-site request forgery -// (CSRF) attacks. This ensures the session cookie will only be sent in a -// first-party (i.e., Pi-hole) context and NOT be sent along with requests -// initiated by third party websites. -// -// HttpOnly: the cookie cannot be accessed through client side script (if the -// browser supports this flag). As a result, even if a cross-site scripting -// (XSS) flaw exists, and a user accidentally accesses a link that exploits this -// flaw, the browser (primarily Internet Explorer) will not reveal the cookie to -// a third party. -#define FTL_SET_COOKIE "Set-Cookie: sid=%s; SameSite=Strict; Path=/; Max-Age=%u; HttpOnly\r\n" -#define FTL_DELETE_COOKIE "Set-Cookie: sid=deleted; SameSite=Strict; Path=/; Max-Age=-1\r\n" - -static struct { - bool used; - struct { - bool login; - bool mixed; - } tls; - time_t login_at; - time_t valid_until; - char remote_addr[48]; // Large enough for IPv4 and IPv6 addresses, hard-coded in civetweb.h as mg_request_info.remote_addr - char user_agent[128]; - char sid[SID_SIZE]; - char csrf[SID_SIZE]; -} auth_data[API_MAX_CLIENTS] = {{false, {false, false}, 0, 0, {0}, {0}, {0}, {0}}}; +static struct session auth_data[API_MAX_CLIENTS] = {{false, {false, false}, 0, 0, {0}, {0}, {0}, {0}}}; static void add_request_info(struct ftl_conn *api, const char *csrf) { @@ -83,6 +41,12 @@ static void add_request_info(struct ftl_conn *api, const char *csrf) memset((int*)&api->request->is_authenticated, 1, sizeof(api->request->is_authenticated)); } +void init_api(void) +{ + // Restore sessions from database + restore_db_sessions(auth_data); +} + // Is this client connecting from localhost? bool __attribute__((pure)) is_local_api_user(const char *remote_addr) { @@ -257,8 +221,13 @@ int check_client_auth(struct ftl_conn *api, const bool is_api) return send_json_error(api, 500, "internal_error", "Internal server error", NULL); } + // Add CSRF token to request add_request_info(api, auth_data[user_id].csrf); + // Store session in database + update_db_session(&auth_data[user_id]); + + // Debug logging if(config.debug.api.v.b) { char timestr[128]; @@ -347,14 +316,20 @@ static void delete_session(const int user_id) if(user_id < 0 || user_id >= API_MAX_CLIENTS) return; + // Delete session from database + del_db_session(&auth_data[user_id]); + // Zero out this session (also sets valid to false == 0) memset(&auth_data[user_id], 0, sizeof(auth_data[user_id])); } void delete_all_sessions(void) { - for(unsigned int i = 0; i < API_MAX_CLIENTS; i++) - delete_session(i); + // Delete all sessions from database + delete_all_sessions(); + + // Zero out all sessions without looping + memset(auth_data, 0, sizeof(auth_data)); } static int send_api_auth_status(struct ftl_conn *api, const int user_id, const time_t now) @@ -588,6 +563,9 @@ int api_auth(struct ftl_conn *api) generateSID(auth_data[i].sid); generateSID(auth_data[i].csrf); + // Store session in database + add_db_session(&auth_data[i]); + user_id = i; break; } diff --git a/src/api/auth.h b/src/api/auth.h new file mode 100644 index 00000000..f5f11d85 --- /dev/null +++ b/src/api/auth.h @@ -0,0 +1,65 @@ +/* Pi-hole: A black hole for Internet advertisements +* (c) 2023 Pi-hole, LLC (https://pi-hole.net) +* Network-wide ad blocking via your own hardware. +* +* FTL Engine +* API authentication prototypes +* +* This file is copyright under the latest version of the EUPL. +* Please see LICENSE file for your rights under this license. */ + +#ifndef AUTH_H +#define AUTH_H + +// How many authenticated API clients are allowed simultaneously? [.] +#define API_MAX_CLIENTS 16 + +// crypto library +#include +#include +#include + +// On 2017-08-27 (after v3.3, before v3.4), nettle changed the type of +// destination from uint_8t* to char* in all base64 and base16 functions +// (armor-signedness branch). This is a breaking change as this is a change in +// signedness causing issues when compiling FTL against older versions of +// nettle. We create this constant here to have a conversion if necessary. +// See https://github.com/gnutls/nettle/commit/f2da403135e2b2f641cf0f8219ad5b72083b7dfd +#if NETTLE_VERSION_MAJOR == 3 && NETTLE_VERSION_MINOR < 4 +#define NETTLE_SIGN (uint8_t*) +#else +#define NETTLE_SIGN +#endif + +// How many bits should the SID and CSRF token use? +#define SID_BITSIZE 128 +#define SID_SIZE BASE64_ENCODE_RAW_LENGTH(SID_BITSIZE/8) + +// SameSite=Strict: Defense against some classes of cross-site request forgery +// (CSRF) attacks. This ensures the session cookie will only be sent in a +// first-party (i.e., Pi-hole) context and NOT be sent along with requests +// initiated by third party websites. +// +// HttpOnly: the cookie cannot be accessed through client side script (if the +// browser supports this flag). As a result, even if a cross-site scripting +// (XSS) flaw exists, and a user accidentally accesses a link that exploits this +// flaw, the browser (primarily Internet Explorer) will not reveal the cookie to +// a third party. +#define FTL_SET_COOKIE "Set-Cookie: sid=%s; SameSite=Strict; Path=/; Max-Age=%u; HttpOnly\r\n" +#define FTL_DELETE_COOKIE "Set-Cookie: sid=deleted; SameSite=Strict; Path=/; Max-Age=-1\r\n" + +struct session { + bool used; + struct { + bool login; + bool mixed; + } tls; + time_t login_at; + time_t valid_until; + char remote_addr[48]; // Large enough for IPv4 and IPv6 addresses, hard-coded in civetweb.h as mg_request_info.remote_addr + char user_agent[128]; + char sid[SID_SIZE]; + char csrf[SID_SIZE]; +}; + +#endif // AUTH_H \ No newline at end of file diff --git a/src/database/CMakeLists.txt b/src/database/CMakeLists.txt index 8549f0bd..3a16bf9b 100644 --- a/src/database/CMakeLists.txt +++ b/src/database/CMakeLists.txt @@ -33,6 +33,8 @@ set(database_sources network-table.h query-table.c query-table.h + session-table.c + session-table.h sqlite3.h sqlite3-ext.c sqlite3-ext.h diff --git a/src/database/common.c b/src/database/common.c index df002cce..5c5266d3 100644 --- a/src/database/common.c +++ b/src/database/common.c @@ -15,8 +15,6 @@ #include "shmem.h" // struct config #include "config/config.h" -// logging routines -#include "log.h" #include "timers.h" // file_exists() #include "files.h" @@ -30,6 +28,8 @@ #include "events.h" // generate_backtrace() #include "signals.h" +// create_session_table() +#include "database/session-table.h" bool DBdeleteoldqueries = false; static bool DBerror = false; @@ -513,6 +513,21 @@ void db_init(void) dbversion = db_get_int(db, DB_VERSION); } + // Update to version 15 if lower + if(dbversion < 15) + { + // Update to version 15: Add session table + log_info("Updating long-term database to version 15"); + if(!create_session_table(db)) + { + log_info("Session table cannot be created, database not available"); + dbclose(&db); + return; + } + // Get updated version + dbversion = db_get_int(db, DB_VERSION); + } + lock_shm(); import_aliasclients(db); unlock_shm(); diff --git a/src/database/common.h b/src/database/common.h index 6ed9a3a9..d2369185 100644 --- a/src/database/common.h +++ b/src/database/common.h @@ -10,6 +10,9 @@ #ifndef DATABASE_COMMON_H #define DATABASE_COMMON_H +// logging routines +#include "log.h" + #include "sqlite3.h" // Database table "ftl" diff --git a/src/database/session-table.c b/src/database/session-table.c new file mode 100644 index 00000000..c7f2d816 --- /dev/null +++ b/src/database/session-table.c @@ -0,0 +1,368 @@ +/* Pi-hole: A black hole for Internet advertisements +* (c) 2023 Pi-hole, LLC (https://pi-hole.net) +* Network-wide ad blocking via your own hardware. +* +* FTL Engine +* Sessions table database routines +* +* This file is copyright under the latest version of the EUPL. +* Please see LICENSE file for your rights under this license. */ + +#include "FTL.h" +#include "database/session-table.h" +#include "database/common.h" + +bool create_session_table(sqlite3 *db) +{ + // Start transaction of database update + SQL_bool(db, "BEGIN TRANSACTION;"); + + // Create session table + SQL_bool(db, "CREATE TABLE session (id INTEGER PRIMARY KEY, "\ + "login_at TIMESTAMP NOT NULL, "\ + "valid_until TIMESTAMP NOT NULL, "\ + "remote_addr TEXT NOT NULL, "\ + "user_agent TEXT, "\ + "sid TEXT NOT NULL, "\ + "csrf TEXT NOT NULL, "\ + "tls_login BOOL, "\ + "tls_mixed BOOL);"); + + // Update database version to 15 + if(!db_set_FTL_property(db, DB_VERSION, 15)) + { + log_err("create_session_table(): Failed to update database version!"); + return false; + } + + // Finish transaction + SQL_bool(db, "COMMIT"); + + return true; +} + +// Add session to database +bool add_db_session(struct session *sess) +{ + sqlite3 *db = dbopen(false, false); + if(db == NULL) + { + log_warn("Failed to open database in add_db_session()"); + return false; + } + + // Insert session into database + sqlite3_stmt *stmt = NULL; + if(sqlite3_prepare_v2(db, "INSERT INTO session (login_at, valid_until, remote_addr, user_agent, sid, csrf, tls_login, tls_mixed) VALUES (?, ?, ?, ?, ?, ?, ?, ?);", -1, &stmt, 0) != SQLITE_OK) + { + log_err("SQL error in add_db_session(): %s (%d)", + sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Bind values to statement + // 1: login_at + if(sqlite3_bind_int64(stmt, 1, sess->login_at) != SQLITE_OK) + { + log_err("Cannot bind login_at = %ld in add_db_session(): %s (%d)", + (long int)sess->login_at, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + // 2: valid_until + if(sqlite3_bind_int64(stmt, 2, sess->valid_until) != SQLITE_OK) + { + log_err("Cannot bind valid_until = %ld in add_db_session(): %s (%d)", + (long int)sess->valid_until, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + // 3: remote_addr + if(sqlite3_bind_text(stmt, 3, sess->remote_addr, -1, SQLITE_STATIC) != SQLITE_OK) + { + log_err("Cannot bind remote_addr = %s in add_db_session(): %s (%d)", + sess->remote_addr, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + // 4: user_agent + if(sqlite3_bind_text(stmt, 4, sess->user_agent, -1, SQLITE_STATIC) != SQLITE_OK) + { + log_err("Cannot bind user_agent = %s in add_db_session(): %s (%d)", + sess->user_agent, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + // 5: sid + if(sqlite3_bind_text(stmt, 5, sess->sid, -1, SQLITE_STATIC) != SQLITE_OK) + { + log_err("Cannot bind sid = %s in add_db_session(): %s (%d)", + sess->sid, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + // 6: csrf + if(sqlite3_bind_text(stmt, 6, sess->csrf, -1, SQLITE_STATIC) != SQLITE_OK) + { + log_err("Cannot bind csrf = %s in add_db_session(): %s (%d)", + sess->csrf, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + // 7: tls_login + if(sqlite3_bind_int(stmt, 7, sess->tls.login ? 1 : 0) != SQLITE_OK) + { + log_err("Cannot bind tls_login = %d in add_db_session(): %s (%d)", + sess->tls.login ? 1 : 0, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + // 8: tls_mixed + if(sqlite3_bind_int(stmt, 8, sess->tls.mixed ? 1: 0) != SQLITE_OK) + { + log_err("Cannot bind tls_mixed = %d in add_db_session(): %s (%d)", + sess->tls.mixed ? 1 : 0, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Execute statement + if(sqlite3_step(stmt) != SQLITE_DONE) + { + log_err("SQL error in add_db_session(): %s (%d)", + sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Finalize statement + if(sqlite3_finalize(stmt) != SQLITE_OK) + { + log_err("SQL error in add_db_session(): %s (%d)", + sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Close database connection + dbclose(&db); + + return true; +} + +// Update valid_until of session in database (by sid) +bool update_db_session(struct session *sess) +{ + sqlite3 *db = dbopen(false, false); + if(db == NULL) + { + log_warn("Failed to open database in update_db_session()"); + return false; + } + + // Update session in database + sqlite3_stmt *stmt = NULL; + if(sqlite3_prepare_v2(db, "UPDATE session SET valid_until = ?, tls_mixed = ? WHERE sid = ?;", -1, &stmt, 0) != SQLITE_OK) + { + log_err("SQL error in update_db_session(): %s (%d)", + sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Bind values to statement + // 1: valid_until + if(sqlite3_bind_int64(stmt, 1, sess->valid_until) != SQLITE_OK) + { + log_err("Cannot bind valid_until = %ld in update_db_session(): %s (%d)", + (long int)sess->valid_until, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + // 2: tls_mixed + if(sqlite3_bind_int(stmt, 2, sess->tls.mixed ? 1 : 0) != SQLITE_OK) + { + log_err("Cannot bind tls_mixed = %d in update_db_session(): %s (%d)", + sess->tls.mixed ? 1 : 0, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + // 3: sid + if(sqlite3_bind_text(stmt, 3, sess->sid, -1, SQLITE_STATIC) != SQLITE_OK) + { + log_err("Cannot bind sid = %s in update_db_session(): %s (%d)", + sess->sid, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Execute statement + if(sqlite3_step(stmt) != SQLITE_DONE) + { + log_err("SQL error in update_db_session(): %s (%d)", + sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Finalize statement + if(sqlite3_finalize(stmt) != SQLITE_OK) + { + log_err("SQL error in update_db_session(): %s (%d)", + sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Close database connection + dbclose(&db); + + return true; +} + +// Restore all sessions found in the database +bool restore_db_sessions(struct session *sessions) +{ + sqlite3 *db = dbopen(false, false); + if(db == NULL) + { + log_warn("Failed to open database in restore_db_sessions()"); + return false; + } + + // Remove expired sessions from database + SQL_bool(db, "DELETE FROM session WHERE valid_until < strftime('%%s', 'now');"); + + // Get all sessions from database + sqlite3_stmt *stmt = NULL; + if(sqlite3_prepare_v2(db, "SELECT login_at, valid_until, remote_addr, user_agent, sid, csrf, tls_login, tls_mixed FROM session;", -1, &stmt, 0) != SQLITE_OK) + { + log_err("SQL error in restore_db_sessions(): %s (%d)", + sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Iterate over all still valid sessions + unsigned int i = 0; + while(sqlite3_step(stmt) == SQLITE_ROW && i++ < API_MAX_CLIENTS) + { + // Allocate memory for new session + struct session *sess = &sessions[i]; + + // Get values from database + // 1: login_at + sess->login_at = sqlite3_column_int64(stmt, 0); + + // 2: valid_until + sess->valid_until = sqlite3_column_int64(stmt, 1); + + // 3: remote_addr + const char *remote_addr = (const char *)sqlite3_column_text(stmt, 2); + if(remote_addr != NULL) + { + strncpy(sess->remote_addr, remote_addr, sizeof(sess->remote_addr)-1); + sess->remote_addr[sizeof(sess->remote_addr)-1] = '\0'; + } + + // 4: user_agent + const char *user_agent = (const char *)sqlite3_column_text(stmt, 3); + if(user_agent != NULL) + { + strncpy(sess->user_agent, user_agent, sizeof(sess->user_agent)-1); + sess->user_agent[sizeof(sess->user_agent)-1] = '\0'; + } + + // 5: sid + const char *sid = (const char *)sqlite3_column_text(stmt, 4); + if(sid != NULL) + { + strncpy(sess->sid, sid, sizeof(sess->sid)-1); + sess->sid[sizeof(sess->sid)-1] = '\0'; + } + + // 6: csrf + const char *csrf = (const char *)sqlite3_column_text(stmt, 5); + if(csrf != NULL) + { + strncpy(sess->csrf, csrf, sizeof(sess->csrf)-1); + sess->csrf[sizeof(sess->csrf)-1] = '\0'; + } + + // 7: tls_login + sess->tls.login = sqlite3_column_int(stmt, 6) == 1 ? true : false; + + // 8: tls_mixed + sess->tls.mixed = sqlite3_column_int(stmt, 7) == 1 ? true : false; + + // Mark session as used + sess->used = true; + } + + log_info("Restored %u API session%s from the database", + i, i == 1 ? "" : "s"); + + // Finalize statement + if(sqlite3_finalize(stmt) != SQLITE_OK) + { + log_err("SQL error in restore_db_sessions(): %s (%d)", + sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Close database connection + dbclose(&db); + + return true; +} + +// Delete session from database (by sid) +bool del_db_session(struct session *sess) +{ + sqlite3 *db = dbopen(false, false); + if(db == NULL) + { + log_warn("Failed to open database in del_db_session()"); + return false; + } + + // Delete session from database + sqlite3_stmt *stmt = NULL; + if(sqlite3_prepare_v2(db, "DELETE FROM session WHERE sid = ?;", -1, &stmt, 0) != SQLITE_OK) + { + log_err("SQL error in del_db_session(): %s (%d)", + sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Bind values to statement + // 1: sid + if(sqlite3_bind_text(stmt, 1, sess->sid, -1, SQLITE_STATIC) != SQLITE_OK) + { + log_err("Cannot bind sid = %s in del_db_session(): %s (%d)", + sess->sid, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Execute statement + if(sqlite3_step(stmt) != SQLITE_DONE) + { + log_err("SQL error in del_db_session(): %s (%d)", + sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Finalize statement + if(sqlite3_finalize(stmt) != SQLITE_OK) + { + log_err("SQL error in del_db_session(): %s (%d)", + sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Close database connection + dbclose(&db); + + return true; +} + +bool del_all_db_sessions(void) +{ + sqlite3 *db = dbopen(false, false); + if(db == NULL) + { + log_warn("Failed to open database in del_all_db_sessions()"); + return false; + } + + // Delete all sessions from database + SQL_bool(db, "DELETE FROM session;"); + + // Close database connection + dbclose(&db); + + return true; +} diff --git a/src/database/session-table.h b/src/database/session-table.h new file mode 100644 index 00000000..9aceeea3 --- /dev/null +++ b/src/database/session-table.h @@ -0,0 +1,24 @@ +/* Pi-hole: A black hole for Internet advertisements +* (c) 2023 Pi-hole, LLC (https://pi-hole.net) +* Network-wide ad blocking via your own hardware. +* +* FTL Engine +* Sessions table database prototypes +* +* This file is copyright under the latest version of the EUPL. +* Please see LICENSE file for your rights under this license. */ +#ifndef SESSION_TABLE_PRIVATE_H +#define SESSION_TABLE_PRIVATE_H + +#include "sqlite3.h" +// struct session +#include "api/auth.h" + +bool create_session_table(sqlite3 *db); +bool add_db_session(struct session *sess); +bool update_db_session(struct session *sess); +bool restore_db_sessions(struct session *sessions); +bool del_db_session(struct session *sess); +bool del_all_db_sessions(void); + +#endif // SESSION_TABLE_PRIVATE_H \ No newline at end of file diff --git a/src/webserver/webserver.c b/src/webserver/webserver.c index 6e6217e4..59f6d93a 100644 --- a/src/webserver/webserver.c +++ b/src/webserver/webserver.c @@ -422,6 +422,9 @@ void http_init(void) // Get server ports get_server_ports(); + + // Restore sessions from database + init_api(); } static char *append_to_path(char *path, const char *append) From a2a8787239550b5f5dd23daace1e4824db9b6f7c Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 22 Oct 2023 08:51:11 +0200 Subject: [PATCH 2/4] Add new config option webserver.api.session.restore defaulting to true and move existing config option webserver.api.sessionTimeout -> webserver.api.session.timeout Signed-off-by: DL6ER --- src/api/auth.c | 12 ++++++------ src/api/docs/content/specs/config.yaml | 13 ++++++++++--- src/config/config.c | 13 +++++++++---- src/config/config.h | 6 +++++- src/config/legacy_reader.c | 2 +- src/database/session-table.c | 22 ++++++++++++++++++++++ test/pihole.toml | 23 ++++++++++++++--------- test/test_suite.bats | 4 +++- 8 files changed, 70 insertions(+), 25 deletions(-) diff --git a/src/api/auth.c b/src/api/auth.c index 86ce17ac..6c1287c5 100644 --- a/src/api/auth.c +++ b/src/api/auth.c @@ -208,7 +208,7 @@ int check_client_auth(struct ftl_conn *api, const bool is_api) // Update timestamp of this client to extend // the validity of their API authentication - auth_data[user_id].valid_until = now + config.webserver.sessionTimeout.v.ui; + auth_data[user_id].valid_until = now + config.webserver.session.timeout.v.ui; // Set strict_tls permanently to false if the client connected via HTTP auth_data[user_id].tls.mixed |= api->request->is_ssl != auth_data[user_id].tls.login; @@ -216,7 +216,7 @@ int check_client_auth(struct ftl_conn *api, const bool is_api) // Update user cookie if(snprintf(pi_hole_extra_headers, sizeof(pi_hole_extra_headers), FTL_SET_COOKIE, - auth_data[user_id].sid, config.webserver.sessionTimeout.v.ui) < 0) + auth_data[user_id].sid, config.webserver.session.timeout.v.ui) < 0) { return send_json_error(api, 500, "internal_error", "Internal server error", NULL); } @@ -264,7 +264,7 @@ static int get_all_sessions(struct ftl_conn *api, cJSON *json) JSON_ADD_BOOL_TO_OBJECT(tls, "mixed", auth_data[i].tls.mixed); JSON_ADD_ITEM_TO_OBJECT(session, "tls", tls); JSON_ADD_NUMBER_TO_OBJECT(session, "login_at", auth_data[i].login_at); - JSON_ADD_NUMBER_TO_OBJECT(session, "last_active", auth_data[i].valid_until - config.webserver.sessionTimeout.v.ui); + JSON_ADD_NUMBER_TO_OBJECT(session, "last_active", auth_data[i].valid_until - config.webserver.session.timeout.v.ui); JSON_ADD_NUMBER_TO_OBJECT(session, "valid_until", auth_data[i].valid_until); JSON_REF_STR_IN_OBJECT(session, "remote_addr", auth_data[i].remote_addr); JSON_REF_STR_IN_OBJECT(session, "user_agent", auth_data[i].user_agent); @@ -326,7 +326,7 @@ static void delete_session(const int user_id) void delete_all_sessions(void) { // Delete all sessions from database - delete_all_sessions(); + del_all_db_sessions(); // Zero out all sessions without looping memset(auth_data, 0, sizeof(auth_data)); @@ -359,7 +359,7 @@ static int send_api_auth_status(struct ftl_conn *api, const int user_id, const t // Ten minutes validity if(snprintf(pi_hole_extra_headers, sizeof(pi_hole_extra_headers), FTL_SET_COOKIE, - auth_data[user_id].sid, config.webserver.sessionTimeout.d.ui) < 0) + auth_data[user_id].sid, config.webserver.session.timeout.d.ui) < 0) { return send_json_error(api, 500, "internal_error", "Internal server error", NULL); } @@ -540,7 +540,7 @@ int api_auth(struct ftl_conn *api) auth_data[i].used = true; // Set validitiy to now + timeout auth_data[i].login_at = now; - auth_data[i].valid_until = now + config.webserver.sessionTimeout.v.ui; + auth_data[i].valid_until = now + config.webserver.session.timeout.v.ui; // Set remote address strncpy(auth_data[i].remote_addr, api->request->remote_addr, sizeof(auth_data[i].remote_addr)); auth_data[i].remote_addr[sizeof(auth_data[i].remote_addr)-1] = '\0'; diff --git a/src/api/docs/content/specs/config.yaml b/src/api/docs/content/specs/config.yaml index 8cb9104b..f01d7e7f 100644 --- a/src/api/docs/content/specs/config.yaml +++ b/src/api/docs/content/specs/config.yaml @@ -351,8 +351,13 @@ components: type: string port: type: string - sessionTimeout: - type: integer + session: + type: object + properties: + timeout: + type: integer + restore: + type: boolean tls: type: object properties: @@ -644,7 +649,9 @@ components: domain: pi.hole acl: "+0.0.0.0/0,::/0" port: 80,[::]:80 - sessionTimeout: 300 + session: + timeout: 300 + restore: true tls: rev_proxy: false cert: "/etc/pihole/tls.pem" diff --git a/src/config/config.c b/src/config/config.c index ac9aa336..ea84af52 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -847,10 +847,15 @@ void initConfig(struct config *conf) conf->webserver.tls.cert.t = CONF_STRING; conf->webserver.tls.cert.d.s = (char*)"/etc/pihole/tls.pem"; - conf->webserver.sessionTimeout.k = "webserver.sessionTimeout"; - conf->webserver.sessionTimeout.h = "Session timeout in seconds. If a session is inactive for more than this time, it will be terminated. Sessions are continuously refreshed by the web interface, preventing sessions from timing out while the web interface is open.\n This option may also be used to make logins persistent for long times, e.g. 86400 seconds (24 hours), 604800 seconds (7 days) or 2592000 seconds (30 days). Note that the total number of concurrent sessions is limited so setting this value too high may result in users being rejected and unable to log in if there are already too many sessions active."; - conf->webserver.sessionTimeout.t = CONF_UINT; - conf->webserver.sessionTimeout.d.ui = 300u; + conf->webserver.session.timeout.k = "webserver.session.timeout"; + conf->webserver.session.timeout.h = "Session timeout in seconds. If a session is inactive for more than this time, it will be terminated. Sessions are continuously refreshed by the web interface, preventing sessions from timing out while the web interface is open.\n This option may also be used to make logins persistent for long times, e.g. 86400 seconds (24 hours), 604800 seconds (7 days) or 2592000 seconds (30 days). Note that the total number of concurrent sessions is limited so setting this value too high may result in users being rejected and unable to log in if there are already too many sessions active."; + conf->webserver.session.timeout.t = CONF_UINT; + conf->webserver.session.timeout.d.ui = 300u; + + conf->webserver.session.restore.k = "webserver.session.restore"; + conf->webserver.session.restore.h = "Should Pi-hole backup and restore sessions from the database? This is useful if you want to keep your sessions after a restart of the web interface."; + conf->webserver.session.restore.t = CONF_BOOL; + conf->webserver.session.restore.d.b = true; // sub-struct paths conf->webserver.paths.webroot.k = "webserver.paths.webroot"; diff --git a/src/config/config.h b/src/config/config.h index 86cca9e2..f5e92c24 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -108,6 +108,7 @@ struct enum_options { // When new config items are added, the following places need to be updated: // - src/config/config.c: New default item // - test/pihole.toml: Add the new item to the test config file +// - api/docs/content/specs/config.yml: Add the new item to the API documentation struct config { struct { struct conf_item upstreams; @@ -205,7 +206,10 @@ struct config { struct conf_item domain; struct conf_item acl; struct conf_item port; - struct conf_item sessionTimeout; + struct { + struct conf_item timeout; + struct conf_item restore; + } session; struct { struct conf_item rev_proxy; struct conf_item cert; diff --git a/src/config/legacy_reader.c b/src/config/legacy_reader.c index 203d7248..bf4600f8 100644 --- a/src/config/legacy_reader.c +++ b/src/config/legacy_reader.c @@ -314,7 +314,7 @@ const char *readFTLlegacy(struct config *conf) value = 0; if(buffer != NULL && sscanf(buffer, "%i", &value) && value > 0) - conf->webserver.sessionTimeout.v.ui = value; + conf->webserver.session.timeout.v.ui = value; // API_PRETTY_JSON // defaults to: false diff --git a/src/database/session-table.c b/src/database/session-table.c index c7f2d816..71e20568 100644 --- a/src/database/session-table.c +++ b/src/database/session-table.c @@ -11,6 +11,7 @@ #include "FTL.h" #include "database/session-table.h" #include "database/common.h" +#include "config/config.h" bool create_session_table(sqlite3 *db) { @@ -44,6 +45,12 @@ bool create_session_table(sqlite3 *db) // Add session to database bool add_db_session(struct session *sess) { + if(!config.webserver.session.restore.v.b) + { + log_debug(DEBUG_API, "Session restore is disabled, not adding session to database"); + return true; + } + sqlite3 *db = dbopen(false, false); if(db == NULL) { @@ -143,6 +150,9 @@ bool add_db_session(struct session *sess) // Update valid_until of session in database (by sid) bool update_db_session(struct session *sess) { + if(!config.webserver.session.restore.v.b) + return true; + sqlite3 *db = dbopen(false, false); if(db == NULL) { @@ -207,6 +217,12 @@ bool update_db_session(struct session *sess) // Restore all sessions found in the database bool restore_db_sessions(struct session *sessions) { + if(!config.webserver.session.restore.v.b) + { + log_debug(DEBUG_API, "Session restore is disabled, not restoring sessions from database"); + return true; + } + sqlite3 *db = dbopen(false, false); if(db == NULL) { @@ -302,6 +318,9 @@ bool restore_db_sessions(struct session *sessions) // Delete session from database (by sid) bool del_db_session(struct session *sess) { + if(!config.webserver.session.restore.v.b) + return true; + sqlite3 *db = dbopen(false, false); if(db == NULL) { @@ -351,6 +370,9 @@ bool del_db_session(struct session *sess) bool del_all_db_sessions(void) { + if(!config.webserver.session.restore.v.b) + return true; + sqlite3 *db = dbopen(false, false); if(db == NULL) { diff --git a/test/pihole.toml b/test/pihole.toml index b1289a18..424177d9 100644 --- a/test/pihole.toml +++ b/test/pihole.toml @@ -522,15 +522,20 @@ # comma-separated list of <[ip_address:]port> port = "80,[::]:80,443s" - # Session timeout in seconds. If a session is inactive for more than this time, it will - # be terminated. Sessions are continuously refreshed by the web interface, preventing - # sessions from timing out while the web interface is open. - # This option may also be used to make logins persistent for long times, e.g. 86400 - # seconds (24 hours), 604800 seconds (7 days) or 2592000 seconds (30 days). Note that - # the total number of concurrent sessions is limited so setting this value too high - # may result in users being rejected and unable to log in if there are already too - # many sessions active. - sessionTimeout = 300 + [webserver.session] + # Session timeout in seconds. If a session is inactive for more than this time, it will + # be terminated. Sessions are continuously refreshed by the web interface, preventing + # sessions from timing out while the web interface is open. + # This option may also be used to make logins persistent for long times, e.g. 86400 + # seconds (24 hours), 604800 seconds (7 days) or 2592000 seconds (30 days). Note that + # the total number of concurrent sessions is limited so setting this value too high + # may result in users being rejected and unable to log in if there are already too + # many sessions active. + timeout = 300 + + # Should Pi-hole backup and restore sessions from the database? This is useful if you + # want to keep your sessions after a restart of the web interface. + restore = true [webserver.tls] # Is Pi-hole running behind a reverse proxy? If yes, Pi-hole will not consider diff --git a/test/test_suite.bats b/test/test_suite.bats index b122093a..7da11596 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -453,7 +453,7 @@ [[ "${lines[@]}" == *"CREATE TABLE IF NOT EXISTS \"network\" (id INTEGER PRIMARY KEY NOT NULL, hwaddr TEXT UNIQUE NOT NULL, interface TEXT NOT NULL, firstSeen INTEGER NOT NULL, lastQuery INTEGER NOT NULL, numQueries INTEGER NOT NULL, macVendor TEXT, aliasclient_id INTEGER);"* ]] [[ "${lines[@]}" == *"CREATE TABLE IF NOT EXISTS \"network_addresses\" (network_id INTEGER NOT NULL, ip TEXT UNIQUE NOT NULL, lastSeen INTEGER NOT NULL DEFAULT (cast(strftime('%s', 'now') as int)), name TEXT, nameUpdated INTEGER, FOREIGN KEY(network_id) REFERENCES network(id));"* ]] [[ "${lines[@]}" == *"CREATE TABLE aliasclient (id INTEGER PRIMARY KEY NOT NULL, name TEXT NOT NULL, comment TEXT);"* ]] - [[ "${lines[@]}" == *"INSERT INTO ftl VALUES(0,14,'Database version');"* ]] # Expecting FTL database version 14 + [[ "${lines[@]}" == *"INSERT INTO ftl VALUES(0,15,'Database version');"* ]] # Expecting FTL database version 14 # vvv This has been added in version 10 vvv [[ "${lines[@]}" == *"CREATE VIEW queries AS SELECT id, timestamp, type, status, CASE typeof(domain) WHEN 'integer' THEN (SELECT domain FROM domain_by_id d WHERE d.id = q.domain) ELSE domain END domain,CASE typeof(client) WHEN 'integer' THEN (SELECT ip FROM client_by_id c WHERE c.id = q.client) ELSE client END client,CASE typeof(forward) WHEN 'integer' THEN (SELECT forward FROM forward_by_id f WHERE f.id = q.forward) ELSE forward END forward,CASE typeof(additional_info) WHEN 'integer' THEN (SELECT content FROM addinfo_by_id a WHERE a.id = q.additional_info) ELSE additional_info END additional_info, reply_type, reply_time, dnssec, regex_id FROM query_storage q;"* ]] [[ "${lines[@]}" == *"CREATE TABLE domain_by_id (id INTEGER PRIMARY KEY, domain TEXT NOT NULL);"* ]] @@ -464,6 +464,8 @@ # vvv This has been added in version 11 vvv [[ "${lines[@]}" == *"CREATE TABLE addinfo_by_id (id INTEGER PRIMARY KEY, type INTEGER NOT NULL, content NOT NULL);"* ]] [[ "${lines[@]}" == *"CREATE UNIQUE INDEX addinfo_by_id_idx ON addinfo_by_id(type,content);"* ]] + # vvv This has been added in version 15 vvv + [[ "${lines[@]}" == *"CREATE TABLE session (id INTEGER PRIMARY KEY, login_at TIMESTAMP NOT NULL, valid_until TIMESTAMP NOT NULL, remote_addr TEXT NOT NULL, user_agent TEXT, sid TEXT NOT NULL, csrf TEXT NOT NULL, tls_login BOOL, tls_mixed BOOL);"* ]] } @test "Ownership, permissions and type of pihole-FTL.db correct" { From f9a69a06ace8ada4250ac1be72538fdd638a4f6f Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 22 Oct 2023 23:32:11 +0200 Subject: [PATCH 3/4] Store sessions only momentarily in between FTL restarts Signed-off-by: DL6ER --- src/api/api.h | 1 + src/api/auth.c | 18 +-- src/daemon.c | 5 + src/database/session-table.c | 306 ++++++++++++----------------------- src/database/session-table.h | 7 +- 5 files changed, 114 insertions(+), 223 deletions(-) diff --git a/src/api/api.h b/src/api/api.h index 3595499f..981cf60f 100644 --- a/src/api/api.h +++ b/src/api/api.h @@ -83,6 +83,7 @@ int api_group(struct ftl_conn *api); // Auth method void init_api(void); +void free_api(void); int check_client_auth(struct ftl_conn *api, const bool is_api); int api_auth(struct ftl_conn *api); void delete_all_sessions(void); diff --git a/src/api/auth.c b/src/api/auth.c index 6c1287c5..3398e163 100644 --- a/src/api/auth.c +++ b/src/api/auth.c @@ -47,6 +47,12 @@ void init_api(void) restore_db_sessions(auth_data); } +void free_api(void) +{ + // Store sessions in database + backup_db_sessions(auth_data); +} + // Is this client connecting from localhost? bool __attribute__((pure)) is_local_api_user(const char *remote_addr) { @@ -224,9 +230,6 @@ int check_client_auth(struct ftl_conn *api, const bool is_api) // Add CSRF token to request add_request_info(api, auth_data[user_id].csrf); - // Store session in database - update_db_session(&auth_data[user_id]); - // Debug logging if(config.debug.api.v.b) { @@ -316,18 +319,12 @@ static void delete_session(const int user_id) if(user_id < 0 || user_id >= API_MAX_CLIENTS) return; - // Delete session from database - del_db_session(&auth_data[user_id]); - // Zero out this session (also sets valid to false == 0) memset(&auth_data[user_id], 0, sizeof(auth_data[user_id])); } void delete_all_sessions(void) { - // Delete all sessions from database - del_all_db_sessions(); - // Zero out all sessions without looping memset(auth_data, 0, sizeof(auth_data)); } @@ -563,9 +560,6 @@ int api_auth(struct ftl_conn *api) generateSID(auth_data[i].sid); generateSID(auth_data[i].csrf); - // Store session in database - add_db_session(&auth_data[i]); - user_id = i; break; } diff --git a/src/daemon.c b/src/daemon.c index 56914fdf..64e8834e 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -33,6 +33,8 @@ #include "database/query-table.h" // http_terminate() #include "webserver/webserver.h" +// free_api() +#include "api/api.h" pthread_t threads[THREADS_MAX] = { 0 }; bool resolver_ready = false; @@ -332,6 +334,9 @@ void cleanup(const int ret) // Free regex filter memory free_regex(); + // Terminate API + free_api(); + // Terminate HTTP server (if running) http_terminate(); diff --git a/src/database/session-table.c b/src/database/session-table.c index 71e20568..2c157941 100644 --- a/src/database/session-table.c +++ b/src/database/session-table.c @@ -42,19 +42,19 @@ bool create_session_table(sqlite3 *db) return true; } -// Add session to database -bool add_db_session(struct session *sess) +// Store all session in database +bool backup_db_sessions(struct session *sessions) { if(!config.webserver.session.restore.v.b) { - log_debug(DEBUG_API, "Session restore is disabled, not adding session to database"); + log_debug(DEBUG_API, "Session restore is disabled, not adding sessions to database"); return true; } sqlite3 *db = dbopen(false, false); if(db == NULL) { - log_warn("Failed to open database in add_db_session()"); + log_warn("Failed to open database in backup_db_sessions()"); return false; } @@ -62,151 +62,116 @@ bool add_db_session(struct session *sess) sqlite3_stmt *stmt = NULL; if(sqlite3_prepare_v2(db, "INSERT INTO session (login_at, valid_until, remote_addr, user_agent, sid, csrf, tls_login, tls_mixed) VALUES (?, ?, ?, ?, ?, ?, ?, ?);", -1, &stmt, 0) != SQLITE_OK) { - log_err("SQL error in add_db_session(): %s (%d)", + log_err("SQL error in backup_db_sessions(): %s (%d)", sqlite3_errmsg(db), sqlite3_errcode(db)); return false; } - // Bind values to statement - // 1: login_at - if(sqlite3_bind_int64(stmt, 1, sess->login_at) != SQLITE_OK) + unsigned int api_sessions = 0; + for(unsigned int i = 0; i < API_MAX_CLIENTS; i++) { - log_err("Cannot bind login_at = %ld in add_db_session(): %s (%d)", - (long int)sess->login_at, sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } - // 2: valid_until - if(sqlite3_bind_int64(stmt, 2, sess->valid_until) != SQLITE_OK) - { - log_err("Cannot bind valid_until = %ld in add_db_session(): %s (%d)", - (long int)sess->valid_until, sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } - // 3: remote_addr - if(sqlite3_bind_text(stmt, 3, sess->remote_addr, -1, SQLITE_STATIC) != SQLITE_OK) - { - log_err("Cannot bind remote_addr = %s in add_db_session(): %s (%d)", - sess->remote_addr, sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } - // 4: user_agent - if(sqlite3_bind_text(stmt, 4, sess->user_agent, -1, SQLITE_STATIC) != SQLITE_OK) - { - log_err("Cannot bind user_agent = %s in add_db_session(): %s (%d)", - sess->user_agent, sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } - // 5: sid - if(sqlite3_bind_text(stmt, 5, sess->sid, -1, SQLITE_STATIC) != SQLITE_OK) - { - log_err("Cannot bind sid = %s in add_db_session(): %s (%d)", - sess->sid, sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } - // 6: csrf - if(sqlite3_bind_text(stmt, 6, sess->csrf, -1, SQLITE_STATIC) != SQLITE_OK) - { - log_err("Cannot bind csrf = %s in add_db_session(): %s (%d)", - sess->csrf, sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } - // 7: tls_login - if(sqlite3_bind_int(stmt, 7, sess->tls.login ? 1 : 0) != SQLITE_OK) - { - log_err("Cannot bind tls_login = %d in add_db_session(): %s (%d)", - sess->tls.login ? 1 : 0, sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } - // 8: tls_mixed - if(sqlite3_bind_int(stmt, 8, sess->tls.mixed ? 1: 0) != SQLITE_OK) - { - log_err("Cannot bind tls_mixed = %d in add_db_session(): %s (%d)", - sess->tls.mixed ? 1 : 0, sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } + // Get session + struct session *sess = &sessions[i]; - // Execute statement - if(sqlite3_step(stmt) != SQLITE_DONE) - { - log_err("SQL error in add_db_session(): %s (%d)", - sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; + // Skip unused sessions + if(!sess->used) + continue; + + // Bind values to statement + // 1: login_at + if(sqlite3_bind_int64(stmt, 1, sess->login_at) != SQLITE_OK) + { + log_err("Cannot bind login_at = %ld in backup_db_sessions(): %s (%d)", + (long int)sess->login_at, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + // 2: valid_until + if(sqlite3_bind_int64(stmt, 2, sess->valid_until) != SQLITE_OK) + { + log_err("Cannot bind valid_until = %ld in backup_db_sessions(): %s (%d)", + (long int)sess->valid_until, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + // 3: remote_addr + if(sqlite3_bind_text(stmt, 3, sess->remote_addr, -1, SQLITE_STATIC) != SQLITE_OK) + { + log_err("Cannot bind remote_addr = %s in backup_db_sessions(): %s (%d)", + sess->remote_addr, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + // 4: user_agent + if(sqlite3_bind_text(stmt, 4, sess->user_agent, -1, SQLITE_STATIC) != SQLITE_OK) + { + log_err("Cannot bind user_agent = %s in backup_db_sessions(): %s (%d)", + sess->user_agent, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + // 5: sid + if(sqlite3_bind_text(stmt, 5, sess->sid, -1, SQLITE_STATIC) != SQLITE_OK) + { + log_err("Cannot bind sid = %s in backup_db_sessions(): %s (%d)", + sess->sid, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + // 6: csrf + if(sqlite3_bind_text(stmt, 6, sess->csrf, -1, SQLITE_STATIC) != SQLITE_OK) + { + log_err("Cannot bind csrf = %s in backup_db_sessions(): %s (%d)", + sess->csrf, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + // 7: tls_login + if(sqlite3_bind_int(stmt, 7, sess->tls.login ? 1 : 0) != SQLITE_OK) + { + log_err("Cannot bind tls_login = %d in backup_db_sessions(): %s (%d)", + sess->tls.login ? 1 : 0, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + // 8: tls_mixed + if(sqlite3_bind_int(stmt, 8, sess->tls.mixed ? 1: 0) != SQLITE_OK) + { + log_err("Cannot bind tls_mixed = %d in backup_db_sessions(): %s (%d)", + sess->tls.mixed ? 1 : 0, sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Execute statement + if(sqlite3_step(stmt) != SQLITE_DONE) + { + log_err("SQL error in backup_db_sessions(): %s (%d)", + sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Clear bindings + if(sqlite3_clear_bindings(stmt) != SQLITE_OK) + { + log_err("SQL error in backup_db_sessions(): %s (%d)", + sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + // Reset statement + if(sqlite3_reset(stmt) != SQLITE_OK) + { + log_err("SQL error in backup_db_sessions(): %s (%d)", + sqlite3_errmsg(db), sqlite3_errcode(db)); + return false; + } + + api_sessions++; } // Finalize statement if(sqlite3_finalize(stmt) != SQLITE_OK) { - log_err("SQL error in add_db_session(): %s (%d)", + log_err("SQL error in backup_db_sessions(): %s (%d)", sqlite3_errmsg(db), sqlite3_errcode(db)); return false; } - // Close database connection - dbclose(&db); - - return true; -} - -// Update valid_until of session in database (by sid) -bool update_db_session(struct session *sess) -{ - if(!config.webserver.session.restore.v.b) - return true; - - sqlite3 *db = dbopen(false, false); - if(db == NULL) - { - log_warn("Failed to open database in update_db_session()"); - return false; - } - - // Update session in database - sqlite3_stmt *stmt = NULL; - if(sqlite3_prepare_v2(db, "UPDATE session SET valid_until = ?, tls_mixed = ? WHERE sid = ?;", -1, &stmt, 0) != SQLITE_OK) - { - log_err("SQL error in update_db_session(): %s (%d)", - sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } - - // Bind values to statement - // 1: valid_until - if(sqlite3_bind_int64(stmt, 1, sess->valid_until) != SQLITE_OK) - { - log_err("Cannot bind valid_until = %ld in update_db_session(): %s (%d)", - (long int)sess->valid_until, sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } - // 2: tls_mixed - if(sqlite3_bind_int(stmt, 2, sess->tls.mixed ? 1 : 0) != SQLITE_OK) - { - log_err("Cannot bind tls_mixed = %d in update_db_session(): %s (%d)", - sess->tls.mixed ? 1 : 0, sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } - // 3: sid - if(sqlite3_bind_text(stmt, 3, sess->sid, -1, SQLITE_STATIC) != SQLITE_OK) - { - log_err("Cannot bind sid = %s in update_db_session(): %s (%d)", - sess->sid, sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } - - // Execute statement - if(sqlite3_step(stmt) != SQLITE_DONE) - { - log_err("SQL error in update_db_session(): %s (%d)", - sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } - - // Finalize statement - if(sqlite3_finalize(stmt) != SQLITE_OK) - { - log_err("SQL error in update_db_session(): %s (%d)", - sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } + log_info("Stored %u API session%s in the database", + api_sessions, api_sessions == 1 ? "" : "s"); // Close database connection dbclose(&db); @@ -309,78 +274,7 @@ bool restore_db_sessions(struct session *sessions) return false; } - // Close database connection - dbclose(&db); - - return true; -} - -// Delete session from database (by sid) -bool del_db_session(struct session *sess) -{ - if(!config.webserver.session.restore.v.b) - return true; - - sqlite3 *db = dbopen(false, false); - if(db == NULL) - { - log_warn("Failed to open database in del_db_session()"); - return false; - } - - // Delete session from database - sqlite3_stmt *stmt = NULL; - if(sqlite3_prepare_v2(db, "DELETE FROM session WHERE sid = ?;", -1, &stmt, 0) != SQLITE_OK) - { - log_err("SQL error in del_db_session(): %s (%d)", - sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } - - // Bind values to statement - // 1: sid - if(sqlite3_bind_text(stmt, 1, sess->sid, -1, SQLITE_STATIC) != SQLITE_OK) - { - log_err("Cannot bind sid = %s in del_db_session(): %s (%d)", - sess->sid, sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } - - // Execute statement - if(sqlite3_step(stmt) != SQLITE_DONE) - { - log_err("SQL error in del_db_session(): %s (%d)", - sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } - - // Finalize statement - if(sqlite3_finalize(stmt) != SQLITE_OK) - { - log_err("SQL error in del_db_session(): %s (%d)", - sqlite3_errmsg(db), sqlite3_errcode(db)); - return false; - } - - // Close database connection - dbclose(&db); - - return true; -} - -bool del_all_db_sessions(void) -{ - if(!config.webserver.session.restore.v.b) - return true; - - sqlite3 *db = dbopen(false, false); - if(db == NULL) - { - log_warn("Failed to open database in del_all_db_sessions()"); - return false; - } - - // Delete all sessions from database + // Delete all sessions from database after restoring them SQL_bool(db, "DELETE FROM session;"); // Close database connection diff --git a/src/database/session-table.h b/src/database/session-table.h index 9aceeea3..a2d48118 100644 --- a/src/database/session-table.h +++ b/src/database/session-table.h @@ -15,10 +15,7 @@ #include "api/auth.h" bool create_session_table(sqlite3 *db); -bool add_db_session(struct session *sess); -bool update_db_session(struct session *sess); +bool backup_db_sessions(struct session *sessions); bool restore_db_sessions(struct session *sessions); -bool del_db_session(struct session *sess); -bool del_all_db_sessions(void); -#endif // SESSION_TABLE_PRIVATE_H \ No newline at end of file +#endif // SESSION_TABLE_PRIVATE_H From 88db8bb144ad831ad2c7ce97b0c6cb4fcd732271 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 22 Oct 2023 23:33:34 +0200 Subject: [PATCH 4/4] Use secure_delete mode to ensure the backed up sessions are overwritten with zeros after restore Signed-off-by: DL6ER --- src/database/session-table.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/database/session-table.c b/src/database/session-table.c index 2c157941..51b8e24b 100644 --- a/src/database/session-table.c +++ b/src/database/session-table.c @@ -275,6 +275,10 @@ bool restore_db_sessions(struct session *sessions) } // Delete all sessions from database after restoring them + // We use secure_delete to make sure the sessions are really gone + // In this mode, SQLite overwrites the deleted content with zeros + // (https://www.sqlite.org/pragma.html#pragma_secure_delete) + SQL_bool(db, "PRAGMA secure_delete = ON;"); SQL_bool(db, "DELETE FROM session;"); // Close database connection