Apply remaining changes

Co-authored-by: rakshasa <475125+rakshasa@users.noreply.github.com>

doc/rtorrent.rc

@@ -45,6 +45,13 @@
 #
 #session.path.set = ./session
 
+# Umask for files written to the session directory, default 0077 (files
+# are mode 0600). Use 0007 for 0660 or 0027 for 0640. The owner must keep
+# read/write access. Session files contain tracker announce URLs
+# (potentially passkeys), so loosening permissions is a user choice.
+#
+#session.umask.set = 0077
+
 # Watch a directory for new torrents, and stop those that have been
 # deleted. Use directory.watch.ready for network shares or other watch
 # directories where files may be copied or written directly into place.

src/command_local.cc

@@ -123,6 +123,8 @@ initialize_command_local() {
   CMD_ANY_STRING_V("session.path.set",                [](auto, auto& str)   { return session_thread::manager()->set_path(str); });
   CMD_ANY         ("session.use_lock",                [](auto, auto)        { return session_thread::manager()->use_lock(); });
   CMD_ANY_VALUE_V ("session.use_lock.set",            [](auto, auto& value) { return session_thread::manager()->set_use_lock(value); });
+  CMD_ANY         ("session.umask",                   [](auto, auto)        { return session_thread::manager()->umask(); });
+  CMD_ANY_VALUE_V ("session.umask.set",               [](auto, auto& value) { return session_thread::manager()->set_umask(value); });
   CMD_VAR_BOOL    ("session.on_completion",           true);
 
   CMD_ANY_V       ("session.save",                    [dList](auto, auto)   { return dList->session_save(); });
@@ -171,6 +173,7 @@ initialize_command_local() {
   rpc::rpc.mark_safe("directory.default.realpath.or_empty");
   rpc::rpc.mark_safe("directory.default.realpath.or_throw");
   rpc::rpc.mark_safe("session.use_lock");
+  rpc::rpc.mark_safe("session.umask");
   rpc::rpc.mark_safe("session.on_completion");
 
   rpc::rpc.mark_safe("pieces.sync.always_safe");

src/session/download_storer.cc

@@ -5,6 +5,7 @@
 #include <cerrno>
 #include <fcntl.h>
 #include <fstream>
+#include <sys/stat.h>
 #include <unistd.h>
 #include <torrent/exceptions.h>
 #include <torrent/object.h>
@@ -123,7 +124,7 @@ is_correct_format(const std::string& f) {
 }
 
 void
-save_stream(const std::string& path, bool use_fsyncdisk, const std::stringstream& stream) {
+save_stream(const std::string& path, bool use_fsyncdisk, mode_t file_mode, const std::stringstream& stream) {
   // Remove any leftover temporary file first so that O_EXCL only ever fails on
   // an entry that appeared after the unlink, and O_NOFOLLOW keeps a symlink
   // planted in the session directory from redirecting the write.
@@ -136,6 +137,12 @@ save_stream(const std::string& path, bool use_fsyncdisk, const std::stringstream
   if (fd < 0)
     throw torrent::storage_error("failed to open file for writing : " + path);
 
+  // open() applies the process umask, so set the configured mode explicitly.
+  if (::fchmod(fd, file_mode) == -1) {
+    ::close(fd);
+    throw torrent::storage_error("failed to set file permissions : " + path);
+  }
+
   const auto  data      = stream.view();
   std::size_t remaining = data.size();
   const char* cursor    = data.data();
@@ -196,7 +203,7 @@ sync_directory_of(const std::string& path) {
 } // namespace anonymous
 
 void
-DownloadStorer::save_and_move_streams(const std::string& path, bool use_fsyncdisk,
+DownloadStorer::save_and_move_streams(const std::string& path, bool use_fsyncdisk, mode_t file_mode,
                                        const std::stringstream* torrent_stream,
                                        const std::stringstream* rtorrent_stream,
                                        const std::stringstream* libtorrent_stream) {
@@ -205,10 +212,10 @@ DownloadStorer::save_and_move_streams(const std::string& path, bool use_fsyncdis
   auto rtorrent_path   = path + ".rtorrent";
 
   if (torrent_stream)
-    save_stream(torrent_path + ".new", use_fsyncdisk, *torrent_stream);
+    save_stream(torrent_path + ".new", use_fsyncdisk, file_mode, *torrent_stream);
 
-  save_stream(libtorrent_path + ".new", use_fsyncdisk, *libtorrent_stream);
+  save_stream(libtorrent_path + ".new", use_fsyncdisk, file_mode, *libtorrent_stream);
-  save_stream(rtorrent_path + ".new", use_fsyncdisk, *rtorrent_stream);
+  save_stream(rtorrent_path + ".new", use_fsyncdisk, file_mode, *rtorrent_stream);
 
   if (torrent_stream) {
     if (::rename((torrent_path + ".new").c_str(), torrent_path.c_str()) == -1)

src/session/download_storer.h

@@ -3,6 +3,7 @@
 
 #include <memory>
 #include <sstream>
+#include <sys/types.h>
 #include <string>
 #include <torrent/common.h>
 
@@ -33,7 +34,7 @@ public:
   auto                rtorrent_stream()      { return std::move(m_rtorrent_stream); }
   auto                libtorrent_stream()    { return std::move(m_libtorrent_stream); }
 
-  static void         save_and_move_streams(const std::string& path, bool use_fsyncdisk,
+  static void         save_and_move_streams(const std::string& path, bool use_fsyncdisk, mode_t file_mode,
                                             const std::stringstream* torrent_stream,
                                             const std::stringstream* rtorrent_stream,
                                             const std::stringstream* libtorrent_stream);

src/session/session_manager.cc

@@ -65,6 +65,22 @@ SessionManager::set_use_lock(bool use_lock) {
   m_use_lock = use_lock;
 }
 
+void
+SessionManager::set_umask(int64_t umask) {
+  assert(torrent::this_thread::thread() == torrent::main_thread::thread());
+
+  if (m_freeze_info)
+    throw torrent::input_error("Session umask option cannot be changed after startup.");
+
+  if (umask < 0 || umask > 0777)
+    throw torrent::input_error("Session umask must be in the range 0 to 0777.");
+
+  if ((umask & 0600) != 0)
+    throw torrent::input_error("Session umask must not mask owner read/write permissions (0600).");
+
+  m_umask = umask;
+}
+
 void
 SessionManager::save_resume_download(core::Download* download) {
   assert(torrent::this_thread::thread() == torrent::main_thread::thread());
@@ -379,7 +395,7 @@ SessionManager::process_next_save_request_unsafe() {
         };
 
       try {
-        DownloadStorer::save_and_move_streams(itr->second.path, m_use_fsyncdisk,
+        DownloadStorer::save_and_move_streams(itr->second.path, m_use_fsyncdisk, file_mode(),
                                               itr->second.torrent_stream.get(),
                                               itr->second.rtorrent_stream.get(),
                                               itr->second.libtorrent_stream.get());

src/session/session_manager.h

@@ -6,6 +6,8 @@
 #include <deque>
 #include <future>
 #include <list>
+#include <cstdint>
+#include <sys/types.h>
 #include <memory>
 #include <mutex>
 #include <sstream>
@@ -59,6 +61,10 @@ public:
   bool                use_lock() const;
   void                set_use_lock(bool use_lock);
 
+  int                 umask() const;
+  void                set_umask(int64_t umask);
+  mode_t              file_mode() const;
+
   void                save_full_download(core::Download* download);
   void                save_resume_download(core::Download* download);
   void                remove_download(core::Download* download);
@@ -99,6 +105,7 @@ private:
   std::string                  m_path;
   bool                         m_use_fsyncdisk{true};
   bool                         m_use_lock{true};
+  int                          m_umask{0077};
 
   align_cacheline
 
@@ -133,6 +140,8 @@ inline bool        SessionManager::is_used() const            { return !m_path.e
 inline std::string SessionManager::path() const               { return m_path; }
 inline bool        SessionManager::use_fsyncdisk() const      { return m_use_fsyncdisk; }
 inline bool        SessionManager::use_lock() const           { return m_use_lock; }
+inline int         SessionManager::umask() const              { return m_umask; }
+inline mode_t      SessionManager::file_mode() const          { return 0666 & ~m_umask; }
 inline void        SessionManager::flush_all_pending_builds() { process_pending_builds(true); }
 
 } // namespace session

test/src/test_session_commit.cc

@@ -73,7 +73,7 @@ TestSessionCommit::commit_and_verify(bool use_fsyncdisk) {
   std::stringstream rtorrent_stream("rtorrent-data");
   std::stringstream libtorrent_stream("libtorrent-data");
 
-  session::DownloadStorer::save_and_move_streams(path, use_fsyncdisk, &torrent_stream, &rtorrent_stream, &libtorrent_stream);
+  session::DownloadStorer::save_and_move_streams(path, use_fsyncdisk, 0600, &torrent_stream, &rtorrent_stream, &libtorrent_stream);
 
   CPPUNIT_ASSERT_EQUAL(std::string("torrent-data"), read_file(path));
   CPPUNIT_ASSERT_EQUAL(std::string("rtorrent-data"), read_file(path + ".rtorrent"));

test/src/test_session_storer.cc

@@ -39,12 +39,12 @@ read_file(const std::string& path) {
 }
 
 void
-save_session_files(const std::string& path) {
+save_session_files(const std::string& path, mode_t file_mode = 0600) {
   std::stringstream torrent_stream("torrent-data");
   std::stringstream rtorrent_stream("rtorrent-data");
   std::stringstream libtorrent_stream("libtorrent-data");
 
-  session::DownloadStorer::save_and_move_streams(path, false, &torrent_stream, &rtorrent_stream, &libtorrent_stream);
+  session::DownloadStorer::save_and_move_streams(path, false, file_mode, &torrent_stream, &rtorrent_stream, &libtorrent_stream);
 }
 
 unsigned int
@@ -130,6 +130,21 @@ TestSessionStorer::test_saved_files_are_owner_only() {
   CPPUNIT_ASSERT_EQUAL(0600u, permissions_of(path + ".libtorrent_resume"));
 }
 
+void
+TestSessionStorer::test_saved_files_use_configured_mode() {
+  for (mode_t mode : {0660, 0640}) {
+    auto path       = m_session_dir + "/" + entry_name;
+    auto prev_umask = ::umask(0);
+
+    save_session_files(path, mode);
+    ::umask(prev_umask);
+
+    CPPUNIT_ASSERT_EQUAL((unsigned int)mode, permissions_of(path));
+    CPPUNIT_ASSERT_EQUAL((unsigned int)mode, permissions_of(path + ".rtorrent"));
+    CPPUNIT_ASSERT_EQUAL((unsigned int)mode, permissions_of(path + ".libtorrent_resume"));
+  }
+}
+
 // Entries listed for loading must report their real type so that a symlink in
 // the session directory is skipped instead of loaded.
 void

test/src/test_session_storer.h

@@ -7,6 +7,7 @@ class TestSessionStorer : public test_fixture {
 
   CPPUNIT_TEST(test_temp_file_symlink_is_not_followed);
   CPPUNIT_TEST(test_saved_files_are_owner_only);
+  CPPUNIT_TEST(test_saved_files_use_configured_mode);
   CPPUNIT_TEST(test_symlinked_entry_is_not_a_file);
 
   CPPUNIT_TEST_SUITE_END();
@@ -17,6 +18,7 @@ public:
 
   void test_temp_file_symlink_is_not_followed();
   void test_saved_files_are_owner_only();
+  void test_saved_files_use_configured_mode();
   void test_symlinked_entry_is_not_a_file();
 
 private: