summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLinus Walleij <linus@foobar.localdomain>2010-02-20 19:47:24 +0100
committerSamuel Lidén Borell <samuel@slbdata.se>2010-02-20 20:38:05 +0100
commit2605e8dd6ffd0b8cc710a3f91d58192cd510473a (patch)
tree2a9e5a2ad6d93b36c39d0666ec31e0b23a58b7ef
parent63ff9686cde62f406601483c85ab7759a3b4a429 (diff)
downloadfribid-2605e8dd6ffd0b8cc710a3f91d58192cd510473a.tar.gz
fribid-2605e8dd6ffd0b8cc710a3f91d58192cd510473a.tar.bz2
fribid-2605e8dd6ffd0b8cc710a3f91d58192cd510473a.zip
Provide a secure memory pool and use it
This implements a secure memory pool for use in fribid, allocating a page at a time for secure use. We currently only use one page for the passphrase, but an arbitrary number of pages can be made available. We currently don't need more intelligence than this. The page pool is mmap():ed in to make sure it's on even pages, and the entire pool is then mlock():ed to hinder it from being spooled out as swap. This also augments the platform_sign function so that it takes the piece of (secure) memory used to store the password as a parameter, and copied the passphrase into it as soon as it's retrieved, so that it is not allocated or passed around elsewhere. Signed-off-by: Linus Walleij <linus.ml.walleij@gmail.com>
-rw-r--r--client/Makefile5
-rw-r--r--client/gtk.c17
-rw-r--r--client/main.c48
-rw-r--r--client/platform.h3
-rw-r--r--client/secmem.c160
-rw-r--r--client/secmem.h35
6 files changed, 245 insertions, 23 deletions
diff --git a/client/Makefile b/client/Makefile
index 1088c86..76f643a 100644
--- a/client/Makefile
+++ b/client/Makefile
@@ -34,7 +34,7 @@ UI_PATH=`../configure --internal--get-define=UI_PATH`
SIGNING_EXECUTABLE=`../configure --internal--get-define=SIGNING_EXECUTABLE`
UI_GTK_XML=`../configure --internal--get-define=UI_GTK_XML`
-OBJECTS=bankid.o keyfile.o main.o misc.o pipe.o posix.o glibconfig.o gtk.o xmldsig.o
+OBJECTS=bankid.o keyfile.o main.o misc.o pipe.o posix.o glibconfig.o gtk.o xmldsig.o secmem.o
all: sign
@@ -42,11 +42,12 @@ bankid.o: ../common/biderror.h bankid.h keyfile.h misc.h xmldsig.h
keyfile.o: keyfile.h misc.h platform.h
glibconfig.o: platform.h
gtk.o: ../common/biderror.h platform.h bankid.h keyfile.h
-main.o: ../common/biderror.h bankid.h misc.h platform.h ../common/pipe.h
+main.o: ../common/biderror.h bankid.h misc.h secmem.h platform.h ../common/pipe.h
misc.o: misc.h
pipe.o: ../common/pipe.h ../common/pipe.c
posix.o: platform.h
xmldsig.o: xmldsig.h keyfile.h misc.h
+secmem.o: secmem.h
.c.o:
$(CC) $(CCFLAGS) -c $< -o $@
diff --git a/client/gtk.c b/client/gtk.c
index c148dbd..b3df837 100644
--- a/client/gtk.c
+++ b/client/gtk.c
@@ -343,9 +343,14 @@ static void selectExternalFile() {
* Waits for the user to fill in the dialog, and loads the P12 file for
* the selected subject.
*/
-bool platform_sign(char **signature, int *siglen, KeyfileSubject **person, char **password) {
+bool platform_sign(char **signature, int *siglen, KeyfileSubject **person,
+ char **password, int password_maxlen) {
guint response;
-
+
+ // Restrict the password to the length of the preallocated
+ // password buffer
+ gtk_entry_set_max_length(passwordEntry, password_maxlen-1);
+
while ((response = gtk_dialog_run(signDialog)) == RESPONSE_EXTERNAL) {
// User pressed "External file..."
selectExternalFile();
@@ -373,11 +378,11 @@ bool platform_sign(char **signature, int *siglen, KeyfileSubject **person, char
free(filename);
}
- // The contents of the text field is automatically cleared when the
- // GtkEntry widget is destroyed, so the password won't stay in memory.
- *password = strdup(gtk_entry_get_text(passwordEntry));
+ // Copy the password to the secure buffer
+ strncpy(*password, gtk_entry_get_text(passwordEntry), password_maxlen-1);
+ // Be sure to terminate this under all circumstances
+ *password[password_maxlen-1] = '\0';
return true;
-
} else {
// User pressed cancel or closed the dialog
return false;
diff --git a/client/main.c b/client/main.c
index 4e48927..d517093 100644
--- a/client/main.c
+++ b/client/main.c
@@ -26,13 +26,13 @@
#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
-#include <sys/mman.h> // For mlock()
#include "../common/defines.h"
#include "../common/pipe.h"
#include "bankid.h"
#include "platform.h"
#include "misc.h"
+#include "secmem.h"
static const char version[] = PACKAGEVERSION;
static unsigned long browserWindowId = PLATFORM_NO_WINDOW;
@@ -86,10 +86,22 @@ void pipeData() {
int p12Length;
KeyfileSubject *person;
char *password = NULL;
+ long password_maxsize = 0;
char *signature = NULL;
char *decodedSubjectFilter = NULL;
error = BIDERR_UserCancel;
-
+
+ // Allocate a secure page for the password
+ password = secmem_get_page(&password_maxsize);
+ if (!password || !password_maxsize) {
+ pipe_sendInt(stdout, BIDERR_InternalError);
+ pipe_sendString(stdout, "Out of secure memory!\n");
+ pipe_flush(stdout);
+
+ platform_leaveMainloop();
+ return;
+ }
+
if (subjectFilter) {
decodedSubjectFilter = base64_decode(subjectFilter);
free(subjectFilter);
@@ -108,11 +120,8 @@ void pipeData() {
if (bankid_versionHasExpired()) {
platform_versionExpiredError();
}
-
- while (platform_sign(&p12Data, &p12Length, &person, &password)) {
- // Lock the password memory to RAM so it cannot be spooled out to swap
- mlock(password, strlen(password));
+ while (platform_sign(&p12Data, &p12Length, &person, &password, password_maxsize)) {
// Try to authenticate/sign
if (command == PMC_Authenticate) {
error = bankid_authenticate(p12Data, p12Length, person, password,
@@ -126,16 +135,16 @@ void pipeData() {
free(p12Data);
keyfile_freeSubject(person);
- memset(password, 0, strlen(password));
- munlock(password, strlen(password));
- free(password);
+ memset(password, 0, password_maxsize);
if (error == BIDERR_OK) break;
platform_signError();
error = BIDERR_UserCancel;
}
-
+
+ secmem_free_page(password);
+
platform_endSign();
free(message);
@@ -182,7 +191,13 @@ int main(int argc, char **argv) {
if (process_non_ui_args(argc, argv)) {
return 0;
}
-
+
+ error = secmem_init_pool();
+ if (error) {
+ fprintf(stderr, BINNAME ": could not initialize secure memory");
+ return 2;
+ }
+
platform_init(&argc, &argv);
bankid_init();
@@ -208,18 +223,23 @@ int main(int argc, char **argv) {
}
}
- if (error) return 2;
-
+ if (error) {
+ secmem_destroy_pool();
+ return 2;
+ }
+
/* Set up pipe */
if (ipc) {
platform_setupPipe(pipeData);
} else {
fprintf(stderr, "This is an internal program.\n");
+ secmem_destroy_pool();
return 2;
}
platform_mainloop();
-
+
+ secmem_destroy_pool();
bankid_shutdown();
return 0;
}
diff --git a/client/platform.h b/client/platform.h
index 151af36..d2231e7 100644
--- a/client/platform.h
+++ b/client/platform.h
@@ -93,7 +93,8 @@ void platform_startSign(const char *url, const char *hostname, const char *ip,
const char *subjectFilter, unsigned long parentWindowId);
void platform_endSign();
void platform_setMessage(const char *message);
-bool platform_sign(char **signature, int *siglen, char **person, char **password);
+bool platform_sign(char **signature, int *siglen, char **person,
+ char **password, int password_maxlen);
void platform_signError();
void platform_versionExpiredError();
diff --git a/client/secmem.c b/client/secmem.c
new file mode 100644
index 0000000..11928a6
--- /dev/null
+++ b/client/secmem.c
@@ -0,0 +1,160 @@
+/*
+
+ Copyright (c) 2010 Linus Walleij <triad@df.lth.se>
+
+ Permission is hereby granted, free of charge, to any person obtaining a copy
+ of this software and associated documentation files (the "Software"), to deal
+ in the Software without restriction, including without limitation the rights
+ to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ copies of the Software, and to permit persons to whom the Software is
+ furnished to do so, subject to the following conditions:
+
+ The above copyright notice and this permission notice shall be included in
+ all copies or substantial portions of the Software.
+
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ THE SOFTWARE.
+
+*/
+
+#define _BSD_SOURCE 1
+#include <stdbool.h>
+#include <string.h>
+#include <unistd.h> // For sysconf()
+#include <sys/mman.h> // For mmap()/mlock() etc
+
+#include "secmem.h"
+
+/*
+ * This is a rather straight-forward secure memory
+ * implementation, i.e. a way to retrieve a pointer to
+ * some memory that will certainly NOT be swapped out
+ * to disk when memory gets crowded. Feel free to expand
+ * this by layering and entire sec_[malloc,realloc,free]
+ * interface with some granularity over this.
+ */
+#define SECPAGES 2
+static int pageindex[SECPAGES];
+static long pagesize = 0;
+static char *pool = NULL;
+static long poolsize = 0;
+
+/**
+ * Initialize the secure memory pool, map and lock
+ * it down
+ * @return false on success, true means "error"
+ */
+bool secmem_init_pool(void)
+{
+ int err;
+ int i;
+
+ // Make sure we weren't called before
+ if (pool)
+ return true;
+
+ // Find out what the size of a page is on this system
+#ifdef _SC_PAGESIZE
+ pagesize = sysconf(_SC_PAGESIZE);
+#else
+ #warning "Cannot determine page size for secure memory"
+ pagesize = 4096;
+#endif
+ if (pagesize < 512)
+ return true;
+
+ poolsize = pagesize * SECPAGES;
+
+ // Allocate a secure memory pool, mmap call explained
+ // inline. We map something anonymous, for reading and
+ // writing.
+ pool = mmap (0, poolsize,
+ PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS,
+ -1, 0);
+ if (pool == MAP_FAILED)
+ return true;
+
+ // Lock this pool from any swapping!
+ err = mlock(pool, poolsize);
+ if (err) {
+ munmap(pool, poolsize);
+ pool = NULL;
+ return true;
+ }
+
+ // Mark all pages as free
+ for (i = 0; i < SECPAGES; i++)
+ pageindex[i] = 0;
+
+ return false;
+}
+
+/**
+ * Get a page of secure memory
+ * @page_size: a pointer to a variable that will hold
+ * the size of the returned page on successful return
+ * @return: a pointer to the secure page or NULL on failure
+ */
+char *secmem_get_page(long *page_size)
+{
+ int i;
+
+ *page_size = 0;
+ if (!pool)
+ return NULL;
+
+ // Locate a free page
+ i = 0;
+ while (i < SECPAGES && pageindex[i] != 0)
+ i++;
+ // All pages taken
+ if (i == SECPAGES)
+ return NULL;
+ // Take this page
+ pageindex[i] = 1;
+ *page_size = pagesize;
+ // Return a pointer to it
+ return pool + (pagesize * i);
+}
+
+/**
+ * Free a page of secure memory
+ * @page: page to be free:d, illegal page pointers will be ignored
+ */
+void secmem_free_page(char *page)
+{
+ int i;
+
+ // Bogus pointers will not match and are ignored
+ for (i = 0; i < SECPAGES; i++) {
+ if (pool + (pagesize * i) == page) {
+ pageindex[i] = 0;
+ memset(page, 0, pagesize);
+ break;
+ }
+ }
+ return;
+}
+
+/**
+ * Destroy the secure memory pool
+ */
+void secmem_destroy_pool(void)
+{
+ int i;
+
+ if (!pool)
+ return;
+ for (i = 0; i < SECPAGES; i++)
+ pageindex[i] = 0;
+ memset(pool, 0, poolsize);
+ munmap(pool, poolsize);
+ poolsize = 0;
+ pool = NULL;
+}
diff --git a/client/secmem.h b/client/secmem.h
new file mode 100644
index 0000000..4404984
--- /dev/null
+++ b/client/secmem.h
@@ -0,0 +1,35 @@
+/*
+
+ Copyright (c) 2010 Linus Walleij <triad@df.lth.se>
+
+ Permission is hereby granted, free of charge, to any person obtaining a copy
+ of this software and associated documentation files (the "Software"), to deal
+ in the Software without restriction, including without limitation the rights
+ to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ copies of the Software, and to permit persons to whom the Software is
+ furnished to do so, subject to the following conditions:
+
+ The above copyright notice and this permission notice shall be included in
+ all copies or substantial portions of the Software.
+
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ THE SOFTWARE.
+
+*/
+
+#ifndef __SECMEM_H__
+#define __SECMEM_H__
+
+bool secmem_init_pool(void);
+char *secmem_get_page(long *page_size);
+void secmem_free_page(char *page);
+void secmem_destroy_pool(void);
+
+#endif
+
+