security: protect local data at rest (issue #146)
All checks were successful
CI Code / Check spelling (pull_request) Successful in 15s
CI Code / Check coding style (pull_request) Successful in 26s
CI Code / Linux (debian) (pull_request) Successful in 5m3s
CI Code / Linux (arch) (pull_request) Successful in 6m25s
CI Code / Linux (ubuntu) (pull_request) Successful in 8m1s
CI Code / Code Coverage (pull_request) Successful in 9m27s
All checks were successful
CI Code / Check spelling (pull_request) Successful in 15s
CI Code / Check coding style (pull_request) Successful in 26s
CI Code / Linux (debian) (pull_request) Successful in 5m3s
CI Code / Linux (arch) (pull_request) Successful in 6m25s
CI Code / Linux (ubuntu) (pull_request) Successful in 8m1s
CI Code / Code Coverage (pull_request) Successful in 9m27s
T03 — keep secrets out of profanity.log: - new redact_secrets() helper (common.c) masks the content of SASL auth/response/challenge/success and <password> elements; applied in the libstrophe logger (_xmpp_file_logger) and the stderr-to-log bridge (REQ-DAR-03, REQ-LOG-06) - _add_to_db() no longer logs decrypted message bodies or the full INSERT statement (REQ-DAR-03) T07 — owner-only permissions on key material and history (REQ-AUTH-01): - chmod 0600 on chatlog.db after open (journal/WAL files inherit) - chmod 0600 on OTR keys.txt and fingerprints.txt after every write - parent dirs are already 0700 (create_dir); this is defense in depth T08 — integrity before trust: - PRAGMA quick_check(1) gates every chatlog.db open; on failure the DB stays closed, the user is warned once in the console and the session continues without history (REQ-RES-02) - OMEMO aesgcm downloads decrypt into a 0600 tempfile next to the target and rename it into place only after the GCM tag verifies; the open-command hook no longer runs on failed decryption (REQ-CRY-06) Tests: redact_secrets unit tests; AES-256-GCM roundtrip and tampered-tag/ciphertext unit tests (crypto.c now linked into unittests under BUILD_OMEMO); functional test planting a corrupt chatlog.db and asserting graceful degradation. Closes #146
This commit is contained in:
26
src/common.c
26
src/common.c
@@ -455,6 +455,32 @@ str_xml_sanitize(const char* const str)
|
||||
return g_string_free(sanitized, FALSE);
|
||||
}
|
||||
|
||||
gchar*
|
||||
redact_secrets(const char* const str)
|
||||
{
|
||||
if (str == NULL) {
|
||||
return NULL;
|
||||
}
|
||||
|
||||
// SASL exchanges and <password> elements carry credentials — strip their content before logging
|
||||
static gsize init = 0;
|
||||
static GRegex* secret_regex = NULL;
|
||||
if (g_once_init_enter(&init)) {
|
||||
secret_regex = g_regex_new(
|
||||
"(<(?:auth|response|challenge|success|password|digest)\\b[^>]*>)[^<]+(</(?:auth|response|challenge|success|password|digest)>)",
|
||||
0, 0, NULL);
|
||||
g_once_init_leave(&init, 1);
|
||||
}
|
||||
|
||||
if (secret_regex == NULL) {
|
||||
return g_strdup(str);
|
||||
}
|
||||
|
||||
auto_gchar gchar* valid = g_utf8_make_valid(str, -1); // invalid UTF-8 would make the regex fail open
|
||||
gchar* redacted = g_regex_replace(secret_regex, valid, -1, 0, "\\1[REDACTED]\\2", 0, NULL);
|
||||
return redacted ? redacted : g_steal_pointer(&valid);
|
||||
}
|
||||
|
||||
char*
|
||||
release_get_latest(void)
|
||||
{
|
||||
|
||||
@@ -161,6 +161,7 @@ gboolean strtoi_range(const char* str, int* saveptr, int min, int max, char** er
|
||||
gsize g_diff_to_gsize(const void* end, const void* start);
|
||||
int utf8_display_len(const char* const str);
|
||||
gchar* str_xml_sanitize(const char* const str);
|
||||
gchar* redact_secrets(const char* const str);
|
||||
|
||||
gboolean string_matches_one_of(const char* what, const char* is, gboolean is_can_be_null, const char* first, ...) __attribute__((sentinel));
|
||||
gboolean valid_tls_policy_option(const char* is);
|
||||
|
||||
@@ -39,6 +39,7 @@
|
||||
#include <sys/statvfs.h>
|
||||
#include <sqlite3.h>
|
||||
#include <glib.h>
|
||||
#include <glib/gstdio.h>
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
@@ -124,6 +125,15 @@ _get_db_filename(ProfAccount* account)
|
||||
return files_file_in_account_data_path(DIR_DATABASE, account->jid, "chatlog.db");
|
||||
}
|
||||
|
||||
static int
|
||||
_quick_check_cb(void* intact, int argc, char** argv, char** column_names)
|
||||
{
|
||||
if (argc > 0 && argv[0] && strcmp(argv[0], "ok") == 0) {
|
||||
*(gboolean*)intact = TRUE;
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
static gboolean
|
||||
_sqlite_init(ProfAccount* account)
|
||||
{
|
||||
@@ -149,6 +159,20 @@ _sqlite_init(ProfAccount* account)
|
||||
return FALSE;
|
||||
}
|
||||
|
||||
g_chmod(filename, S_IRUSR | S_IWUSR); // history holds plaintext; journal/WAL files inherit these perms
|
||||
|
||||
// catch corruption before running queries or migrations against the file
|
||||
char* check_err = NULL;
|
||||
gboolean intact = FALSE;
|
||||
ret = sqlite3_exec(g_chatlog_database, "PRAGMA quick_check(1);", _quick_check_cb, &intact, &check_err);
|
||||
if (ret != SQLITE_OK || !intact) {
|
||||
log_error("Chat history database failed integrity check (%s): %s", filename,
|
||||
check_err ? check_err : "quick_check did not return 'ok'");
|
||||
sqlite3_free(check_err);
|
||||
_db_teardown("_sqlite_init(quick_check)");
|
||||
return FALSE;
|
||||
}
|
||||
|
||||
char* err_msg = NULL;
|
||||
|
||||
int db_version = _get_db_version();
|
||||
@@ -691,7 +715,7 @@ _add_to_db(ProfMessage* message, const char* type, const Jid* const from_jid, co
|
||||
original_message_id = tmp ? tmp : original_message_id;
|
||||
|
||||
if (g_strcmp0(from_jid_orig, from_jid->barejid) != 0) {
|
||||
log_error("Mismatch in sender JIDs when trying to do LMC. Corrected message sender: %s. Original message sender: %s. Replace-ID: %s. Message: %s", from_jid->barejid, from_jid_orig, message->replace_id, message->plain);
|
||||
log_error("Mismatch in sender JIDs when trying to do LMC. Corrected message sender: %s. Original message sender: %s. Replace-ID: %s.", from_jid->barejid, from_jid_orig, message->replace_id);
|
||||
cons_show_error("%s sent a message correction with mismatched sender. See log for details.", from_jid->barejid);
|
||||
sqlite3_finalize(lmc_stmt);
|
||||
return;
|
||||
@@ -714,7 +738,7 @@ _add_to_db(ProfMessage* message, const char* type, const Jid* const from_jid, co
|
||||
sqlite3_stmt* stmt;
|
||||
if (_db_prepare_ctx(duplicate_check_query, &stmt, "_add_to_db(duplicate_check)")) {
|
||||
if (sqlite3_step(stmt) == SQLITE_ROW) {
|
||||
log_error("Duplicate stanza-id found for the message. stanza_id: %s; archive_id: %s; sender: %s; content: %s", message->id, message->stanzaid, from_jid->barejid, message->plain);
|
||||
log_error("Duplicate stanza-id found for the message. stanza_id: %s; archive_id: %s; sender: %s", message->id, message->stanzaid, from_jid->barejid);
|
||||
cons_show_error("Got a message with duplicate (server-generated) stanza-id from %s.", from_jid->fulljid);
|
||||
}
|
||||
sqlite3_finalize(stmt);
|
||||
@@ -745,7 +769,7 @@ _add_to_db(ProfMessage* message, const char* type, const Jid* const from_jid, co
|
||||
return;
|
||||
}
|
||||
|
||||
log_debug("Writing to DB. Query: %s", query);
|
||||
log_debug("Writing message to DB (id: %s, stanza_id: %s, type: %s)", message->id, message->stanzaid, type); // no query text: it embeds the plaintext body
|
||||
|
||||
if (SQLITE_OK != sqlite3_exec(g_chatlog_database, query, NULL, 0, &err_msg)) {
|
||||
if (err_msg) {
|
||||
@@ -757,7 +781,7 @@ _add_to_db(ProfMessage* message, const char* type, const Jid* const from_jid, co
|
||||
} else {
|
||||
int inserted_rows_count = sqlite3_changes(g_chatlog_database);
|
||||
if (inserted_rows_count < 1) {
|
||||
log_error("SQLite did not insert message (rows: %d, id: %s, content: %s)", inserted_rows_count, message->id, message->plain);
|
||||
log_error("SQLite did not insert message (rows: %d, id: %s)", inserted_rows_count, message->id);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -83,6 +83,7 @@ sv_ev_login_account_success(char* account_name, gboolean secured)
|
||||
|
||||
if (!log_database_init(account)) {
|
||||
log_error("Failed to initialize database for account: %s", account->jid);
|
||||
cons_show_error("Chat history storage is unavailable for this session, messages will not be saved. See the log for details.");
|
||||
}
|
||||
vcard_user_refresh();
|
||||
avatar_pep_subscribe();
|
||||
|
||||
@@ -286,7 +286,8 @@ log_stderr_handler(void)
|
||||
|
||||
for (int i = 0; i < size; ++i) {
|
||||
if (buf[i] == '\n') {
|
||||
log_msg(stderr_level, "stderr", s->str);
|
||||
auto_gchar gchar* redacted = redact_secrets(s->str); // third-party libs may echo credentials
|
||||
log_msg(stderr_level, "stderr", redacted);
|
||||
g_string_assign(s, "");
|
||||
} else
|
||||
g_string_append_c(s, buf[i]);
|
||||
@@ -294,7 +295,8 @@ log_stderr_handler(void)
|
||||
} while (1);
|
||||
|
||||
if (s->len > 0 && s->str[0] != '\0') {
|
||||
log_msg(stderr_level, "stderr", s->str);
|
||||
auto_gchar gchar* redacted = redact_secrets(s->str);
|
||||
log_msg(stderr_level, "stderr", redacted);
|
||||
g_string_assign(s, "");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14,6 +14,8 @@
|
||||
#include <libotr/message.h>
|
||||
#include <libotr/sm.h>
|
||||
#include <glib.h>
|
||||
#include <glib/gstdio.h>
|
||||
#include <sys/stat.h>
|
||||
|
||||
#include "log.h"
|
||||
#include "chatlog.h"
|
||||
@@ -114,6 +116,8 @@ cb_write_fingerprints(void* opdata)
|
||||
if (err != GPG_ERR_NO_ERROR) {
|
||||
log_error("Failed to write fingerprints file");
|
||||
cons_show_error("Failed to write fingerprints file");
|
||||
} else {
|
||||
g_chmod(fpsfilename, S_IRUSR | S_IWUSR);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -376,6 +380,7 @@ otr_keygen(ProfAccount* account)
|
||||
cons_show_error("Failed to generate private key");
|
||||
return;
|
||||
}
|
||||
g_chmod(keysfilename->str, S_IRUSR | S_IWUSR);
|
||||
log_info("Private key generated");
|
||||
cons_show("");
|
||||
cons_show("Private key generation complete.");
|
||||
@@ -390,6 +395,7 @@ otr_keygen(ProfAccount* account)
|
||||
cons_show_error("Failed to create fingerprints file");
|
||||
return;
|
||||
}
|
||||
g_chmod(fpsfilename->str, S_IRUSR | S_IWUSR);
|
||||
log_info("Fingerprints file created");
|
||||
|
||||
err = otrl_privkey_read(user_state, keysfilename->str);
|
||||
|
||||
@@ -13,10 +13,12 @@
|
||||
#include <stdlib.h>
|
||||
#include <stdio.h>
|
||||
#include <string.h>
|
||||
#include <unistd.h>
|
||||
#include <sys/stat.h>
|
||||
#include <sys/types.h>
|
||||
#include <curl/curl.h>
|
||||
#include <gio/gio.h>
|
||||
#include <glib/gstdio.h>
|
||||
#include <pthread.h>
|
||||
#include <assert.h>
|
||||
#include <errno.h>
|
||||
@@ -62,14 +64,28 @@ aesgcm_file_get(void* userdata)
|
||||
return NULL;
|
||||
}
|
||||
|
||||
// Open the target file for storing the cleartext.
|
||||
auto_FILE FILE* outfh = fopen(aesgcm_dl->filename, "wb");
|
||||
// Decrypt into a temporary file next to the target and rename it into
|
||||
// place only after the GCM tag verified, so tampered or truncated
|
||||
// content never appears at the destination path.
|
||||
auto_gchar gchar* partname = g_strdup_printf("%s.part.XXXXXX", aesgcm_dl->filename);
|
||||
gint outfd = g_mkstemp(partname);
|
||||
if (outfd == -1) {
|
||||
http_print_transfer_update(aesgcm_dl->window, aesgcm_dl->id, THEME_ERROR, ENTRY_ERROR,
|
||||
"Downloading '%s' failed: Unable to open "
|
||||
"output file at '%s' for writing (%s).",
|
||||
https_url, aesgcm_dl->filename,
|
||||
g_strerror(errno));
|
||||
return NULL;
|
||||
}
|
||||
FILE* outfh = fdopen(outfd, "wb");
|
||||
if (outfh == NULL) {
|
||||
http_print_transfer_update(aesgcm_dl->window, aesgcm_dl->id, THEME_ERROR, ENTRY_ERROR,
|
||||
"Downloading '%s' failed: Unable to open "
|
||||
"output file at '%s' for writing (%s).",
|
||||
https_url, aesgcm_dl->filename,
|
||||
g_strerror(errno));
|
||||
close(outfd);
|
||||
remove(partname);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
@@ -90,6 +106,8 @@ aesgcm_file_get(void* userdata)
|
||||
|
||||
ssize_t* p_bytes_received = http_file_get(http_dl);
|
||||
if (!p_bytes_received) {
|
||||
fclose(outfh);
|
||||
remove(partname);
|
||||
return NULL;
|
||||
}
|
||||
ssize_t bytes_received = *p_bytes_received;
|
||||
@@ -102,6 +120,8 @@ aesgcm_file_get(void* userdata)
|
||||
"temporary file at '%s' for reading (%s).",
|
||||
aesgcm_dl->url, tmpname,
|
||||
g_strerror(errno));
|
||||
fclose(outfh);
|
||||
remove(partname);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
@@ -110,20 +130,32 @@ aesgcm_file_get(void* userdata)
|
||||
bytes_received, fragment);
|
||||
fclose(tmpfh);
|
||||
remove(tmpname);
|
||||
fclose(outfh);
|
||||
|
||||
gboolean saved = FALSE;
|
||||
if (crypt_res != GPG_ERR_NO_ERROR) {
|
||||
remove(partname);
|
||||
http_print_transfer_update(aesgcm_dl->window, aesgcm_dl->id, THEME_ERROR, ENTRY_ERROR,
|
||||
"Downloading '%s' failed: Failed to decrypt "
|
||||
"file (%s).",
|
||||
https_url, gcry_strerror(crypt_res));
|
||||
} else if (g_rename(partname, aesgcm_dl->filename) != 0) {
|
||||
remove(partname);
|
||||
http_print_transfer_update(aesgcm_dl->window, aesgcm_dl->id, THEME_ERROR, ENTRY_ERROR,
|
||||
"Downloading '%s' failed: Unable to move "
|
||||
"decrypted file to '%s' (%s).",
|
||||
https_url, aesgcm_dl->filename,
|
||||
g_strerror(errno));
|
||||
} else {
|
||||
saved = TRUE;
|
||||
http_print_transfer_update(aesgcm_dl->window, aesgcm_dl->id, THEME_ONLINE, ENTRY_COMPLETED,
|
||||
"Downloading '%s': done\nSaved to '%s'",
|
||||
aesgcm_dl->url, aesgcm_dl->filename);
|
||||
win_mark_received(aesgcm_dl->window, aesgcm_dl->id);
|
||||
}
|
||||
|
||||
if (aesgcm_dl->cmd_template != NULL) {
|
||||
// never hand an unverified file to the external command
|
||||
if (saved && aesgcm_dl->cmd_template != NULL) {
|
||||
gchar** argv = format_call_external_argv(aesgcm_dl->cmd_template,
|
||||
aesgcm_dl->filename,
|
||||
aesgcm_dl->filename);
|
||||
@@ -140,8 +172,8 @@ aesgcm_file_get(void* userdata)
|
||||
}
|
||||
|
||||
g_strfreev(argv);
|
||||
free(aesgcm_dl->cmd_template);
|
||||
}
|
||||
free(aesgcm_dl->cmd_template);
|
||||
|
||||
free(aesgcm_dl->id);
|
||||
free(aesgcm_dl->filename);
|
||||
|
||||
@@ -1104,7 +1104,8 @@ _xmpp_file_logger(void* const userdata, const xmpp_log_level_t xmpp_level, const
|
||||
break;
|
||||
}
|
||||
|
||||
log_msg(prof_level, area, msg);
|
||||
auto_gchar gchar* redacted = redact_secrets(msg); // raw traffic contains SASL/register credentials
|
||||
log_msg(prof_level, area, redacted);
|
||||
|
||||
if ((g_strcmp0(area, "xmpp") == 0) || (g_strcmp0(area, "conn")) == 0) {
|
||||
sv_ev_xmpp_stanza(msg);
|
||||
|
||||
Reference in New Issue
Block a user