[sec] Dispatcher issues

[mmd-osm]

Controlling dispatcher file access

Today, the dispatcher config details are kept in shared memory. This memory area is accessible via /run/shm and is currently world-writable via a simple cat > /run/shm/osm3s_v0.7.52_osm_base. For obvious reasons this is not desirable in a multi-user environment.

I propose the following changes:

• Only the overpass dispatcher user is allowed to write to this file

Following these changes, user www-data needs also to be added to this overpass read-only group, so that the db directory can be retrieved from shared memory. In addition, any user on the overpass group ("group of database consumers") should only have read access to the database directory.

• Reading to shared memory is only permitted for users belonging to the overpass group.

In addition, the Dispatcher Client doesn't need read/write access to the shared memory area. After all, the config data belongs to the Dispatcher server and the client is not supposed to change it. Although, we cannot really avoid write access for the overpass user, the Dispatcher client would be better off with read only access by default.

Controlling dispatcher command execution

• Restrict dispatcher write commands to overpass user only

Let's assume user roland is running a dispatcher under his user and another user joe is testing update_database. Unfortunately, joe uses the stock settings ("osm3s_v0.7.52") and forgot to change those.

Although update_database doesn't have write access to Roland's database directory, the dispatcher will happily create a lock file and some shadow files on his behalf. At that time, Roland's own update_database job will stop working as there's already a lock set.

=> Critical dispatcher commands like TERMINATE, WRITE_START, WRITE_COMMIT, WRITE_ROLLBACK, SET_GLOBAL_LIMITS, ... should only be allowed to be triggered by the overpass user. Access to those commands must be forbidden for any other database consumer.

I suggest to use getpeereid and check the peer user id. Only if it matches the dispatcher's own user id, certain critical commands may be called.

• http://doxygen.postgresql.org/getpeereid_8c_source.html

Example:

echo -n -e '\x0\x0\x0\x0\x1\x0\x0\x0' | socat - UNIX-CLIENT:osm3s_v0.7.53_osm_base

• Check for invalid commands in standby_loop

By sending some random, unknown command to the dispatcher unix socket, the dispatcher starts to permanently consume 100% CPU. Other commands are still handled, though.

The issue here is that invalid command is never consumed and the 100ms wait delay no longer applied. Invalid commands should be discarded right away and the connection closed.

To reproduce:

echo "demo" | socat - UNIX-CLIENT:osm3s_v0.7.52_osm_base 

Some suggested changes:

Note: list is not complete.

diff --git a/src/template_db/dispatcher.cc b/src/template_db/dispatcher.cc
index c8fa812..fbbb318 100644
--- a/src/template_db/dispatcher.cc
+++ b/src/template_db/dispatcher.cc
@@ -278,11 +278,11 @@ Dispatcher::Dispatcher
         (errno, dispatcher_share_name, "Dispatcher_Server::APPLE::1");
 #else
   dispatcher_shm_fd = shm_open
-      (dispatcher_share_name.c_str(), O_RDWR|O_CREAT|O_TRUNC|O_EXCL, S_666);
+      (dispatcher_share_name.c_str(), O_RDWR|O_CREAT|O_TRUNC|O_EXCL, S_IRUSR|S_IWUSR|S_IRGRP);
   if (dispatcher_shm_fd < 0)
     throw File_Error
         (errno, dispatcher_share_name, "Dispatcher_Server::1");
-  fchmod(dispatcher_shm_fd, S_666);
+  fchmod(dispatcher_shm_fd, S_IRUSR|S_IWUSR|S_IRGRP);
 #endif

diff --git a/src/template_db/dispatcher_client.cc b/src/template_db/dispatcher_client.cc
index e8611de..3770259 100644
--- a/src/template_db/dispatcher_client.cc
+++ b/src/template_db/dispatcher_client.cc
@@ -40,7 +40,7 @@ Dispatcher_Client::Dispatcher_Client

   // open dispatcher_share
   dispatcher_shm_fd = shm_open
-      (dispatcher_share_name.c_str(), O_RDWR, S_666);
+      (dispatcher_share_name.c_str(), O_RDONLY, S_IRUSR | S_IRGRP);
   if (dispatcher_shm_fd < 0)
     throw File_Error
         (errno, dispatcher_share_name, "Dispatcher_Client::1");
@@ -48,7 +48,7 @@ Dispatcher_Client::Dispatcher_Client
   fstat(dispatcher_shm_fd, &stat_buf);
   dispatcher_shm_ptr = (uint8*)mmap
       (0, stat_buf.st_size,
-       PROT_READ|PROT_WRITE, MAP_SHARED, dispatcher_shm_fd, 0);
+       PROT_READ, MAP_SHARED, dispatcher_shm_fd, 0);

   // get db_dir and shadow_name
   db_dir = std::string((const char *)(dispatcher_shm_ptr + 4*sizeof(uint32)),

[mmd-osm]

All proposed changes have been implemented in https://github.com/mmd-osm/Overpass-API, closing here.