init: service file command only opens existing files

Mixing open or create, along with attribute(MAC) and permissions(DAC)
is a security and confusion issue.

Fix an issue where fcntl F_SETFD was called to clear O_NONBLOCK, when
it should have been F_SETFL.  Did not present a problem because the
current user of this feature does writes and control messages only.

Test: gTest logd-unit-tests and check dmesg for logd content.
Bug: 32450474
Bug: 33242020
Change-Id: I23cb9a9be5ddb7e8e9c58c79838bc07536e766e6

init/Android.mk

@@ -126,7 +126,6 @@ LOCAL_SRC_FILES := \
 LOCAL_SHARED_LIBRARIES += \
     libcutils \
     libbase \
-    libselinux \
 
 LOCAL_STATIC_LIBRARIES := libinit
 LOCAL_SANITIZE := integer

init/descriptors.cpp

@@ -23,6 +23,7 @@
 #include <unistd.h>
 
 #include <android-base/stringprintf.h>
+#include <android-base/unique_fd.h>
 #include <cutils/android_get_control_file.h>
 #include <cutils/sockets.h>
 
@@ -89,14 +90,34 @@ const std::string SocketInfo::key() const {
 
 FileInfo::FileInfo(const std::string& name, const std::string& type, uid_t uid,
                    gid_t gid, int perm, const std::string& context)
+        // defaults OK for uid,..., they are ignored for this class.
         : DescriptorInfo(name, type, uid, gid, perm, context) {
 }
 
-int FileInfo::Create(const std::string& context) const {
-  int flags = ((type() == "r" ? O_RDONLY :
-                (type() == "w" ? (O_WRONLY | O_CREAT) :
-                 (O_RDWR | O_CREAT))));
-  return create_file(name().c_str(), flags, perm(), uid(), gid(), context.c_str());
+int FileInfo::Create(const std::string&) const {
+  int flags = (type() == "r") ? O_RDONLY :
+              (type() == "w") ? O_WRONLY :
+                                O_RDWR;
+
+  // Make sure we do not block on open (eg: devices can chose to block on
+  // carrier detect).  Our intention is never to delay launch of a service
+  // for such a condition.  The service can perform its own blocking on
+  // carrier detect.
+  android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(name().c_str(),
+                                                      flags | O_NONBLOCK)));
+
+  if (fd < 0) {
+    PLOG(ERROR) << "Failed to open file '" << name().c_str() << "'";
+    return -1;
+  }
+
+  // Fixup as we set O_NONBLOCK for open, the intent for fd is to block reads.
+  fcntl(fd, F_SETFL, flags);
+
+  LOG(INFO) << "Opened file '" << name().c_str() << "'"
+            << ", flags " << std::oct << flags << std::dec;
+
+  return fd.release();
 }
 
 const std::string FileInfo::key() const {

init/readme.txt

@@ -148,13 +148,10 @@ socket <name> <type> <perm> [ <user> [ <group> [ <seclabel> ] ] ]
   seclabel or computed based on the service executable file security context.
   For native executables see libcutils android_get_control_socket().
 
-file <path> <type> <perm> [ <user> [ <group> [ <seclabel> ] ] ]
-  Open/Create a file path and pass its fd to the launched process.  <type> must
-  be "r", "w" or "rw".  User and group default to 0.  'seclabel' is the SELinux
-  security context for the file if it must be created.  It defaults to the
-  service security context, as specified by seclabel or computed based on the
-  service executable file security context.  For native executables see
-  libcutils android_get_control_file().
+file <path> <type>
+  Open a file path and pass its fd to the launched process.  <type> must be
+  "r", "w" or "rw".  For native executables see libcutils
+  android_get_control_file().
 
 user <username>
   Change to 'username' before exec'ing this service.

init/service.cpp

@@ -526,7 +526,7 @@ Service::OptionParserMap::Map& Service::OptionParserMap::map() const {
         {"seclabel",    {1,     1,    &Service::ParseSeclabel}},
         {"setenv",      {2,     2,    &Service::ParseSetenv}},
         {"socket",      {3,     6,    &Service::ParseSocket}},
-        {"file",        {2,     6,    &Service::ParseFile}},
+        {"file",        {2,     2,    &Service::ParseFile}},
         {"user",        {1,     1,    &Service::ParseUser}},
         {"writepid",    {1,     kMax, &Service::ParseWritepid}},
     };

init/util.cpp

@@ -160,69 +160,6 @@ out_unlink:
     return -1;
 }
 
-/*
- * create_file - opens and creates a file as dictated in init.rc.
- * This file is inherited by the daemon. We communicate the file
- * descriptor's value via the environment variable ANDROID_FILE_<basename>
- */
-int create_file(const char *path, int flags, mode_t perm, uid_t uid,
-                  gid_t gid, const char *filecon)
-{
-    char *secontext = NULL;
-
-    if (filecon) {
-        if (setsockcreatecon(filecon) == -1) {
-            PLOG(ERROR) << "setsockcreatecon(\"" << filecon << "\") failed";
-            return -1;
-        }
-    } else if (sehandle) {
-        if (selabel_lookup(sehandle, &secontext, path, perm) != -1) {
-            if (setfscreatecon(secontext) == -1) {
-                freecon(secontext); // does not upset errno value
-                PLOG(ERROR) << "setfscreatecon(\"" << secontext << "\") failed";
-                return -1;
-            }
-        }
-    }
-
-    android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(path, flags | O_NDELAY, perm)));
-    int savederrno = errno;
-
-    if (filecon) {
-        setsockcreatecon(NULL);
-        lsetfilecon(path, filecon);
-    } else {
-        setfscreatecon(NULL);
-        freecon(secontext);
-    }
-
-    if (fd < 0) {
-        errno = savederrno;
-        PLOG(ERROR) << "Failed to open/create file '" << path << "'";
-        return -1;
-    }
-
-    if (!(flags & O_NDELAY)) fcntl(fd, F_SETFD, flags);
-
-    if (lchown(path, uid, gid)) {
-        PLOG(ERROR) << "Failed to lchown file '" << path << "'";
-        return -1;
-    }
-    if (perm != static_cast<mode_t>(-1)) {
-        if (fchmodat(AT_FDCWD, path, perm, AT_SYMLINK_NOFOLLOW)) {
-            PLOG(ERROR) << "Failed to fchmodat file '" << path << "'";
-            return -1;
-        }
-    }
-
-    LOG(INFO) << "Created file '" << path << "'"
-              << ", mode " << std::oct << perm << std::dec
-              << ", user " << uid
-              << ", group " << gid;
-
-    return fd.release();
-}
-
 bool read_file(const char* path, std::string* content) {
     content->clear();
 

init/util.h

@@ -31,8 +31,6 @@ using namespace std::chrono_literals;
 
 int create_socket(const char *name, int type, mode_t perm,
                   uid_t uid, gid_t gid, const char *socketcon);
-int create_file(const char *path, int mode, mode_t perm,
-                uid_t uid, gid_t gid, const char *filecon);
 
 bool read_file(const char* path, std::string* content);
 int write_file(const char* path, const char* content);

init/util_test.cpp

@@ -16,19 +16,9 @@
 
 #include "util.h"
 
-#include <ctype.h>
 #include <errno.h>
-#include <fcntl.h>
-#include <stdlib.h>
-#include <sys/stat.h>
-#include <sys/types.h>
-#include <unistd.h>
 
-#include <android-base/stringprintf.h>
-#include <android-base/test_utils.h>
-#include <cutils/android_get_control_file.h>
 #include <gtest/gtest.h>
-#include <selinux/android.h>
 
 TEST(util, read_file_ENOENT) {
   std::string s("hello");
@@ -52,54 +42,3 @@ TEST(util, decode_uid) {
   EXPECT_EQ(UINT_MAX, decode_uid("toot"));
   EXPECT_EQ(123U, decode_uid("123"));
 }
-
-struct selabel_handle *sehandle;
-
-TEST(util, create_file) {
-  if (!sehandle) sehandle = selinux_android_file_context_handle();
-
-  TemporaryFile tf;
-  close(tf.fd);
-  EXPECT_GE(unlink(tf.path), 0);
-
-  std::string key(ANDROID_FILE_ENV_PREFIX);
-  key += tf.path;
-
-  std::for_each(key.begin(), key.end(), [] (char& c) { c = isalnum(c) ? c : '_'; });
-
-  EXPECT_EQ(unsetenv(key.c_str()), 0);
-
-  uid_t uid = decode_uid("logd");
-  gid_t gid = decode_uid("system");
-  mode_t perms = S_IRWXU | S_IWGRP | S_IRGRP | S_IROTH;
-  static const char context[] = "u:object_r:misc_logd_file:s0";
-  EXPECT_GE(tf.fd = create_file(tf.path, O_RDWR | O_CREAT, perms, uid, gid, context), 0);
-  if (tf.fd < 0) return;
-  static const char hello[] = "hello world\n";
-  static const ssize_t len = strlen(hello);
-  EXPECT_EQ(write(tf.fd, hello, len), len);
-  char buffer[sizeof(hello) + 1];
-  memset(buffer, 0, sizeof(buffer));
-  EXPECT_GE(lseek(tf.fd, 0, SEEK_SET), 0);
-  EXPECT_EQ(read(tf.fd, buffer, sizeof(buffer)), len);
-  EXPECT_EQ(std::string(hello), buffer);
-  EXPECT_EQ(android_get_control_file(tf.path), -1);
-  EXPECT_EQ(setenv(key.c_str(), android::base::StringPrintf("%d", tf.fd).c_str(), true), 0);
-  EXPECT_EQ(android_get_control_file(tf.path), tf.fd);
-  close(tf.fd);
-  EXPECT_EQ(android_get_control_file(tf.path), -1);
-  EXPECT_EQ(unsetenv(key.c_str()), 0);
-  struct stat st;
-  EXPECT_EQ(stat(tf.path, &st), 0);
-  EXPECT_EQ(st.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO), perms);
-  EXPECT_EQ(st.st_uid, uid);
-  EXPECT_EQ(st.st_gid, gid);
-  security_context_t con;
-  EXPECT_GE(getfilecon(tf.path, &con), 0);
-  EXPECT_NE(con, static_cast<security_context_t>(NULL));
-  if (con) {
-    EXPECT_EQ(context, std::string(con));
-  }
-  freecon(con);
-  EXPECT_EQ(unlink(tf.path), 0);
-}