From 2129f5ddf65db4ee015263a3b3c0c4f4fef941fb Mon Sep 17 00:00:00 2001
From: ale <ale@incal.net>
Date: Thu, 19 Jun 2014 09:34:53 +0100
Subject: [PATCH] move buffer-management code to its own module, with tests;
fix memory corruption bug in quote()
---
pam/.gitignore | 3 +++
pam/Makefile.am | 9 ++++++--
pam/auth_client.c | 46 +++++------------------------------------
pam/auth_client_test.cc | 19 +++++++++++++++--
pam/cbuf.c | 23 +++++++++++++++++++++
pam/cbuf.h | 21 +++++++++++++++++++
pam/cbuf_test.cc | 23 +++++++++++++++++++++
pam/configure.ac | 2 +-
8 files changed, 100 insertions(+), 46 deletions(-)
create mode 100644 pam/cbuf.c
create mode 100644 pam/cbuf.h
create mode 100644 pam/cbuf_test.cc
diff --git a/pam/.gitignore b/pam/.gitignore
index 204f95f..cb79b5c 100644
--- a/pam/.gitignore
+++ b/pam/.gitignore
@@ -1,6 +1,7 @@
*.in
aclocal.m4
autom4te.cache
+compile
config.guess
config.sub
configure
@@ -26,5 +27,7 @@ libtool
stamp-*
Makefile
auth_client_test
+cbuf_test
test-driver
test-suite.log
+.dirstamp
diff --git a/pam/Makefile.am b/pam/Makefile.am
index 87da913..3dfff69 100644
--- a/pam/Makefile.am
+++ b/pam/Makefile.am
@@ -8,7 +8,8 @@ pam_LTLIBRARIES = pam_authclient.la
noinst_LIBRARIES = libgtest.a
libauthclient_la_SOURCES = \
- auth_client.c auth_client.h
+ auth_client.c auth_client.h \
+ cbuf.c cbuf.h
libauthclient_la_includedir = $(includedir)/authclient
libauthclient_la_include_HEADERS = auth_client.h
@@ -18,7 +19,8 @@ pam_authclient_la_LDFLAGS = -module
pam_authclient_la_LIBADD = libauthclient.la
check_PROGRAMS = \
- auth_client_test
+ auth_client_test \
+ cbuf_test
TESTS = $(check_PROGRAMS)
@@ -26,6 +28,9 @@ auth_client_test_CPPFLAGS = $(GTEST_CPPFLAGS)
auth_client_test_LDADD = libauthclient.la libgtest.a
auth_client_test_SOURCES = auth_client_test.cc
+cbuf_test_CPPFLAGS = $(GTEST_CPPFLAGS)
+cbuf_test_LDADD = libauthclient.la libgtest.a
+cbuf_test_SOURCES = cbuf_test.cc
# GTest sources.
diff --git a/pam/auth_client.c b/pam/auth_client.c
index dab6226..373d318 100644
--- a/pam/auth_client.c
+++ b/pam/auth_client.c
@@ -9,6 +9,7 @@
#include <sys/stat.h>
#include <curl/curl.h>
#include "auth_client.h"
+#include "cbuf.h"
#define CURL_CHECK(x) { \
int _err = (x); if (_err != CURLE_OK) { return auth_client_err_from_curl(_err); } \
@@ -106,44 +107,6 @@ const char *auth_client_strerror(int err) {
}
}
-/*
- * A dynamically sized memory buffer that can be appended to, and will
- * grow accordingly. It is optimized to perform well for a specific
- * size (the initial allocation).
- */
-struct cbuf {
- char *buf;
- size_t alloc, size;
-};
-
-static void cbuf_init(struct cbuf *cbuf, size_t alloc) {
- cbuf->buf = (char *)malloc(alloc);
- cbuf->alloc = alloc;
- cbuf->size = 0;
-}
-
-static void cbuf_free(struct cbuf *cbuf) {
- free(cbuf->buf);
-}
-
-static void cbuf_append(struct cbuf *cbuf, void *data, size_t size) {
- // Resize if necessary.
- size_t required_alloc = cbuf->size + size + 1;
- if (required_alloc > cbuf->alloc) {
- size_t new_alloc = cbuf->alloc;
- while (new_alloc < required_alloc) {
- new_alloc *= 2;
- }
- cbuf->buf = (char *)realloc(cbuf->buf, new_alloc);
- cbuf->alloc = new_alloc;
- }
-
- // Append data to the buffer.
- memcpy(cbuf->buf + cbuf->size, data, size);
- cbuf->size += size;
- cbuf->buf[cbuf->size] = '\0';
-}
-
static char *quote(const char *s) {
char *out = (char *)malloc(strlen(s) * 3 + 1), *optr;
for (optr = out; *s; s++) {
@@ -165,6 +128,7 @@ static char *quote(const char *s) {
*optr++ = *s;
}
}
+ *optr = '\0';
return out;
}
@@ -177,14 +141,13 @@ static size_t responsebuf_callback(void *contents, size_t size, size_t nmemb, vo
}
static void post_field_add(struct cbuf *form_data, const char *key, const char *value) {
- char *quoted_key = quote(key), *quoted_value = quote(value);
+ char *quoted_value = quote(value);
if (form_data->size != 0) {
cbuf_append(form_data, "&", 1);
}
- cbuf_append(form_data, quoted_key, strlen(quoted_key));
+ cbuf_append(form_data, key, strlen(key));
cbuf_append(form_data, "=", 1);
cbuf_append(form_data, quoted_value, strlen(quoted_value));
- free(quoted_key);
free(quoted_value);
}
@@ -217,6 +180,7 @@ int auth_client_authenticate(auth_client_t ac,
post_field_add(&form, "shard", shard);
}
curl_easy_setopt(ac->c, CURLOPT_POSTFIELDS, form.buf);
+ curl_easy_setopt(ac->c, CURLOPT_POSTFIELDSIZE, form.size);
// Set request headers.
curl_slist_append(headers, "Content-Type: application/x-form-www-urlencoded");
diff --git a/pam/auth_client_test.cc b/pam/auth_client_test.cc
index b8251e4..b3f0352 100644
--- a/pam/auth_client_test.cc
+++ b/pam/auth_client_test.cc
@@ -8,8 +8,8 @@ extern "C" {
static const char *server = NULL;
-static const char *ssl_ca = "../authserv/test/testca/ca.pem";
-static const char *ssl_cert = "../authserv/test/testca/certs/client.pem";
+static const char *ssl_ca = "../authserv/test/testca/public/ca.pem";
+static const char *ssl_cert = "../authserv/test/testca/public/certs/client.pem";
static const char *ssl_key = "../authserv/test/testca/private/client.key";
static const char *ssl_bad_ca = "../authserv/test/testca-bad/ca.pem";
static const char *ssl_bad_cert = "../authserv/test/testca-bad/certs/client.pem";
@@ -59,6 +59,21 @@ TEST_F(AuthClientTest, AuthOK) {
<< ", server=" << server;
}
+TEST_F(AuthClientTest, ManyAuthOK) {
+ int result;
+
+ for (int i = 0; i < 3; i++) {
+
+ result = auth_client_set_certificate(ac, ssl_ca, ssl_cert, ssl_key);
+ EXPECT_EQ(AC_OK, result) << "set_certificate() error: " << auth_client_strerror(result);
+
+ result = auth_client_authenticate(ac, "user", "pass", NULL, "127.0.0.1", NULL);
+ EXPECT_EQ(AC_OK, result) << "authenticate() error: " << auth_client_strerror(result)
+ << ", server=" << server;
+ }
+
+}
+
TEST_F(AuthClientTest, AuthFail) {
int result;
diff --git a/pam/cbuf.c b/pam/cbuf.c
new file mode 100644
index 0000000..96a0230
--- /dev/null
+++ b/pam/cbuf.c
@@ -0,0 +1,23 @@
+
+#include <stdlib.h>
+#include <memory.h>
+#include "cbuf.h"
+
+
+void cbuf_init(struct cbuf *cbuf, size_t alloc) {
+ cbuf->buf = (char *)malloc(1);
+ cbuf->buf[0] = '\0';
+ cbuf->alloc = cbuf->size = 0;
+}
+
+void cbuf_free(struct cbuf *cbuf) {
+ free(cbuf->buf);
+}
+
+void cbuf_append(struct cbuf *cbuf, const char *data, size_t size) {
+ size_t new_alloc = cbuf->alloc + size;
+ cbuf->buf = (char *)realloc(cbuf->buf, new_alloc + 1);
+ memcpy(cbuf->buf + cbuf->size, data, size);
+ cbuf->alloc = cbuf->size = new_alloc;
+ cbuf->buf[cbuf->size] = '\0';
+}
diff --git a/pam/cbuf.h b/pam/cbuf.h
new file mode 100644
index 0000000..5485490
--- /dev/null
+++ b/pam/cbuf.h
@@ -0,0 +1,21 @@
+#ifndef __libauthclient_cbuf_h
+#define __libauthclient_cbuf_h 1
+
+#include <sys/types.h>
+
+/*
+ * A dynamically sized memory buffer that can be appended to, and will
+ * grow accordingly. It is optimized to perform well for a specific
+ * size (the initial allocation).
+ */
+
+struct cbuf {
+ char *buf;
+ size_t alloc, size;
+};
+
+void cbuf_init(struct cbuf *cbuf, size_t alloc);
+void cbuf_free(struct cbuf *cbuf);
+void cbuf_append(struct cbuf *cbuf, const char *data, size_t size);
+
+#endif
diff --git a/pam/cbuf_test.cc b/pam/cbuf_test.cc
new file mode 100644
index 0000000..81e3306
--- /dev/null
+++ b/pam/cbuf_test.cc
@@ -0,0 +1,23 @@
+// Tests for cbuf.c.
+
+#include <stdlib.h>
+#include <string>
+#include "gtest/gtest.h"
+extern "C" {
+#include "cbuf.h"
+}
+
+TEST(Cbuf, Append) {
+ struct cbuf c;
+ cbuf_init(&c, 0);
+ cbuf_append(&c, "hello, ", 7);
+ cbuf_append(&c, "world", 5);
+ std::string a(c.buf);
+ EXPECT_EQ("hello, world", a);
+ cbuf_free(&c);
+}
+
+int main(int argc, char **argv) {
+ ::testing::InitGoogleTest(&argc, argv);
+ return RUN_ALL_TESTS();
+}
diff --git a/pam/configure.ac b/pam/configure.ac
index 237ce3f..267505c 100644
--- a/pam/configure.ac
+++ b/pam/configure.ac
@@ -2,7 +2,7 @@ AC_INIT([libauthclient], [0.1], [info@autistici.org])
AC_CONFIG_SRCDIR([auth_client.h])
dnl AC_LANG(C)
-AM_INIT_AUTOMAKE([foreign])
+AM_INIT_AUTOMAKE([foreign subdir-objects])
AC_CONFIG_HEADERS(config.h)
AC_CONFIG_MACRO_DIR([m4])
AC_DISABLE_STATIC
--
GitLab