[sql] Allow restricting database to user read access.

By default POSIX umask is generally 0644.  For some databases, it
makes sense to restrict access to 0600.

Use new setting for password database.

BUG=258771
R=gbillock@chromium.org, isherman@chromium.org, jorgelo@chromium.org

Review URL: https://codereview.chromium.org/5125579611308032

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@212106 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 116 insertions, 6 deletions

diff --git a/sql/connection.cc b/sql/connection.cc
index 95d09c2..d11b40c 100644
--- a/sql/connection.cc
+++ b/sql/connection.cc
@@ -168,6 +168,7 @@ Connection::Connection()
       page_size_(0),
       cache_size_(0),
       exclusive_locking_(false),
+      restrict_to_user_(false),
       transaction_nesting_(0),
       needs_rollback_(false),
       in_memory_(false),
@@ -732,6 +733,30 @@ bool Connection::OpenInternal(const std::string& file_name,
     return false;
   }
 
+  // TODO(shess): OS_WIN support?
+#if defined(OS_POSIX)
+  if (restrict_to_user_) {
+    DCHECK_NE(file_name, std::string(":memory"));
+    base::FilePath file_path(file_name);
+    int mode = 0;
+    // TODO(shess): Arguably, failure to retrieve and change
+    // permissions should be fatal if the file exists.
+    if (file_util::GetPosixFilePermissions(file_path, &mode)) {
+      mode &= file_util::FILE_PERMISSION_USER_MASK;
+      file_util::SetPosixFilePermissions(file_path, mode);
+
+      // SQLite sets the permissions on these files from the main
+      // database on create.  Set them here in case they already exist
+      // at this point.  Failure to set these permissions should not
+      // be fatal unless the file doesn't exist.
+      base::FilePath journal_path(file_name + FILE_PATH_LITERAL("-journal"));
+      base::FilePath wal_path(file_name + FILE_PATH_LITERAL("-wal"));
+      file_util::SetPosixFilePermissions(journal_path, mode);
+      file_util::SetPosixFilePermissions(wal_path, mode);
+    }
+  }
+#endif  // defined(OS_POSIX)
+
   // SQLite uses a lookaside buffer to improve performance of small mallocs.
   // Chromium already depends on small mallocs being efficient, so we disable
   // this to avoid the extra memory overhead.
diff --git a/sql/connection.h b/sql/connection.h
index 1c4d72c..ab80a6c 100644
--- a/sql/connection.h
+++ b/sql/connection.h
@@ -116,6 +116,12 @@ class SQL_EXPORT Connection {
   // This must be called before Open() to have an effect.
   void set_exclusive_locking() { exclusive_locking_ = true; }
 
+  // Call to cause Open() to restrict access permissions of the
+  // database file to only the owner.
+  // TODO(shess): Currently only supported on OS_POSIX, is a noop on
+  // other platforms.
+  void set_restrict_to_user() { restrict_to_user_ = true; }
+
   // Set an error-handling callback.  On errors, the error number (and
   // statement, if available) will be passed to the callback.
   //
@@ -488,6 +494,7 @@ class SQL_EXPORT Connection {
   int page_size_;
   int cache_size_;
   bool exclusive_locking_;
+  bool restrict_to_user_;
 
   // All cached statements. Keeping a reference to these statements means that
   // they'll remain active.
diff --git a/sql/connection_unittest.cc b/sql/connection_unittest.cc
index 8cf32fb..f516215 100644
--- a/sql/connection_unittest.cc
+++ b/sql/connection_unittest.cc
@@ -71,6 +71,21 @@ void ErrorCallbackResetHelper(sql::Connection* db,
   EXPECT_GT(*counter, 0u);
 }
 
+#if defined(OS_POSIX)
+// Set a umask and restore the old mask on destruction.  Cribbed from
+// shared_memory_unittest.cc.  Used by POSIX-only UserPermission test.
+class ScopedUmaskSetter {
+ public:
+  explicit ScopedUmaskSetter(mode_t target_mask) {
+    old_umask_ = umask(target_mask);
+  }
+  ~ScopedUmaskSetter() { umask(old_umask_); }
+ private:
+  mode_t old_umask_;
+  DISALLOW_IMPLICIT_CONSTRUCTORS(ScopedUmaskSetter);
+};
+#endif
+
 class SQLConnectionTest : public testing::Test {
  public:
   SQLConnectionTest() {}
@@ -224,9 +239,6 @@ TEST_F(SQLConnectionTest, ErrorCallback) {
   {
     sql::ScopedErrorCallback sec(
         &db(), base::Bind(&sql::CaptureErrorCallback, &error));
-
-    // Inserting something other than a number into the primary key
-    // should result in the callback seeing SQLITE_MISMATCH.
     EXPECT_FALSE(db().Execute("INSERT INTO foo (id) VALUES (12)"));
     EXPECT_EQ(SQLITE_CONSTRAINT, error);
   }
@@ -242,9 +254,9 @@ TEST_F(SQLConnectionTest, ErrorCallback) {
   }
 
   // base::Bind() can curry arguments to be passed by const reference
-  // to the callback function.  If the callback function causes
-  // re/set_error_callback() to be called, the storage for those
-  // arguments can be deleted.
+  // to the callback function.  If the callback function calls
+  // re/set_error_callback(), the storage for those arguments can be
+  // deleted while the callback function is still executing.
   //
   // RefCounter() counts how many objects are live using an external
   // count.  The same counter is passed to the callback, so that it
@@ -671,4 +683,70 @@ TEST_F(SQLConnectionTest, Delete) {
   EXPECT_FALSE(base::PathExists(journal));
 }
 
+#if defined(OS_POSIX)
+// Test that set_restrict_to_user() trims database permissions so that
+// only the owner (and root) can read.
+TEST_F(SQLConnectionTest, UserPermission) {
+  // If the bots all had a restrictive umask setting such that
+  // databases are always created with only the owner able to read
+  // them, then the code could break without breaking the tests.
+  // Temporarily provide a more permissive umask.
+  db().Close();
+  sql::Connection::Delete(db_path());
+  ASSERT_FALSE(base::PathExists(db_path()));
+  ScopedUmaskSetter permissive_umask(S_IWGRP | S_IWOTH);
+  ASSERT_TRUE(db().Open(db_path()));
+
+  // Cause the journal file to be created.  If the default
+  // journal_mode is changed back to DELETE, then parts of this test
+  // will need to be updated.
+  EXPECT_TRUE(db().Execute("CREATE TABLE x (x)"));
+
+  base::FilePath journal(db_path().value() + FILE_PATH_LITERAL("-journal"));
+  int mode;
+
+  // Given a permissive umask, the database is created with permissive
+  // read access for the database and journal.
+  ASSERT_TRUE(base::PathExists(db_path()));
+  ASSERT_TRUE(base::PathExists(journal));
+  mode = file_util::FILE_PERMISSION_MASK;
+  EXPECT_TRUE(file_util::GetPosixFilePermissions(db_path(), &mode));
+  ASSERT_NE((mode & file_util::FILE_PERMISSION_USER_MASK), mode);
+  mode = file_util::FILE_PERMISSION_MASK;
+  EXPECT_TRUE(file_util::GetPosixFilePermissions(journal, &mode));
+  ASSERT_NE((mode & file_util::FILE_PERMISSION_USER_MASK), mode);
+
+  // Re-open with restricted permissions and verify that the modes
+  // changed for both the main database and the journal.
+  db().Close();
+  db().set_restrict_to_user();
+  ASSERT_TRUE(db().Open(db_path()));
+  ASSERT_TRUE(base::PathExists(db_path()));
+  ASSERT_TRUE(base::PathExists(journal));
+  mode = file_util::FILE_PERMISSION_MASK;
+  EXPECT_TRUE(file_util::GetPosixFilePermissions(db_path(), &mode));
+  ASSERT_EQ((mode & file_util::FILE_PERMISSION_USER_MASK), mode);
+  mode = file_util::FILE_PERMISSION_MASK;
+  EXPECT_TRUE(file_util::GetPosixFilePermissions(journal, &mode));
+  ASSERT_EQ((mode & file_util::FILE_PERMISSION_USER_MASK), mode);
+
+  // Delete and re-create the database, the restriction should still apply.
+  db().Close();
+  sql::Connection::Delete(db_path());
+  ASSERT_TRUE(db().Open(db_path()));
+  ASSERT_TRUE(base::PathExists(db_path()));
+  ASSERT_FALSE(base::PathExists(journal));
+  mode = file_util::FILE_PERMISSION_MASK;
+  EXPECT_TRUE(file_util::GetPosixFilePermissions(db_path(), &mode));
+  ASSERT_EQ((mode & file_util::FILE_PERMISSION_USER_MASK), mode);
+
+  // Verify that journal creation inherits the restriction.
+  EXPECT_TRUE(db().Execute("CREATE TABLE x (x)"));
+  ASSERT_TRUE(base::PathExists(journal));
+  mode = file_util::FILE_PERMISSION_MASK;
+  EXPECT_TRUE(file_util::GetPosixFilePermissions(journal, &mode));
+  ASSERT_EQ((mode & file_util::FILE_PERMISSION_USER_MASK), mode);
+}
+#endif  // defined(OS_POSIX)
+
 }  // namespace