diff --git a/src/cookies.c b/src/cookies.c index 60a90c43..55bcf0de 100644 --- a/src/cookies.c +++ b/src/cookies.c @@ -169,141 +169,3 @@ cleanup: } -void cookie_db_deinit(struct cookie_entry_db_st* db) -{ -struct cookie_entry_st * e; -struct htable_iter iter; - - e = htable_first(db->db, &iter); - while(e != NULL) { - if (e->proc) - e->proc->cookie_ptr = NULL; - - e->proc = NULL; - talloc_free(e); - - e = htable_next(db->db, &iter); - } - htable_clear(db->db); - talloc_free(db->db); - - return; -} - -void expire_cookies(struct cookie_entry_db_st* db) -{ -struct cookie_entry_st * e; -struct htable_iter iter; -time_t now = time(0); - - e = htable_first(db->db, &iter); - while(e != NULL) { - if (e->expiration == -1 || now < e->expiration) - goto cont; - - if (e->proc) { - syslog(LOG_ERR, "found proc that references expired cookie!"); - e->proc->cookie_ptr = NULL; - } - - htable_delval(db->db, &iter); - db->total--; - - talloc_free(e); - cont: - e = htable_next(db->db, &iter); - } - - return; -} - -static size_t rehash(const void* _e, void* unused) -{ -const struct cookie_entry_st * e = _e; - - return hash_any(e->cookie_hash, sizeof(e->cookie_hash), 0); -} - -void cookie_db_init(void *pool, struct cookie_entry_db_st* db) -{ - db->db = talloc(pool, struct htable); - htable_init(db->db, rehash, NULL); - db->total = 0; -} - -static bool cookie_entry_cmp(const void* _c1, void* _c2) -{ -const struct cookie_entry_st* c1 = _c1; -struct cookie_entry_st* c2 = _c2; - - if (memcmp(c1->cookie_hash, c2->cookie_hash, sizeof(c1->cookie_hash)) == 0) - return 1; - - return 0; -} - -struct cookie_entry_st *find_cookie_entry(struct cookie_entry_db_st* db, void *cookie, unsigned cookie_size) -{ - struct cookie_entry_st *e; - struct cookie_entry_st t; - int ret; - - ret = gnutls_hash_fast(COOKIE_HASH, cookie, cookie_size, t.cookie_hash); - if (ret < 0) { - return NULL; - } - - e = htable_get(db->db, hash_any(t.cookie_hash, sizeof(t.cookie_hash), 0), cookie_entry_cmp, &t); - if (e == NULL) - return NULL; - - if (e->expiration != -1 && e->expiration < time(0)) { - delete_cookie(db, e); - return NULL; - } - - return e; -} - -struct cookie_entry_st *new_cookie_entry(struct cookie_entry_db_st* db, proc_st *proc, void *cookie, unsigned cookie_size) -{ - struct cookie_entry_st *t; - int ret; - - t = talloc(db->db, struct cookie_entry_st); - if (t == NULL) - return NULL; - - t->expiration = -1; - - ret = gnutls_hash_fast(COOKIE_HASH, cookie, cookie_size, t->cookie_hash); - if (ret < 0) { - goto fail; - } - - t->proc = proc; - - if (htable_add(db->db, rehash(t, NULL), t) == 0) { - goto fail; - } - - db->total++; - - return t; - - fail: - talloc_free(t); - return NULL; -} - -void delete_cookie(struct cookie_entry_db_st* db, struct cookie_entry_st *e) -{ - if (e->proc) { - syslog(LOG_ERR, "found proc that references cookie to be deleted!"); - e->proc->cookie_ptr = NULL; - } - htable_del(db->db, rehash(e, NULL), e); - db->total--; - talloc_free(e); - return; -} diff --git a/src/cookies.h b/src/cookies.h index 4cc10ad6..b2ecc5ae 100644 --- a/src/cookies.h +++ b/src/cookies.h @@ -37,16 +37,4 @@ int decrypt_cookie(ProtobufCAllocator *pa, gnutls_datum_t *key, uint8_t *cookie, unsigned cookie_size, Cookie **msg); -void cookie_db_init(void *pool, struct cookie_entry_db_st* db); -void cookie_db_deinit(struct cookie_entry_db_st* db); -void expire_cookies(struct cookie_entry_db_st* db); -struct cookie_entry_st *new_cookie_entry(struct cookie_entry_db_st* db, proc_st *proc, void *cookie, unsigned cookie_size); -struct cookie_entry_st *find_cookie_entry(struct cookie_entry_db_st* db, void *cookie, unsigned cookie_len); -void delete_cookie(struct cookie_entry_db_st* db, struct cookie_entry_st *e); - -inline static void revive_cookie(struct cookie_entry_st * e) -{ - e->expiration = -1; -} - #endif diff --git a/src/main-auth.c b/src/main-auth.c index 7ee09042..8cda736e 100644 --- a/src/main-auth.c +++ b/src/main-auth.c @@ -170,11 +170,10 @@ int handle_auth_cookie_req(main_server_st* s, struct proc_st* proc, { int ret; Cookie *cmsg; -time_t now = time(0); gnutls_datum_t key = {s->cookie_key, sizeof(s->cookie_key)}; char str_ip[MAX_IP_STR+1]; PROTOBUF_ALLOCATOR(pa, proc); -struct cookie_entry_st *old = NULL; +struct proc_st *old_proc; if (req->cookie.len == 0) { mslog(s, proc, LOG_INFO, "error in cookie size"); @@ -243,43 +242,25 @@ struct cookie_entry_st *old = NULL; } } - /* check for a valid stored cookie */ - if ((old=find_cookie_entry(&s->cookies, req->cookie.data, req->cookie.len)) != NULL) { - mslog(s, proc, LOG_DEBUG, "reusing cookie for '%s' (%u)", proc->username, (unsigned)proc->pid); - if (old->proc != NULL) { - mslog(s, old->proc, LOG_DEBUG, "disconnecting (%u) due to new cookie connection", - (unsigned)old->proc->pid); + /* check for a user with the same sid as in the cookie */ + old_proc = proc_search_sid(s, cmsg->sid.data, cmsg->sid.len); + if (old_proc != NULL) { + mslog(s, old_proc, LOG_DEBUG, "disconnecting (%u) due to new cookie session", + (unsigned)old_proc->pid); - if (strcmp(proc->username, old->proc->username) != 0) { - mslog(s, old->proc, LOG_ERR, "the user of the cookie don't match (new: %s)", - proc->username); - return -1; - } - - /* steal its leases */ - steal_ip_leases(old->proc, proc); - - /* steal its cookie */ - old->proc->cookie_ptr = NULL; - - if (old->proc->pid > 0) - kill(old->proc->pid, SIGTERM); - } else { - revive_cookie(old); + if (strcmp(proc->username, old_proc->username) != 0) { + mslog(s, old_proc, LOG_ERR, "the user of the cookie doesn't match (new: %s)", + proc->username); + return -1; } - proc->cookie_ptr = old; - old->proc = proc; + + /* steal its leases */ + steal_ip_leases(old_proc, proc); + + if (old_proc->pid > 0) + kill(old_proc->pid, SIGTERM); } else { - if (cmsg->expiration < now) { - mslog(s, proc, LOG_INFO, "ignoring expired cookie"); - return -1; - } - - mslog(s, proc, LOG_DEBUG, "new cookie for (%u)", (unsigned)proc->pid); - - proc->cookie_ptr = new_cookie_entry(&s->cookies, proc, req->cookie.data, req->cookie.len); - if (proc->cookie_ptr == NULL) - return -1; + mslog(s, proc, LOG_DEBUG, "new cookie session for (%u)", (unsigned)proc->pid); } if (proc->config.require_cert != 0 && cmsg->tls_auth_ok == 0) { diff --git a/src/main-misc.c b/src/main-misc.c index 1df83171..5e97ed7d 100644 --- a/src/main-misc.c +++ b/src/main-misc.c @@ -369,12 +369,6 @@ void remove_proc(main_server_st * s, struct proc_st *proc, unsigned k) if (proc->ipv4 || proc->ipv6) remove_ip_leases(s, proc); - /* expire any available cookies */ - if (proc->cookie_ptr) { - proc->cookie_ptr->proc = NULL; - proc->cookie_ptr->expiration = time(0) + s->config->cookie_timeout; - } - close_tun(s, proc); proc_table_del(s, proc); diff --git a/src/main.c b/src/main.c index 5b93f148..2c6d89a1 100644 --- a/src/main.c +++ b/src/main.c @@ -606,8 +606,6 @@ void clear_lists(main_server_st *s) close(ctmp->fd); if (ctmp->tun_lease.fd >= 0) close(ctmp->tun_lease.fd); - if (ctmp->cookie_ptr) - ctmp->cookie_ptr->proc = NULL; list_del(&ctmp->list); safe_memset(ctmp, 0, sizeof(*ctmp)); talloc_free(ctmp); @@ -623,7 +621,6 @@ void clear_lists(main_server_st *s) ip_lease_deinit(&s->ip_leases); proc_table_deinit(s); ctl_handler_deinit(s); - cookie_db_deinit(&s->cookies); } static void kill_children(main_server_st* s) @@ -859,7 +856,6 @@ unsigned total = 10; need_maintenance = 0; mslog(s, NULL, LOG_DEBUG, "performing maintenance"); expire_tls_sessions(s); - expire_cookies(&s->cookies); alarm(MAINTAINANCE_TIME(s)); } } @@ -932,7 +928,6 @@ int main(int argc, char** argv) list_head_init(&s->proc_list.head); list_head_init(&s->script_list.head); tls_cache_init(s, &s->tls_db); - cookie_db_init(s, &s->cookies); ip_lease_init(&s->ip_leases); proc_table_init(s); diff --git a/src/main.h b/src/main.h index 569b7b84..eaed6de1 100644 --- a/src/main.h +++ b/src/main.h @@ -80,19 +80,6 @@ enum { PS_AUTH_COMPLETED, /* successful authentication */ }; -#define COOKIE_HASH_SIZE 20 -#define COOKIE_HASH GNUTLS_DIG_SHA1 - -typedef struct cookie_entry_st { - struct proc_st *proc; /* may be null, otherwise the proc that uses that cookie */ - time_t expiration; /* -1 or the time it should expire */ - - /* We store the hash of the cookie that is associated with a particular session. - * The reason is to avoid a memory leak to an unprivileged process to expose - * data that can be used to authenticate as another user */ - uint8_t cookie_hash[COOKIE_HASH_SIZE]; -} cookie_st; - /* Each worker process maps to a unique proc_st structure. */ typedef struct proc_st { @@ -136,9 +123,6 @@ typedef struct proc_st { char dtls_compr[8]; unsigned mtu; - /* pointer to the cookie used by this session */ - struct cookie_entry_st *cookie_ptr; - /* if the session is initiated by a cookie the following two are set * and are considered when generating an IP address. That is used to * generate the same address as previously allocated. diff --git a/src/sec-mod-auth.c b/src/sec-mod-auth.c index c29e8bdf..d58112b6 100644 --- a/src/sec-mod-auth.c +++ b/src/sec-mod-auth.c @@ -329,8 +329,10 @@ int handle_sec_auth_session_cmd(int cfd, sec_mod_st * sec, const SecAuthSessionM if (cmd == SM_CMD_AUTH_SESSION_OPEN) { SecAuthSessionReplyMsg rep = SEC_AUTH_SESSION_REPLY_MSG__INIT; - /* we don't check for expiration of session here. This is - * done at the main server. */ + if (e->time != -1 && time(0) > e->time + sec->config->cookie_timeout) { + seclog(sec, LOG_ERR, "session is expired"); + return -1; + } if (module != NULL && module->open_session != NULL) { ret = module->open_session(e->auth_ctx, req->sid.data, req->sid.len); @@ -371,6 +373,8 @@ int handle_sec_auth_session_cmd(int cfd, sec_mod_st * sec, const SecAuthSessionM if (rep.reply != AUTH__REP__OK) del_client_entry(sec, e); + else /* set expiration time to unlimited (until someone closes the session) */ + e->time = -1; } else { /* CLOSE */ if (req->has_uptime && req->uptime > e->stats.uptime) {