Modify implementation of AccessController

After this patch is applied, the cyanra object is managed as an instance.
When checking the permission using cynara API, the cynara instance is used.
When the process is terminating, a crash issue occurs when the cynara-thread
calls the cynara_initialize(). The patch is to solve that.

Change-Id: I6b76645c5a43387a48bf996360680dfe87b6e90c
Signed-off-by: Hwankyu Jhun <h.jhun@samsung.com>

diff --git a/src/ac-internal.cc b/src/ac-internal.cc
index 7f21f8f..fed0fb4 100644 (file)
--- a/src/ac-internal.cc
+++ b/src/ac-internal.cc
@@ -14,7 +14,10 @@
  * limitations under the License.
  */
 
+#include "ac-internal.hh"
+
 #include <aul.h>
+#include <cynara-client.h>
 #include <cynara-creds-socket.h>
 #include <cynara-error.h>
 #include <dlog.h>
@@ -22,10 +25,9 @@
 
 #include <utility>
 
-#include "ac-internal.hh"
 #include "aul-internal.hh"
-#include "log-private.hh"
 #include "cynara_thread.hh"
+#include "log-private.hh"
 
 namespace rpc_port {
 namespace internal {
@@ -33,6 +35,90 @@ namespace {
 
 constexpr const uid_t kRegularUidMin = 5000;
 
+class Cynara {
+ public:
+  class Creds {
+   public:
+    Creds(int fd, std::string user, std::string client)
+        : fd_(fd), user_(std::move(user)), client_(std::move(client)) {
+      _W("client(%s), user(%s), fd(%d)", client_.c_str(), user_.c_str(), fd);
+    }
+
+    int GetFd() const { return fd_; }
+
+    const std::string& GetUser() const { return user_; }
+
+    const std::string& GetClient() const { return client_; }
+
+   private:
+    int fd_;
+    std::string user_;
+    std::string client_;
+  };
+
+  Cynara() : handle_(nullptr, cynara_finish) {
+    cynara* handle = nullptr;
+    if (cynara_initialize(&handle, nullptr) != CYNARA_API_SUCCESS) {
+      _E("cynara_initialize() is failed");  // LCOV_EXCL_LINE
+    } else {
+      handle_.reset(handle);
+    }
+  }
+
+  ~Cynara() = default;
+
+  std::shared_ptr<Creds> FetchCredsFromSocket(int fd) {
+    char* user = nullptr;
+    int ret = cynara_creds_socket_get_user(fd, USER_METHOD_DEFAULT, &user);
+    if (ret != CYNARA_API_SUCCESS) {
+      // LCOV_EXCL_START
+      char buf[128] = { 0, };
+      cynara_strerror(ret, buf, sizeof(buf));
+      _E("cynara_creds_socket_get_user() is failed. fd(%d), error(%d:%s)",
+         fd, ret, buf);
+      return nullptr;
+      // LCOV_EXCL_STOP
+    }
+    auto user_auto = std::unique_ptr<char, decltype(free)*>(user, free);
+
+    char* client = nullptr;
+    ret = cynara_creds_socket_get_client(fd, CLIENT_METHOD_DEFAULT, &client);
+    if (ret != CYNARA_API_SUCCESS) {
+      // LCOV_EXCL_START
+      char buf[128] = { 0, };
+      cynara_strerror(ret, buf, sizeof(buf));
+      _E("cynara_creds_socket_get_client() is failed. fd(%d), error(%d:%s)",
+          fd, ret, buf);
+      return nullptr;
+      // LCOV_EXCL_STOP
+    }
+    auto client_auto = std::unique_ptr<char, decltype(free)*>(client, free);
+
+    return std::make_shared<Creds>(fd, user, client);
+  }
+
+  int Check(const std::shared_ptr<Creds>& creds,
+            const std::string& privilege) const {
+    std::lock_guard<std::recursive_mutex> lock(mutex_);
+    _D("check privilege %s", privilege.c_str());
+    int ret = cynara_check(handle_.get(), creds->GetClient().c_str(), "",
+                           creds->GetUser().c_str(), privilege.c_str());
+    if (ret != CYNARA_API_ACCESS_ALLOWED) {
+      _E("cynara_check() is not allowed. privilege(%s), error(%d)",
+         privilege.c_str(), ret);
+      return -1;
+    }
+
+    return 0;
+  }
+
+ private:
+  std::unique_ptr<cynara, decltype(cynara_finish)*> handle_;
+  mutable std::recursive_mutex mutex_;
+};
+
+Cynara cynara_inst;
+
 }  // namespace
 
 void AccessController::AddPrivilege(std::string privilege) {
@@ -43,11 +129,13 @@ void AccessController::SetTrusted(const bool trusted) {
   trusted_ = trusted;
 }
 
-int AccessController::CheckPrivilege(const Cynara& c) {
-  for (auto& privilege : privileges_) {
-    if (c.Check(privilege) != 0) {
+int AccessController::CheckPrivilege(int fd) {
+  auto creds = cynara_inst.FetchCredsFromSocket(fd);
+  if (creds == nullptr) return -1;
+
+  for (const auto& privilege : privileges_) {
+    if (cynara_inst.Check(creds, privilege) != 0)
       return -1;
-    }
   }
 
   return 0;
@@ -79,13 +167,9 @@ int AccessController::CheckTrusted(const std::string& sender_appid) {
 // LCOV_EXCL_STOP
 
 int AccessController::Check(int fd, const std::string& sender_appid) {
-  Cynara cynara;
-  int ret = cynara.FetchCredsFromSocket(fd);
-  if (ret != 0)
-    return -1;
-
+  int ret = 0;
   if (!privileges_.empty()) {
-    ret = CheckPrivilege(cynara);
+    ret = CheckPrivilege(fd);
     if (ret != 0)
       return ret;
   }
@@ -99,7 +183,7 @@ int AccessController::Check(int fd, const std::string& sender_appid) {
 void AccessController::CheckAsync(int fd, std::string sender_appid,
     CompleteCallback callback) {
   /* This is for handle freed issue */
-  auto tmp_handle = new std::shared_ptr<AccessController>(shared_from_this());
+  auto* tmp_handle = new std::shared_ptr<AccessController>(shared_from_this());
   Job job([=]() -> Job::Type {
     if ((*tmp_handle).use_count() == 1) {
       delete tmp_handle;  // LCOV_EXCL_LINE
@@ -129,63 +213,5 @@ void AccessController::CheckAsync(int fd, std::string sender_appid,
   CynaraThread::GetInst().Push(std::move(job));
 }
 
-AccessController::Cynara::Cynara()
-    : cynara_(nullptr, cynara_finish), client_(nullptr, std::free),
-    user_(nullptr, std::free) {
-  cynara* cynara_inst = nullptr;
-
-  if (cynara_initialize(&cynara_inst, NULL) != CYNARA_API_SUCCESS) {
-    _E("cynara_initialize() is failed");  // LCOV_EXCL_LINE
-  } else {
-    cynara_.reset(cynara_inst);
-  }
-}
-
-AccessController::Cynara::~Cynara() = default;
-
-int AccessController::Cynara::FetchCredsFromSocket(int fd) {
-  char* user = nullptr;
-  int ret = cynara_creds_socket_get_user(fd, USER_METHOD_DEFAULT, &user);
-  if (ret != CYNARA_API_SUCCESS) {
-    // LCOV_EXCL_START
-    char buf[128] = { 0, };
-    cynara_strerror(ret, buf, sizeof(buf));
-    _E("cynara_creds_socket_get_user() is failed. fd(%d), error(%d:%s)",
-        fd, ret, buf);
-    return -1;
-    // LCOV_EXCL_STOP
-  }
-  user_.reset(user);
-
-  char* client = nullptr;
-  ret = cynara_creds_socket_get_client(fd, CLIENT_METHOD_DEFAULT, &client);
-  if (ret != CYNARA_API_SUCCESS) {
-    // LCOV_EXCL_START
-    char buf[128] = { 0, };
-    cynara_strerror(ret, buf, sizeof(buf));
-    _E("cynara_creds_socket_get_client() is failed. fd(%d), error(%d:%s)",
-        fd, ret, buf);
-    return -1;
-    // LCOV_EXCL_STOP
-  }
-  client_.reset(client);
-
-  _W("Cred client(%s), user(%s), fd(%d)", client_.get(), user_.get(), fd);
-  return 0;
-}
-
-// LCOV_EXCL_START
-int AccessController::Cynara::Check(const std::string& privilege) const {
-  _D("check privilege %s", privilege.c_str());
-  if (cynara_check(cynara_.get(), client_.get(), "", user_.get(),
-      privilege.c_str()) != CYNARA_API_ACCESS_ALLOWED) {
-    _E("cynara_check() is not allowed : %s", privilege.c_str());
-    return -1;
-  }
-
-  return 0;
-}
-// LCOV_EXCL_STOP
-
 }  // namespace internal
 }  // namespace rpc_port

diff --git a/src/ac-internal.hh b/src/ac-internal.hh
index daefbd7..0b48d5c 100644 (file)
--- a/src/ac-internal.hh
+++ b/src/ac-internal.hh
@@ -17,7 +17,6 @@
 #ifndef AC_INTERNAL_HH_
 #define AC_INTERNAL_HH_
 
-#include <cynara-client.h>
 #include <gio/gio.h>
 #include <glib-unix.h>
 #include <glib.h>
@@ -43,22 +42,8 @@ class AccessController : public std::enable_shared_from_this<AccessController> {
   void CheckAsync(int fd, std::string sender_appid, CompleteCallback callback);
 
  private:
-  class Cynara {
-   public:
-    Cynara();
-    ~Cynara();
-
-    int FetchCredsFromSocket(int fd);
-    int Check(const std::string& privilege) const;
-
-   private:
-    std::unique_ptr<cynara, decltype(cynara_finish)*> cynara_;
-    std::unique_ptr<char, decltype(std::free)*> client_;
-    std::unique_ptr<char, decltype(std::free)*> user_;
-  };
-
   int CheckTrusted(const std::string& sender_appid);
-  int CheckPrivilege(const Cynara& c);
+  int CheckPrivilege(int fd);
 
  private:
   std::vector<std::string> privileges_;