Authenticate user/password with PAM in BeginSessionRequest()

remoting/host/user_auth_pam.cc

+// Copyright (c) 2011 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 "remoting/host/user_auth_pam.h"
+
+#include <security/pam_appl.h>
+#include <stdlib.h>
+#include <string>
+
+namespace remoting {
+
+// Private Implementation pattern to avoid leaking PAM types, and the
+// conversation function, into the header file.
+class UserAuthPamPimpl {
+ public:
+  explicit UserAuthPamPimpl(UserAuthPam* user_auth_pam);
+  ~UserAuthPamPimpl();
+  bool Authenticate(const std::string& username,
+                    const std::string& password);
+
+ private:
+  // Conversation function passed to PAM as a callback.
+  static int convFn(int num_msg, const struct pam_message** msg,

[Sergey Ulanov, 2011/02/10 20:14:28]

ConvFunction?

+                     struct pam_response** resp, void* appdata_ptr);
+
+  UserAuthPam* user_auth_pam_;
+  pam_handle_t* pam_handle_;
+
+  // Store these for the PAM conversation function.
+  std::string username_;
+  std::string password_;
+};
+
+UserAuthPamPimpl::UserAuthPamPimpl(UserAuthPam* user_auth_pam)
+    : user_auth_pam_(user_auth_pam) {
+}
+
+UserAuthPamPimpl::~UserAuthPamPimpl() {
+}
+
+bool UserAuthPamPimpl::Authenticate(const std::string& username,
+                                   const std::string& password) {

[Sergey Ulanov, 2011/02/10 20:14:28]

nit: wrapped arguments should be aligned with the arguments on the previous
line. 
  int fun(int arg1, int arg2,
          int arg3, int arg4)

Alternatively you can wrap first argument and indent it with 4 spaces:
  int fun(
      int arg1, int arg2,
      int arg3, int arg4)

+  username_ = username;
+  password_ = password;
+  struct pam_conv c;
+  c.conv = convFn;
+  c.appdata_ptr = static_cast<void*>(this);
+  // TODO(lambroslambrou): allow PAM service name to be configurable

[Sergey Ulanov, 2011/02/10 20:14:28]

nit: comments should be full sentences: start with capital letter and end with
period

+  if (pam_start("chromoting", username_.c_str(), &c, &pam_handle_) !=
+      PAM_SUCCESS) {
+    return false;
+  }
+
+  // TODO(lambroslambrou): move to separate thread

[Sergey Ulanov, 2011/02/10 20:14:28]

nit: same here

+  int pam_status = pam_authenticate(pam_handle_, 0);
+  pam_end(pam_handle_, pam_status);
+  return pam_status == PAM_SUCCESS;
+}
+
+// static
+int UserAuthPamPimpl::convFn(int num_msg, const struct pam_message** msg,
+            struct pam_response** resp, void* appdata_ptr) {

[Sergey Ulanov, 2011/02/10 20:14:28]

nit: indent properly

+  if (num_msg <= 0)
+    return PAM_CONV_ERR;
+  UserAuthPamPimpl* user_auth =
+      reinterpret_cast<UserAuthPamPimpl*>(appdata_ptr);
+  // Must allocate with malloc(), as the calling PAM module will
+  // release the memory with free().
+  struct pam_response* resp_tmp = reinterpret_cast<struct pam_response*>(
+      malloc(num_msg * sizeof(struct pam_response)));
+  if (resp_tmp == NULL)
+    return PAM_CONV_ERR;
+  bool raise_error = false;
+  int count;

[Sergey Ulanov, 2011/02/10 20:14:28]

This can be inside of for()

+  for (count = 0; count < num_msg; count++) {
+    // Alias for readability

[Sergey Ulanov, 2011/02/10 20:14:28]

nit: period at the end of the sentence.

+    struct pam_response &r = resp_tmp[count];
+    r.resp_retcode = 0;
+    r.resp = NULL;
+    switch (msg[count]->msg_style) {
+      case PAM_PROMPT_ECHO_ON:
+        r.resp = strdup(user_auth->username_.c_str());
+        if (r.resp == NULL)
+          raise_error = true;
+        break;
+      case PAM_PROMPT_ECHO_OFF:
+        r.resp = strdup(user_auth->password_.c_str());
+        if (r.resp == NULL)
+          raise_error = true;
+        break;
+      case PAM_TEXT_INFO:
+        // No response needed, continue with next prompt.
+        break;
+      default:
+        // Unexpected style code, so abort.
+        raise_error = true;
+    }
+    if (raise_error)
+      break;
+  }
+
+  if (raise_error) {
+    // Not passing the response back, so free up any memory used.
+    for (int n = 0; n < count; n++) {
+      if (resp_tmp[n].resp) {
+        free(resp_tmp[n].resp);
+      }
+    }
+    free(resp_tmp);
+    return PAM_CONV_ERR;
+  } else {
+    *resp = resp_tmp;
+    return PAM_SUCCESS;
+  }
+}
+
+UserAuthPam::UserAuthPam()
+    : pimpl_(new UserAuthPamPimpl(this)) {
+}
+
+UserAuthPam::~UserAuthPam() {
+}
+
+bool UserAuthPam::Authenticate(const std::string& username,
+                               const std::string& password) {
+  return pimpl_->Authenticate(username, password);
+}
+
+}  // namespace remoting