Make saving the proxy data secure, and add some notes to the security document
This commit is contained in:
Richard Hughes
parent
e59ba218c6
commit
2f742a809a
@ -91,3 +91,22 @@ Possible attack vectors:
|
||||
(MaximumPackagesToProcess) for each method, and also limiting the total length
|
||||
of the data.
|
||||
|
||||
* The HTTP and FTP proxies that are sent by the session may contain embedded
|
||||
usernames and passwords. Whilst these are sent in plain text over the system
|
||||
D-Bus (FIXME?), the database is not be readable by users (mode 0640).
|
||||
|
||||
* The matching of UID and session to proxies assumes that the user cannot
|
||||
modify the login UID, and cannot spoof a session in ConsoleKit. If a user were
|
||||
able to change the apparent UID, then this would allow them to use proxy
|
||||
settings set from another user.
|
||||
This is manifested when a graphical application is run using sudo (which you
|
||||
should never do, but I digress) and different proxy settings may be used from
|
||||
the user settings. It also allows applications run using sudo to use the proxy
|
||||
specified by root, which may be different.
|
||||
|
||||
* We save the UID and session to a database to get the proxy state for each
|
||||
transaction. If the user is able to create a large number of different sessions
|
||||
then this will add many entries to the database. Some rate-limiting could be
|
||||
added to ConsoleKit or PackageKit to solve this, but has not been done at this
|
||||
time.
|
||||
|
||||
|
||||
@ -27,29 +27,3 @@ proxy settings and this is passed to the backend. This also allows the daemon to
|
||||
have to correct proxy settings if the daemon times out (as it is designed to do)
|
||||
and is restarted by a client application using DBus system activation.
|
||||
|
||||
Implementation considerations;
|
||||
|
||||
The PID and session should be be cached in PkDbus as these will be used on each transaction.
|
||||
|
||||
Security considerations:
|
||||
The HTTP and FTP proxies that are sent by the session may contain embedded
|
||||
usernames and passwords. Whilst these are sent in plain text over the system
|
||||
DBus (TODO), the database should not be readable by a user as this could be used
|
||||
to discover proxy server passwords.
|
||||
|
||||
The matching of UID and session to proxies assumes that the user cannot modify
|
||||
the login UID, and cannot spoof a session in ConsoleKit. If a user were able to
|
||||
change the apparent UID, then this would allow them to use proxy settings set
|
||||
from another user. This is manifested when a graphical application is run using
|
||||
sudo (which you should never do, but I digress) and different proxy settings may
|
||||
be used from the user settings. It also allows applications run using sudo to
|
||||
use the proxy specified by root, which may be different.
|
||||
|
||||
We save the UID and session to a database to get the proxy state for each
|
||||
transaction. If the user is able to create a large number of different sessions
|
||||
then this will add many entries to the database. Some rate-limiting could be
|
||||
added to ConsoleKit or PackageKit to solve this, but has not been done at this
|
||||
time.
|
||||
|
||||
TODO: need to check for injected SQL in the proxy data before we add freeform text.
|
||||
|
||||
|
||||
@ -95,7 +95,7 @@ pk_dbus_get_pid (PkDbus *dbus, const gchar *sender)
|
||||
g_return_val_if_fail (dbus->priv->proxy_pid != NULL, G_MAXUINT);
|
||||
g_return_val_if_fail (sender != NULL, G_MAXUINT);
|
||||
|
||||
/* get pid from DBus (quite slow) */
|
||||
/* get pid from DBus (quite slow) - TODO: cache this */
|
||||
ret = dbus_g_proxy_call (dbus->priv->proxy_pid,
|
||||
"GetConnectionUnixProcessID", &error,
|
||||
G_TYPE_STRING, sender,
|
||||
|
||||
@ -683,13 +683,12 @@ gboolean
|
||||
pk_transaction_db_set_proxy (PkTransactionDb *tdb, guint uid, const gchar *session,
|
||||
const gchar *proxy_http, const gchar *proxy_ftp)
|
||||
{
|
||||
gchar *error_msg = NULL;
|
||||
gchar *statement = NULL;
|
||||
gchar *timespec = NULL;
|
||||
gchar *proxy_http_tmp = NULL;
|
||||
gchar *proxy_ftp_tmp = NULL;
|
||||
gboolean ret = FALSE;
|
||||
gint rc;
|
||||
sqlite3_stmt *statement = NULL;
|
||||
|
||||
g_return_val_if_fail (PK_IS_TRANSACTION_DB (tdb), FALSE);
|
||||
g_return_val_if_fail (uid != G_MAXUINT, FALSE);
|
||||
@ -703,13 +702,27 @@ pk_transaction_db_set_proxy (PkTransactionDb *tdb, guint uid, const gchar *sessi
|
||||
|
||||
/* any data */
|
||||
if (proxy_http_tmp != NULL || proxy_ftp_tmp != NULL) {
|
||||
statement = g_strdup_printf ("UPDATE proxy SET proxy_http = '%s', proxy_ftp = '%s' WHERE uid = '%i' AND session = '%s'",
|
||||
proxy_http, proxy_ftp, uid, session);
|
||||
egg_debug ("updated proxy for uid:%i and session:%s", uid, session);
|
||||
rc = sqlite3_exec (tdb->priv->db, statement, NULL, NULL, &error_msg);
|
||||
|
||||
/* prepare statement */
|
||||
rc = sqlite3_prepare_v2 (tdb->priv->db,
|
||||
"UPDATE proxy SET proxy_http = ?, proxy_ftp = ? WHERE uid = ? AND session = ?",
|
||||
-1, &statement, NULL);
|
||||
if (rc != SQLITE_OK) {
|
||||
egg_warning ("SQL error: %s", error_msg);
|
||||
sqlite3_free (error_msg);
|
||||
egg_warning ("failed to prepare statement: %s", sqlite3_errmsg (tdb->priv->db));
|
||||
goto out;
|
||||
}
|
||||
|
||||
/* bind data, so that the freeform proxy text cannot be used to inject SQL */
|
||||
sqlite3_bind_text (statement, 1, proxy_http, -1, SQLITE_STATIC);
|
||||
sqlite3_bind_text (statement, 2, proxy_ftp, -1, SQLITE_STATIC);
|
||||
sqlite3_bind_int (statement, 3, uid);
|
||||
sqlite3_bind_text (statement, 4, session, -1, SQLITE_STATIC);
|
||||
|
||||
/* execute statement */
|
||||
rc = sqlite3_step (statement);
|
||||
if (rc != SQLITE_DONE) {
|
||||
egg_warning ("failed to execute statement: %s", sqlite3_errmsg (tdb->priv->db));
|
||||
goto out;
|
||||
}
|
||||
goto out;
|
||||
@ -717,20 +730,36 @@ pk_transaction_db_set_proxy (PkTransactionDb *tdb, guint uid, const gchar *sessi
|
||||
|
||||
/* insert new entry */
|
||||
timespec = pk_iso8601_present ();
|
||||
statement = g_strdup_printf ("INSERT INTO proxy (created, uid, session, proxy_http, proxy_ftp) VALUES ('%s', '%i', '%s', '%s', '%s')",
|
||||
timespec, uid, session, proxy_http, proxy_ftp);
|
||||
egg_debug ("set proxy for uid:%i and session:%s", uid, session);
|
||||
rc = sqlite3_exec (tdb->priv->db, statement, NULL, NULL, &error_msg);
|
||||
|
||||
/* prepare statement */
|
||||
rc = sqlite3_prepare_v2 (tdb->priv->db,
|
||||
"INSERT INTO proxy (created, uid, session, proxy_http, proxy_ftp) VALUES (?, ?, ?, ?, ?)",
|
||||
-1, &statement, NULL);
|
||||
if (rc != SQLITE_OK) {
|
||||
egg_warning ("SQL error: %s", error_msg);
|
||||
sqlite3_free (error_msg);
|
||||
egg_warning ("failed to prepare statement: %s", sqlite3_errmsg (tdb->priv->db));
|
||||
goto out;
|
||||
}
|
||||
|
||||
/* bind data, so that the freeform proxy text cannot be used to inject SQL */
|
||||
sqlite3_bind_text (statement, 1, timespec, -1, SQLITE_STATIC);
|
||||
sqlite3_bind_int (statement, 2, uid);
|
||||
sqlite3_bind_text (statement, 3, session, -1, SQLITE_STATIC);
|
||||
sqlite3_bind_text (statement, 4, proxy_http, -1, SQLITE_STATIC);
|
||||
sqlite3_bind_text (statement, 5, proxy_ftp, -1, SQLITE_STATIC);
|
||||
|
||||
/* execute statement */
|
||||
rc = sqlite3_step (statement);
|
||||
if (rc != SQLITE_DONE) {
|
||||
egg_warning ("failed to execute statement: %s", sqlite3_errmsg (tdb->priv->db));
|
||||
goto out;
|
||||
}
|
||||
|
||||
ret = TRUE;
|
||||
out:
|
||||
if (statement != NULL)
|
||||
sqlite3_finalize (statement);
|
||||
g_free (timespec);
|
||||
g_free (statement);
|
||||
g_free (proxy_http_tmp);
|
||||
g_free (proxy_ftp_tmp);
|
||||
return ret;
|
||||
|
||||
Loading…
Reference in New Issue
Block a user