ZOOKEEPER-4895: Support encrypted password file during SASL authentication for C client

Reviewers: cnauroth, kezhuw
Author: empiredan
Closes #2223 from empiredan/zk-c-client-sasl-passwd-cb

zookeeper-client/zookeeper-client-c/CMakeLists.txt

@@ -249,6 +249,7 @@ set(test_sources
   tests/ThreadingUtil.cc
   tests/TestZookeeperInit.cc
   tests/TestZookeeperClose.cc
+  tests/TestSASLAuth.cc
   tests/TestReconfig.cc
   tests/TestReconfigServer.cc
   tests/TestClientRetry.cc

zookeeper-client/zookeeper-client-c/include/zookeeper.h

@@ -667,6 +667,70 @@ ZOOAPI zhandle_t *zookeeper_init_sasl(const char *host, watcher_fn fn,
 ZOOAPI sasl_callback_t *zoo_sasl_make_basic_callbacks(const char *user,
   const char *realm, const char* password_file);
 
+/**
+ * \brief signature of the callback function for SASL password.
+ *
+ * This callback is defined by user to decrypt the content of password file with
+ * context into the actual password.
+ *
+ * \param content the string read from the password file.
+ * \param content_len the size of the content in bytes.
+ * \param context the handback object that will be associated with the password
+ *   file. The object is not used by zookeeper internally and can be null.
+ * \param buf the buffer where the resulting actual password is saved.
+ * \param buf_len the size of buf in bytes, which is also the max allowed
+ *   password length.
+ * \param passwd_len as an output parameter of the callback function, passwd_len
+ *   points to the actual length of the password stored in buf. Its size must not
+ *   exceed buf_len; otherwise, SASL_BUFOVER will be returned.
+ * \return SASL_OK, or the possible errors defined by the SASL library.
+ */
+typedef int (*zoo_sasl_password_callback_t)(const char *content, size_t content_len,
+        void *context, char *buf, size_t buf_len, size_t *passwd_len);
+
+/**
+ * \brief zoo_sasl_password structure.
+ *
+ * This structure holds the parameters for getting the actual SASL password. The
+ * user-defined callback is a processor that decrypts the content of password_file
+ * with context as a handback object helping the decryption into the actual password
+ * as is described above for zoo_sasl_password_callback_t.
+ *
+ * All of password_file, context and callback are allowed to be null; however, once
+ * callback is defined, password_file should not be null since the content processed
+ * by callback is from password_file. If callback is null and password_file is given,
+ * the first line of the file is just the actual password. While both password_file
+ * and callback are null, the password would be read from terminal with prompt.
+ *
+ * zoo_sasl_password is defined by user and used to generate the actual password in
+ * response to \c SASL_CB_PASS.
+ */
+typedef struct zoo_sasl_password {
+  const char *password_file;                /*!< The path of the password file */
+  void *context;                            /*!< The handback object used by callback */
+  zoo_sasl_password_callback_t callback;    /*!< Callback over the content of password file */
+} zoo_sasl_password_t;
+
+/**
+ * \brief allocates and initializes a basic array of Cyrus SASL callbacks.
+ *
+ * This helper function is similar to zoo_sasl_make_basic_callbacks, except that
+ * the actual password is generated by zoo_sasl_password object rather than just
+ * read from the password file.
+ *
+ * \param user the "canned" response to \c SASL_CB_USER and \c SASL_CB_AUTHNAME,
+ *   or NULL for none.
+ * \param realm the "canned" response to \c SASL_CB_GETREALM, or NULL for none.
+ * \param password the object defined by user to specify how the actual password
+ *   is generated in response to \c SASL_CB_PASS, should never be NULL (otherwise
+ *   the behaviour is undefined), see struct zoo_sasl_password for details.
+ * \return the freshly-malloc()ed callbacks array, or NULL if allocation
+ *   failed. Deallocate with free(), but only after the corresponding
+ *   ZooKeeper handle is closed.
+ */
+ZOOAPI sasl_callback_t *zoo_sasl_make_password_callbacks(const char *user,
+  const char *realm, zoo_sasl_password_t *password);
+
 #endif /* HAVE_CYRUS_SASL_H */
 
 /**

zookeeper-client/zookeeper-client-c/src/zk_sasl.c

@@ -442,6 +442,8 @@ getpassphrase(const char *prompt) {
 
 struct zsasl_secret_ctx {
     const char *password_file;
+    void *context;
+    zoo_sasl_password_callback_t callback;
     sasl_secret_t *secret;
 };
 
@@ -451,53 +453,111 @@ struct zsasl_secret_ctx {
 static int _zsasl_getsecret(sasl_conn_t *conn, void *context, int id,
                             sasl_secret_t **psecret)
 {
+    /* Max allowed length of a password */
+    const size_t MAX_PASSWORD_LEN = 1023; 
     struct zsasl_secret_ctx *secret_ctx = (struct zsasl_secret_ctx *)context;
-    char buf[1024];
-    char *password;
-    size_t len;
+    /* The extra 1 byte is reserved for storing the null terminator. */
+    char buf[MAX_PASSWORD_LEN + 1]; 
+    char *password = NULL;
+    size_t len = 0;
+    int res = 0;
+    /* The extra 1 byte is reserved for storing the null terminator. */
+    char new_passwd[MAX_PASSWORD_LEN + 1];
+    char *p = NULL;
     sasl_secret_t *x;
 
     /* paranoia check */
-    if (!conn || !psecret || id != SASL_CB_PASS)
+    if (!conn || !psecret || id != SASL_CB_PASS) {
         return SASL_BADPARAM;
+    }
 
     if (secret_ctx->password_file) {
-        char *p;
         FILE *fh = fopen(secret_ctx->password_file, "rt");
-        if (!fh)
+        if (!fh) {
             return SASL_FAIL;
+        }
 
-        if (!fgets(buf, sizeof(buf), fh)) {
+        /*
+         * The file's content may be the encrypted password with binary characters,
+         * thus use fread().
+         */
+        len = fread(buf, sizeof(buf[0]), MAX_PASSWORD_LEN, fh);
+        if (ferror(fh)) {
             fclose(fh);
+            fh = NULL;
+
             return SASL_FAIL;
         }
 
         fclose(fh);
+        fh = NULL;
+
+        /*
+         * Write the null terminator immediately after the last character of the
+         * content since it would be used as a null-terminated string once it is
+         * the actual password.
+         */
+        buf[len] = '\0';
+        password = buf;
+    }
 
-        p = strrchr(buf, '\n');
-        if (p)
-            *p = '\0';
+    if (secret_ctx->callback) {
+        if (!password) {
+            /*
+             * The callback takes effect only when password_file is provided.
+             */
+            return SASL_BADPARAM;
+        }
 
-        password = buf;
+        res = secret_ctx->callback(password, len, secret_ctx->context,
+            new_passwd, MAX_PASSWORD_LEN, &len);
+        if (res != SASL_OK) {
+            return res;
+        }
+
+        if (len > MAX_PASSWORD_LEN) {
+            return SASL_BUFOVER;
+        }
+
+        /* 
+         * Append the null terminator  to the end of the password obtained from
+         * the callback function.
+         */
+        new_passwd[len] = '\0';
+        password = new_passwd;
+    } else if (secret_ctx->password_file) {
+        /*
+         * The file's content is the actual password, which must consist only of
+         * text characters (i.e., without null terminator). The first line would
+         * be read as the password once there are multiple lines in the file.
+         */
+        p = strchr(password, '\n');
+        if (p) {
+            *p = '\0';
+        }
     } else {
         password = getpassphrase("Password: ");
-
-        if (!password)
+        if (!password) {
             return SASL_FAIL;
+        }
     }
 
+    /*
+     * Any password, regardless of its source, is always null-terminated.
+     */
     len = strlen(password);
 
     x = secret_ctx->secret = (sasl_secret_t *)realloc(
         secret_ctx->secret, sizeof(sasl_secret_t) + len);
-
     if (!x) {
         memset(password, 0, len);
         return SASL_NOMEM;
     }
 
     x->len = len;
-    strcpy((char *) x->data, password);
+
+    /* The extra 1 byte is the null terminator. */
+    memcpy(x->data, password, len + 1);
     memset(password, 0, len);
 
     *psecret = x;
@@ -506,9 +566,9 @@ static int _zsasl_getsecret(sasl_conn_t *conn, void *context, int id,
 
 typedef int (* sasl_callback_fn_t)(void);
 
-sasl_callback_t *zoo_sasl_make_basic_callbacks(const char *user,
-                                               const char *realm,
-                                               const char* password_file)
+sasl_callback_t *zoo_sasl_make_password_callbacks(const char *user,
+                                                  const char *realm,
+                                                  zoo_sasl_password_t *password)
 {
     struct zsasl_secret_ctx *secret_ctx;
     const char *user_ctx = NULL;
@@ -521,7 +581,9 @@ sasl_callback_t *zoo_sasl_make_basic_callbacks(const char *user,
 
     rc = rc < 0 ? rc : _zsasl_strdup(&user_ctx, user);
     rc = rc < 0 ? rc : _zsasl_strdup(&realm_ctx, realm);
-    rc = rc < 0 ? rc : _zsasl_strdup(&secret_ctx->password_file, password_file);
+    rc = rc < 0 ? rc : _zsasl_strdup(&secret_ctx->password_file, password->password_file);
+    secret_ctx->context = password->context;
+    secret_ctx->callback = password->callback;
 
     {
         sasl_callback_t callbacks[] = {
@@ -551,6 +613,14 @@ sasl_callback_t *zoo_sasl_make_basic_callbacks(const char *user,
     }
 }
 
+sasl_callback_t *zoo_sasl_make_basic_callbacks(const char *user,
+                                               const char *realm,
+                                               const char* password_file)
+{
+    zoo_sasl_password_t password = {password_file, NULL, NULL};
+    return zoo_sasl_make_password_callbacks(user, realm, &password);
+}
+
 #ifdef __APPLE__
 #pragma GCC diagnostic pop
 #endif

zookeeper-client/zookeeper-client-c/tests/TestSASLAuth.cc

@@ -34,6 +34,10 @@ class Zookeeper_SASLAuth : public CPPUNIT_NS::TestFixture {
     CPPUNIT_TEST(testServerRequireClientSASL);
 #ifdef HAVE_CYRUS_SASL_H
     CPPUNIT_TEST(testClientSASL);
+    CPPUNIT_TEST(testClientSASLWithPasswordFileNewline);
+    CPPUNIT_TEST(testClientSASLWithPasswordFileFirstLine);
+    CPPUNIT_TEST(testClientSASLWithPasswordEncryptedText);
+    CPPUNIT_TEST(testClientSASLWithPasswordEncryptedBinary);
 #ifdef ZOO_IPV6_ENABLED
     CPPUNIT_TEST(testClientSASLOverIPv6);
 #endif/* ZOO_IPV6_ENABLED */
@@ -140,7 +144,8 @@ public:
 #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
 #endif
 
-    void testClientSASLHelper(const char *hostPorts, const char *path) {
+    void testClientSASLHelper(const char *hostPorts, const char *path,
+                              const sasl_callback_t *callbacks) {
         startServer();
 
         // Initialize Cyrus SASL.
@@ -152,8 +157,7 @@ public:
         sasl_params.service = "zookeeper";
         sasl_params.host = "zk-sasl-md5";
         sasl_params.mechlist = "DIGEST-MD5";
-        sasl_params.callbacks = zoo_sasl_make_basic_callbacks(
-            "myuser", NULL, "Zookeeper_SASLAuth.password");
+        sasl_params.callbacks = callbacks;
 
         // Connect.
         watchctx_t ctx;
@@ -188,10 +192,115 @@ public:
         stopServer();
     }
 
+    void testClientSASLHelper(const char *hostPorts, const char *path,
+                              const char *password_file) {
+        const sasl_callback_t *callbacks = zoo_sasl_make_basic_callbacks(
+            "myuser", NULL, password_file);
+        testClientSASLHelper(hostPorts, path, callbacks);
+    }
+
+    void testClientSASLHelper(const char *hostPorts, const char *path) {
+        testClientSASLHelper(hostPorts, path, "Zookeeper_SASLAuth.password");
+    }
+
+    void testClientSASLHelper(const char *hostPorts, const char *path,
+                              zoo_sasl_password_t *password) {
+        const sasl_callback_t *callbacks = zoo_sasl_make_password_callbacks(
+            "myuser", NULL, password);
+        testClientSASLHelper(hostPorts, path, callbacks);
+    }
+
     void testClientSASL() {
         testClientSASLHelper(hostPorts, "/clientSASL");
     }
 
+    void testClientSASL(const char *password_file, const char *content, size_t content_len,
+                        const char *path, zoo_sasl_password_callback_t callback) {
+        // Create password file for client.
+        FILE *passf = fopen(password_file, "wt");
+        CPPUNIT_ASSERT(passf);
+
+        // Write the specified content into the file.
+        size_t len = fwrite(content, sizeof(content[0]), content_len, passf);
+        CPPUNIT_ASSERT_EQUAL(len, content_len);
+        CPPUNIT_ASSERT_EQUAL(fclose(passf), 0);
+        passf = NULL;
+
+        zoo_sasl_password_t passwd = {password_file, this, callback};
+        testClientSASLHelper(hostPorts, path, &passwd);
+    }
+
+    void testClientSASLWithPasswordFileNewline() {
+        // Insert a newline immediately after the correct password.
+        const char content[] = "mypassword\nabc";
+        testClientSASL("Zookeeper_SASLAuth.password.newline",
+                       content,
+                       sizeof(content) - 1,
+                       "/clientSASLWithPasswordFileNewline",
+                       NULL);
+    }
+
+    void testClientSASLWithPasswordFileFirstLine() {
+        // Insert multiple newlines and check if only the first line is accepted as the
+        // actual password.
+        const char content[] = "mypassword\nabc\nxyz";
+        testClientSASL("Zookeeper_SASLAuth.password.firstline",
+                       content,
+                       sizeof(content) - 1,
+                       "/clientSASLWithPasswordFileFirstLine",
+                       NULL);
+    }
+
+    int decryptPassword(const char *content, size_t content_len,
+                        char incr, char *buf, size_t buf_len,
+                        size_t *passwd_len) {
+        CPPUNIT_ASSERT(content_len <= buf_len);
+
+        // A simple decryption that only increases each character by a fixed value.
+        for (size_t i = 0; i < content_len; ++i) {
+            buf[i] = content[i] + incr;
+        }
+        *passwd_len = content_len;
+
+        // Since null terminator has not been appended to buf, use memcmp.  
+        CPPUNIT_ASSERT_EQUAL(memcmp(buf, "mypassword", *passwd_len), 0);
+        return SASL_OK;
+    }
+
+    static int textPasswordCallback(const char *content, size_t content_len,
+                                    void *context, char *buf, size_t buf_len,
+                                    size_t *passwd_len) {
+        Zookeeper_SASLAuth *auth = static_cast<Zookeeper_SASLAuth *>(context);
+        return auth->decryptPassword(content, content_len, 1, buf, buf_len, passwd_len);
+    }
+
+    void testClientSASLWithPasswordEncryptedText() {
+        // Encrypt "mypassword" by subtracting 1 from each character in it as plain text.
+        const char content[] = {0x6C, 0x78, 0x6F, 0x60, 0x72, 0x72, 0x76, 0x6E, 0x71, 0x63};
+        testClientSASL("Zookeeper_SASLAuth.password.encrypted.text",
+                       content,
+                       sizeof(content),
+                       "/clientSASLWithPasswordEncryptedText",
+                       textPasswordCallback);
+    }
+
+    static int binaryPasswordCallback(const char *content, size_t content_len,
+                                      void *context, char *buf, size_t buf_len,
+                                      size_t *passwd_len) {
+        Zookeeper_SASLAuth *auth = static_cast<Zookeeper_SASLAuth *>(context);
+        return auth->decryptPassword(content, content_len, 'a', buf, buf_len, passwd_len);
+    }
+
+    void testClientSASLWithPasswordEncryptedBinary() {
+        // Encrypt "mypassword" by subtracting 'a' from each character in it as binary format.
+        const char content[] = {0x0C, 0x18, 0x0F, 0x00, 0x12, 0x12, 0x16, 0x0E, 0x11, 0x03};
+        testClientSASL("Zookeeper_SASLAuth.password.encrypted.binary",
+                       content,
+                       sizeof(content),
+                       "/clientSASLWithPasswordEncryptedBinary",
+                       binaryPasswordCallback);
+    }
+
     void testClientSASLOverIPv6() {
         const char *ipAndPort = "::1:22181";