shithub: riscv

Info  •  Files  •  Log  •  Branches

devcap: timeout capabilities after a minute, fix memory leak, paranoia

the manpage states that capabilities time out after a minute,
so we add ticks field into the Caphash struct and record the
time when the capability was inserted. freeing old capabilities
is handled in trimcaps(), which makes room for one extra cap
and frees timed out ones.

we also limit the capuse write size to less than 1024 bytes to
prevent denial of service as we have to copy the user buffer.
(memory exhaustion).

we have to check the from user *before* attempting to remove
the capability! the wrong user shouldnt be able to change any
state. this fixes the memory leak of the caphash.

do the hash comparsion with tsmemcmp(), avoiding timing
side channels.

allocate the capabilities in secret memory pool to prevent
debugger access.

--- a/sys/src/9/port/devcap.c

+++ b/sys/src/9/port/devcap.c

@@ -11,6 +11,7 @@

 {

 	Hashlen=	SHA1dlen,

 	Maxhash=	256,

+	Timeout=	60,	/* seconds */

 };

 /*

@@ -21,11 +22,11 @@

 struct Caphash

 {

 	Caphash	*next;

-	char		hash[Hashlen];

-	ulong		ticks;

+	ulong	ticks;

+	char	hash[Hashlen];

 };

-struct

+static struct

 {

 	QLock;

 	Caphash	*first;

@@ -104,19 +105,19 @@

 	return c;

 }

-/*

-static char*

-hashstr(uchar *hash)

+static void

+trimcaps(void)

 {

-	static char buf[2*Hashlen+1];

-	int i;

+	Caphash *t;

-	for(i = 0; i < Hashlen; i++)

-		sprint(buf+2*i, "%2.2ux", hash[i]);

-	buf[2*Hashlen] = 0;

-	return buf;

+	while((t = capalloc.first) != nil){

+		if(capalloc.nhash < Maxhash && TK2SEC(MACHP(0)->ticks - t->ticks) < Timeout)

+			break;

+		capalloc.nhash--;

+		capalloc.first = t->next;

+		secfree(t);

+	}

 }

- */

 static Caphash*

 remcap(uchar *hash)

@@ -125,10 +126,13 @@

 	qlock(&capalloc);

+	/* timeout old caps */

+	trimcaps();

+

 	/* find the matching capability */

 	for(l = &capalloc.first; *l != nil;){

 		t = *l;

-		if(memcmp(hash, t->hash, Hashlen) == 0)

+		if(tsmemcmp(hash, t->hash, Hashlen) == 0)

 			break;

 		l = &t->next;

 	}

@@ -146,24 +150,17 @@

 static void

 addcap(uchar *hash)

 {

-	Caphash *p, *t, **l;

+	Caphash *p, **l;

-	p = smalloc(sizeof *p);

+	p = secalloc(sizeof *p);

 	memmove(p->hash, hash, Hashlen);

 	p->next = nil;

-	p->ticks = m->ticks;

+	p->ticks = MACHP(0)->ticks;

 	qlock(&capalloc);

-	/* trim extras */

-	while(capalloc.nhash >= Maxhash){

-		t = capalloc.first;

-		if(t == nil)

-			panic("addcap");

-		capalloc.first = t->next;

-		free(t);

-		capalloc.nhash--;

-	}

+	/* make room for one extra */

+	trimcaps();

 	/* add new one */

 	for(l = &capalloc.first; *l != nil; l = &(*l)->next)

@@ -213,13 +210,14 @@

 		break;

 	case Quse:

+		if((ulong)n >= 1024)

+			error(Etoobig);

 		/* copy key to avoid a fault in hmac_xx */

-		cp = nil;

+		cp = secalloc(n+1);

 		if(waserror()){

-			free(cp);

+			secfree(cp);

 			nexterror();

 		}

-		cp = smalloc(n+1);

 		memmove(cp, va, n);

 		cp[n] = 0;

@@ -231,12 +229,6 @@

 		hmac_sha1((uchar*)from, strlen(from), (uchar*)key, strlen(key), hash, nil);

-		p = remcap(hash);

-		if(p == nil){

-			snprint(err, sizeof err, "invalid capability %s@%s", from, key);

-			error(err);

-		}

-

 		/* if a from user is supplied, make sure it matches */

 		to = strchr(from, '@');

 		if(to == nil){

@@ -247,12 +239,18 @@

 				error("capability must match user");

 		}

+		p = remcap(hash);

+		if(p == nil){

+			snprint(err, sizeof err, "invalid capability %s@%s", from, key);

+			error(err);

+		}

+		secfree(p);

+

 		/* set user id */

 		kstrdup(&up->user, to);

 		up->basepri = PriNormal;

-		free(p);

-		free(cp);

+		secfree(cp);

 		poperror();

 		break;

@@ -260,6 +258,7 @@

 		error(Eperm);

 		break;

 	}

+	memset(hash, 0, Hashlen);

 	return n;

 }