[sql] Allow restricting database to user read access.

sql/connection_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/logging.h"
 #include "sql/connection.h"
 #include "sql/meta_table.h"
@@ skipping 541 matching lines @@
   // of journal_mode PERSIST.
   base::FilePath journal(db_path().value() + FILE_PATH_LITERAL("-journal"));
   ASSERT_TRUE(base::PathExists(db_path()));
   ASSERT_TRUE(base::PathExists(journal));
 
   sql::Connection::Delete(db_path());
   EXPECT_FALSE(base::PathExists(db_path()));
   EXPECT_FALSE(base::PathExists(journal));
 }
 
+#if defined(OS_POSIX)
+TEST_F(SQLConnectionTest, UserPermission) {
+  // 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;
+
+  // If the umask is restrictive, the database files might be created
+  // without group or other access.  In that case, later tests
+  // woudln't test anything real.

[Greg Billock, 2013/07/15 23:26:06]

wouldn't

But I'm not sure I understood this comment anyway. I think you mean that the
user as which the test executes could have a umask which is already  0x700 or
something, in which case tests aren't doing anything. Is it all the tests? Just
after some point? I think that's where I'm confused.

[Scott Hess - ex-Googler, 2013/07/16 18:08:13]

Usually, I would expect the default umask for OSX or Linux to be 022, with some
people setting a more-restrictive umask like 077.  Unfortunately, some of our
bots are that kind of people, and it's entirely possible that people running
bots would consider more-restrictive umask to be a good thing, and if all of the
bots were running with 077, then none of them would actually be testing that the
code works.

Maybe a scoper is in order.  Indeed, base/memory/shared_memory_unittest.cc has
just such a scoper for just such a reason.  I'm going to go with that.

+  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);

[Greg Billock, 2013/07/15 23:26:06]

Will this pass for such a umask? Looks like not, correct? Perhaps bail out of
the test completely here...

[Scott Hess - ex-Googler, 2013/07/16 18:08:13]

Changed so this won't happen.

+  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