From b12be54a68a7738d948d866eb7b9231f8e55a12e Mon Sep 17 00:00:00 2001 Message-Id: From: Mark Wooding Date: Tue, 18 Dec 2007 17:04:00 +0000 Subject: [PATCH] server side support for cookies, basic tests Organization: Straylight/Edgeware From: Richard Kettlewell --- lib/Makefile.am | 1 + lib/configuration.c | 4 + lib/configuration.h | 6 + lib/cookies.c | 250 ++++++++++++++++++++++++++++++++++++++++++ lib/cookies.h | 40 +++++++ lib/kvp.c | 18 +++ lib/kvp.h | 2 + python/disorder.py.in | 29 ++++- server/Makefile.am | 1 + server/disorderd.c | 3 + server/server.c | 101 ++++++++++++++--- tests/Makefile.am | 2 +- tests/cookie.py | 56 ++++++++++ tests/dtest.py | 7 +- 14 files changed, 491 insertions(+), 29 deletions(-) create mode 100644 lib/cookies.c create mode 100644 lib/cookies.h create mode 100755 tests/cookie.py diff --git a/lib/Makefile.am b/lib/Makefile.am index cf53878..13bafc6 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -30,6 +30,7 @@ libdisorder_a_SOURCES=charset.c charset.h \ client.c client.h \ client-common.c client-common.h \ configuration.c configuration.h \ + cookies.c cookies.h \ defs.c defs.h \ eclient.c eclient.h \ event.c event.h \ diff --git a/lib/configuration.c b/lib/configuration.c index 2dcf8f9..3ddfea1 100644 --- a/lib/configuration.c +++ b/lib/configuration.c @@ -899,6 +899,8 @@ static const struct conf conf[] = { { C(checkpoint_min), &type_integer, validate_non_negative }, { C(collection), &type_collections, validate_any }, { C(connect), &type_stringlist, validate_addrport }, + { C(cookie_login_lifetime), &type_integer, validate_positive }, + { C(cookie_key_lifetime), &type_integer, validate_positive }, { C(dbversion), &type_integer, validate_positive }, { C(device), &type_string, validate_any }, { C(gap), &type_integer, validate_non_negative }, @@ -1058,6 +1060,8 @@ static struct config *config_default(void) { c->mixer = xstrdup("/dev/mixer"); c->channel = xstrdup("pcm"); c->dbversion = 2; + c->cookie_login_lifetime = 86400; + c->cookie_key_lifetime = 86400 * 7; return c; } diff --git a/lib/configuration.h b/lib/configuration.h index 02c953a..eb028b0 100644 --- a/lib/configuration.h +++ b/lib/configuration.h @@ -245,6 +245,12 @@ struct config { /** @brief Whether to loop back multicast packets */ int multicast_loop; + + /** @brief Login lifetime in seconds */ + long cookie_login_lifetime; + + /** @brief Signing key lifetime in seconds */ + long cookie_key_lifetime; /* derived values: */ int nparts; /* number of distinct name parts */ diff --git a/lib/cookies.c b/lib/cookies.c new file mode 100644 index 0000000..4ac9f65 --- /dev/null +++ b/lib/cookies.c @@ -0,0 +1,250 @@ +/* + * This file is part of DisOrder + * Copyright (C) 2007 Richard Kettlewell + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 + * USA + */ +/** @file lib/cookies.c + * @brief Cookie support + */ + +#include +#include "types.h" + +#include +#include +#include +#include +#include +#include + +#include "cookies.h" +#include "hash.h" +#include "mem.h" +#include "log.h" +#include "printf.h" +#include "mime.h" +#include "configuration.h" +#include "kvp.h" + +/** @brief Hash function used in signing HMAC */ +#define ALGO GCRY_MD_SHA1 + +/** @brief Size of key to use */ +#define HASHSIZE 20 + +/** @brief Signing key */ +static uint8_t signing_key[HASHSIZE]; + +/** @brief Previous signing key */ +static uint8_t old_signing_key[HASHSIZE]; + +/** @brief Signing key validity limit or 0 if none */ +static time_t signing_key_validity_limit; + +/** @brief Hash of revoked cookies */ +static hash *revoked; + +/** @brief Callback to expire revocation list */ +static int revoked_cleanup_callback(const char *key, void *value, + void *u) { + if(*(time_t *)value < *(time_t *)u) + hash_remove(revoked, key); + return 0; +} + +/** @brief Generate a new key */ +static void newkey(void) { + time_t now; + + time(&now); + memcpy(old_signing_key, signing_key, HASHSIZE); + gcry_randomize(signing_key, HASHSIZE, GCRY_STRONG_RANDOM); + signing_key_validity_limit = now + config->cookie_key_lifetime; + /* Now is a good time to clean up the revocation list... */ + if(revoked) + hash_foreach(revoked, revoked_cleanup_callback, &now); +} + +/** @brief Sign @p subject with @p key and return the base64 of the result + * @param key Key to sign with (@ref HASHSIZE bytes) + * @param subject Subject string + * @return Base64-encoded signature or NULL + */ +static char *sign(const uint8_t *key, + const char *subject) { + gcry_error_t e; + gcry_md_hd_t h; + uint8_t *sig; + char *sig64; + + if((e = gcry_md_open(&h, ALGO, GCRY_MD_FLAG_HMAC))) { + error(0, "gcry_md_open: %s", gcry_strerror(e)); + return 0; + } + if((e = gcry_md_setkey(h, key, HASHSIZE))) { + error(0, "gcry_md_setkey: %s", gcry_strerror(e)); + gcry_md_close(h); + return 0; + } + gcry_md_write(h, subject, strlen(subject)); + sig = gcry_md_read(h, ALGO); + sig64 = mime_to_base64(sig, HASHSIZE); + gcry_md_close(h); + return sig64; +} + +/** @brief Create a login cookie + * @param user Username + * @return Cookie or NULL + */ +char *make_cookie(const char *user) { + char *password; + time_t now; + char *b, *bp, *c, *g; + int n; + + /* semicolons aren't allowed in usernames */ + if(strchr(user, ';')) { + error(0, "make_cookie for username with semicolon"); + return 0; + } + /* look up the password */ + for(n = 0; n < config->allow.n + && strcmp(config->allow.s[n].s[0], user); ++n) + ; + if(n >= config->allow.n) { + error(0, "make_cookie for nonexistent user"); + return 0; + } + password = config->allow.s[n].s[1]; + /* make sure we have a valid signing key */ + time(&now); + if(now >= signing_key_validity_limit) + newkey(); + /* construct the subject */ + byte_xasprintf(&b, "%jx;%s;", (intmax_t)now + config->cookie_login_lifetime, + urlencodestring(user)); + byte_xasprintf(&bp, "%s%s", b, password); + /* sign it */ + if(!(g = sign(signing_key, bp))) + return 0; + /* put together the final cookie */ + byte_xasprintf(&c, "%s%s", b, g); + return c; +} + +/** @brief Verify a cookie + * @param cookie Cookie to verify + * @return Verified user or NULL + */ +char *verify_cookie(const char *cookie) { + char *c1, *c2; + intmax_t t; + time_t now; + char *user, *bp, *password, *sig; + int n; + + /* check the revocation list */ + if(revoked && hash_find(revoked, cookie)) { + error(0, "attempt to log in with revoked cookie"); + return 0; + } + /* parse the cookie */ + errno = 0; + t = strtoimax(cookie, &c1, 16); + if(errno) { + error(errno, "error parsing cookie timestamp"); + return 0; + } + if(*c1 != ';') { + error(0, "invalid cookie timestamp"); + return 0; + } + /* There'd better be two semicolons */ + c2 = strchr(c1 + 1, ';'); + if(c2 == 0) { + error(0, "invalid cookie syntax"); + return 0; + } + /* Extract the username */ + user = xstrndup(c1 + 1, c2 - (c1 + 1)); + /* check expiry */ + time(&now); + if(now >= t) { + error(0, "cookie has expired"); + return 0; + } + /* look up the password */ + for(n = 0; n < config->allow.n + && strcmp(config->allow.s[n].s[0], user); ++n) + ; + if(n >= config->allow.n) { + error(0, "verify_cookie for nonexistent user"); + return 0; + } + password = config->allow.s[n].s[1]; + /* construct the expected subject. We re-encode the timestamp and the + * password. */ + byte_xasprintf(&bp, "%jx;%s;%s", t, urlencodestring(user), password); + /* Compute the expected signature. NB we base64 the expected signature and + * compare that rather than exposing our base64 parser to the cookie. */ + if(!(sig = sign(signing_key, bp))) + return 0; + if(!strcmp(sig, c2 + 1)) + return user; + /* that didn't match, try the old key */ + if(!(sig = sign(old_signing_key, bp))) + return 0; + if(!strcmp(sig, c2 + 1)) + return user; + /* that didn't match either */ + error(0, "cookie signature does not match"); + return 0; +} + +/** @brief Revoke a cookie + * @param cookie Cookie to revoke + * + * Further attempts to log in with @p cookie will fail. + */ +void revoke_cookie(const char *cookie) { + time_t when; + char *ptr; + + /* find the cookie's expiry time */ + errno = 0; + when = (time_t)strtoimax(cookie, &ptr, 16); + /* reject bogus cookies */ + if(errno) + return; + if(*ptr != ';') + return; + /* make sure the revocation list exists */ + if(!revoked) + revoked = hash_new(sizeof(time_t)); + /* add the cookie to it; its value is the expiry time */ + hash_add(revoked, cookie, &when, HASH_INSERT); +} + +/* +Local Variables: +c-basic-offset:2 +comment-column:40 +fill-column:79 +indent-tabs-mode:nil +End: +*/ diff --git a/lib/cookies.h b/lib/cookies.h new file mode 100644 index 0000000..f7e3a3a --- /dev/null +++ b/lib/cookies.h @@ -0,0 +1,40 @@ +/* + * This file is part of DisOrder + * Copyright (C) 2007 Richard Kettlewell + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 + * USA + */ +/** @file lib/cookies.h + * @brief Cookie support + */ + +#ifndef COOKIES_H +#define COOKIES_H + +char *make_cookie(const char *user); +char *verify_cookie(const char *cookie); +void revoke_cookie(const char *cookie); + +#endif /* COOKIES_H */ + +/* +Local Variables: +c-basic-offset:2 +comment-column:40 +fill-column:79 +indent-tabs-mode:nil +End: +*/ diff --git a/lib/kvp.c b/lib/kvp.c index 6c18a8f..471de78 100644 --- a/lib/kvp.c +++ b/lib/kvp.c @@ -124,6 +124,10 @@ int urlencode(struct sink *sink, const char *s, size_t n) { return 0; } +/** @brief URL-encode @p s + * @param s String to encode + * @return Encoded string + */ const char *urlencodestring(const char *s) { struct dynstr d; @@ -133,6 +137,20 @@ const char *urlencodestring(const char *s) { return d.vec; } +/** @brief URL-decode @p s + * @param s String to decode + * @param ns Length of string + * @return Decoded string + */ +const char *urldecodestring(const char *s, size_t ns) { + struct dynstr d; + + dynstr_init(&d); + urldecode(sink_dynstr(&d), s, ns); + dynstr_terminate(&d); + return d.vec; +} + char *kvp_urlencode(const struct kvp *kvp, size_t *np) { struct dynstr d; struct sink *sink; diff --git a/lib/kvp.h b/lib/kvp.h index db6d177..98c62cd 100644 --- a/lib/kvp.h +++ b/lib/kvp.h @@ -55,6 +55,8 @@ int urlencode(struct sink *sink, const char *s, size_t n); const char *urlencodestring(const char *s); /* return the url-encoded form of @s@ */ +const char *urldecodestring(const char *s, size_t ns); + #endif /* KVP_H */ /* diff --git a/python/disorder.py.in b/python/disorder.py.in index 0f16c1a..c501be5 100644 --- a/python/disorder.py.in +++ b/python/disorder.py.in @@ -327,8 +327,10 @@ class client: sys.stderr.write("\n") sys.stderr.flush() - def connect(self): - """Connect to the DisOrder server and authenticate. + def connect(self, cookie=None): + """c.connect(cookie=None) + + Connect to the DisOrder server and authenticate. Raises communicationError if connection fails and operationError if authentication fails (in which case disconnection is automatic). @@ -339,6 +341,9 @@ class client: Other operations automatically connect if we're not already connected, so it is not strictly necessary to call this method. + + If COOKIE is specified then that is used to log in instead of + the username/password. """ if self.state == 'disconnected': try: @@ -369,10 +374,13 @@ class client: self.w = s.makefile("wb") self.r = s.makefile("rb") (res, challenge) = self._simple() - h = sha.sha() - h.update(self.config['password']) - h.update(binascii.unhexlify(challenge)) - self._simple("user", self.config['username'], h.hexdigest()) + if cookie is None: + h = sha.sha() + h.update(self.config['password']) + h.update(binascii.unhexlify(challenge)) + self._simple("user", self.config['username'], h.hexdigest()) + else: + self._simple("cookie", cookie) self.state = 'connected' except socket.error, e: self._disconnect() @@ -833,6 +841,15 @@ class client: else: return details + def make_cookie(self): + """Create a login cookie""" + ret, details = self._simple("make-cookie") + return details + + def revoke(self): + """Revoke a login cookie""" + self._simple("revoke") + ######################################################################## # I/O infrastructure diff --git a/server/Makefile.am b/server/Makefile.am index eae90c6..4332ed4 100644 --- a/server/Makefile.am +++ b/server/Makefile.am @@ -144,6 +144,7 @@ check-decode: disorder-decode disorder-normalize sox ${top_srcdir}/sounds/scratch.ogg scratch.wav ./disorder-decode scratch.wav | \ ./disorder-normalize --config config > decoded.raw + ls -l *.raw cmp decoded.raw oggdec.raw rm -f scratch.wav config decoded.raw oggdec.raw diff --git a/server/disorderd.c b/server/disorderd.c index ba18c23..34d9e23 100644 --- a/server/disorderd.c +++ b/server/disorderd.c @@ -38,6 +38,7 @@ #include #include #include +#include #include "daemonize.h" #include "event.h" @@ -238,6 +239,8 @@ int main(int argc, char **argv) { info("process ID %lu", (unsigned long)getpid()); fix_path(); srand(time(0)); /* don't start the same every time */ + /* gcrypt initialization */ + gcry_control(GCRYCTL_INIT_SECMEM, 1); /* create event loop */ ev = ev_new(); if(ev_child_setup(ev)) fatal(0, "ev_child_setup failed"); diff --git a/server/server.c b/server/server.c index a369964..cd97435 100644 --- a/server/server.c +++ b/server/server.c @@ -64,6 +64,7 @@ #include "defs.h" #include "cache.h" #include "unicode.h" +#include "cookies.h" #ifndef NONCE_SIZE # define NONCE_SIZE 16 @@ -107,6 +108,8 @@ struct conn { struct eventlog_output *lo; /** @brief Parent listener */ const struct listener *l; + /** @brief Login cookie or NULL */ + char *cookie; }; static int reader_callback(ev_source *ev, @@ -370,39 +373,48 @@ static int c_become(struct conn *c, return 1; } -static int c_user(struct conn *c, - char **vec, - int attribute((unused)) nvec) { - int n; - const char *res; +static const char *connection_host(struct conn *c) { union { struct sockaddr sa; struct sockaddr_in in; struct sockaddr_in6 in6; } u; socklen_t l; + int n; char host[1024]; - if(c->who) { - sink_writes(ev_writer_sink(c->w), "530 already authenticated\n"); - return 1; - } /* get connection data */ l = sizeof u; if(getpeername(c->fd, &u.sa, &l) < 0) { error(errno, "S%x error calling getpeername", c->tag); - sink_writes(ev_writer_sink(c->w), "530 authentication failure\n"); - return 1; + return 0; } if(c->l->pf != PF_UNIX) { if((n = getnameinfo(&u.sa, l, host, sizeof host, 0, 0, NI_NUMERICHOST))) { error(0, "S%x error calling getnameinfo: %s", c->tag, gai_strerror(n)); - sink_writes(ev_writer_sink(c->w), "530 authentication failure\n"); - return 1; + return 0; } + return xstrdup(host); } else - strcpy(host, "local"); + return "local"; +} + +static int c_user(struct conn *c, + char **vec, + int attribute((unused)) nvec) { + int n; + const char *res, *host; + + if(c->who) { + sink_writes(ev_writer_sink(c->w), "530 already authenticated\n"); + return 1; + } + /* get connection data */ + if(!(host = connection_host(c))) { + sink_writes(ev_writer_sink(c->w), "530 authentication failure\n"); + return 1; + } /* find the user */ for(n = 0; n < config->allow.n && strcmp(config->allow.s[n].s[0], vec[0]); ++n) @@ -418,7 +430,7 @@ static int c_user(struct conn *c, if(wideopen || (res && !strcmp(res, vec[1]))) { c->who = vec[0]; /* currently we only bother logging remote connections */ - if(c->l->pf != PF_UNIX) + if(strcmp(host, "local")) info("S%x %s connected from %s", c->tag, vec[0], host); sink_writes(ev_writer_sink(c->w), "230 OK\n"); return 1; @@ -962,7 +974,61 @@ static int c_rtp_address(struct conn *c, sink_writes(ev_writer_sink(c->w), "550 No RTP\n"); return 1; } - + +static int c_cookie(struct conn *c, + char **vec, + int attribute((unused)) nvec) { + const char *host; + char *user; + + /* Can't log in twice on the same connection */ + if(c->who) { + sink_writes(ev_writer_sink(c->w), "530 already authenticated\n"); + return 1; + } + /* Get some kind of peer identifcation */ + if(!(host = connection_host(c))) { + sink_writes(ev_writer_sink(c->w), "530 authentication failure\n"); + return 1; + } + /* Check the cookie */ + user = verify_cookie(vec[0]); + if(!user) { + sink_writes(ev_writer_sink(c->w), "530 authentication failure\n"); + return 1; + } + /* Log in */ + c->who = user; + c->cookie = vec[0]; + if(strcmp(host, "local")) + info("S%x %s connected with cookie from %s", c->tag, user, host); + sink_writes(ev_writer_sink(c->w), "230 OK\n"); + return 1; +} + +static int c_make_cookie(struct conn *c, + char attribute((unused)) **vec, + int attribute((unused)) nvec) { + const char *cookie = make_cookie(c->who); + + if(cookie) + sink_printf(ev_writer_sink(c->w), "252 %s\n", cookie); + else + sink_writes(ev_writer_sink(c->w), "550 Cannot create cookie\n"); + return 1; +} + +static int c_revoke(struct conn *c, + char attribute((unused)) **vec, + int attribute((unused)) nvec) { + if(c->cookie) { + revoke_cookie(c->cookie); + sink_writes(ev_writer_sink(c->w), "250 OK\n"); + } else + sink_writes(ev_writer_sink(c->w), "550 Did not log in with cookie\n"); + return 1; +} + #define C_AUTH 0001 /* must be authenticated */ #define C_TRUSTED 0002 /* must be trusted user */ @@ -974,6 +1040,7 @@ static const struct command { } commands[] = { { "allfiles", 0, 2, c_allfiles, C_AUTH }, { "become", 1, 1, c_become, C_AUTH|C_TRUSTED }, + { "cookie", 1, 1, c_cookie, 0 }, { "dirs", 0, 2, c_dirs, C_AUTH }, { "disable", 0, 1, c_disable, C_AUTH }, { "enable", 0, 0, c_enable, C_AUTH }, @@ -984,6 +1051,7 @@ static const struct command { { "get-global", 1, 1, c_get_global, C_AUTH }, { "length", 1, 1, c_length, C_AUTH }, { "log", 0, 0, c_log, C_AUTH }, + { "make-cookie", 0, 0, c_make_cookie, C_AUTH }, { "move", 2, 2, c_move, C_AUTH }, { "moveafter", 1, INT_MAX, c_moveafter, C_AUTH }, { "new", 0, 1, c_new, C_AUTH }, @@ -1003,6 +1071,7 @@ static const struct command { { "rescan", 0, 0, c_rescan, C_AUTH|C_TRUSTED }, { "resolve", 1, 1, c_resolve, C_AUTH }, { "resume", 0, 0, c_resume, C_AUTH }, + { "revoke", 0, 0, c_revoke, C_AUTH }, { "rtp-address", 0, 0, c_rtp_address, C_AUTH }, { "scratch", 0, 1, c_scratch, C_AUTH }, { "search", 1, 1, c_search, C_AUTH }, diff --git a/tests/Makefile.am b/tests/Makefile.am index 2cd5e63..9527a55 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -30,4 +30,4 @@ check: ${PYTHON} ${srcdir}/alltests EXTRA_DIST=alltests dtest.py dbversion.py search.py \ - queue.py dump.py play.py + queue.py dump.py play.py cookie.py diff --git a/tests/cookie.py b/tests/cookie.py new file mode 100755 index 0000000..7f00454 --- /dev/null +++ b/tests/cookie.py @@ -0,0 +1,56 @@ +#! /usr/bin/env python +# +# This file is part of DisOrder. +# Copyright (C) 2007 Richard Kettlewell +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 +# USA +# +import dtest,disorder + +def test(): + """Exercise cookie protocol""" + dtest.start_daemon() + print " connecting" + c = disorder.client() + v = c.version() + print " getting cookie" + k = c.make_cookie() + print " connecting with cookie" + c = disorder.client() + c.connect(k) + v = c.version() + print " it worked" + print " connecting with cookie again" + c = disorder.client() + c.connect(k) + v = c.version() + print " it worked" + print " revoking cookie" + c.revoke() + v = c.version() + print " connection still works" + print " connecting with revoked cookie" + c = disorder.client() + try: + c.connect(k) + print "*** should not be able to connect with revoked cookie ***" + assert False + except disorder.operationError: + pass # good + print " revoked cookie was rejected" + +if __name__ == '__main__': + dtest.run() diff --git a/tests/dtest.py b/tests/dtest.py index cd87c50..ef25fc4 100644 --- a/tests/dtest.py +++ b/tests/dtest.py @@ -300,12 +300,7 @@ def run(module=None, report=True): # Create some standard tracks stdtracks() try: - try: - module.test() - except AssertionError, e: - global failures - failures += 1 - print "assertion failed: %s" % e.message + module.test() finally: stop_daemon() if report: -- [mdw]