shithub: riscv

Info  •  Files  •  Log  •  Branches

libsec: fix double free in pkcs1_decrypt(), handle bad epm length in tlsSecRSAs(), cleanup

--- a/sys/src/libsec/port/tlshand.c

+++ b/sys/src/libsec/port/tlshand.c

@@ -398,9 +398,7 @@

 static void	tlsSecKill(TlsSec *sec);

 static void	tlsSecClose(TlsSec *sec);

 static void	setMasterSecret(TlsSec *sec, Bytes *pm);

-static void	serverMasterSecret(TlsSec *sec, Bytes *epm);

 static void	setSecrets(TlsSec *sec, uchar *kd, int nkd);

-static Bytes*	clientMasterSecret(TlsSec *sec, RSApub *pub);

 static Bytes*	pkcs1_encrypt(Bytes* data, RSApub* key, int blocktype);

 static Bytes*	pkcs1_decrypt(TlsSec *sec, Bytes *cipher);

 static void	tls10SetFinished(TlsSec *sec, HandshakeHash hsh, uchar *finished, int isClient);

@@ -2452,9 +2450,9 @@

 }

 static void

-factotum_rsa_close(AuthRpc*rpc)

+factotum_rsa_close(AuthRpc *rpc)

 {

-	if(!rpc)

+	if(rpc == nil)

 		return;

 	close(rpc->afd);

 	auth_freerpc(rpc);

@@ -2605,14 +2603,27 @@

 static int

 tlsSecRSAs(TlsSec *sec, int vers, Bytes *epm)

 {

-	if(epm != nil){

-		if(setVers(sec, vers) < 0)

-			goto Err;

-		serverMasterSecret(sec, epm);

-	}else if(sec->vers != vers){

-		werrstr("mismatched session versions");

+	Bytes *pm;

+

+	if(setVers(sec, vers) < 0)

 		goto Err;

+	if(epm == nil){

+		werrstr("no encrypted premaster secret");

+		goto Err;

 	}

+	// if the client messed up, just continue as if everything is ok,

+	// to prevent attacks to check for correctly formatted messages.

+	// Hence the fprint(2,) can't be replaced by tlsError(), which sends an Alert msg to the client.

+	pm = pkcs1_decrypt(sec, epm);

+	if(sec->ok < 0 || pm == nil || pm->len != MasterSecretSize || get16(pm->data) != sec->clientVers){

+		fprint(2, "tlsSecRSAs failed ok=%d pm=%p pmvers=%x cvers=%x nepm=%d\n",

+			sec->ok, pm, pm != nil ? get16(pm->data) : -1, sec->clientVers, epm->len);

+		sec->ok = -1;

+		freebytes(pm);

+		pm = newbytes(MasterSecretSize);

+		genrandom(pm->data, MasterSecretSize);

+	}

+	setMasterSecret(sec, pm);

 	return 0;

 Err:

 	sec->ok = -1;

@@ -2657,7 +2668,7 @@

 tlsSecRSAc(TlsSec *sec, uchar *sid, int nsid, uchar *srandom, uchar *cert, int ncert, int vers)

 {

 	RSApub *pub;

-	Bytes *epm;

+	Bytes *pm, *epm;

 	USED(sid);

 	USED(nsid);

@@ -2670,7 +2681,11 @@

 		werrstr("invalid x509/rsa certificate");

 		goto Err;

 	}

-	epm = clientMasterSecret(sec, pub);

+	pm = newbytes(MasterSecretSize);

+	put16(pm->data, sec->clientVers);

+	genrandom(pm->data+2, MasterSecretSize - 2);

+	epm = pkcs1_encrypt(pm, pub, 2);

+	setMasterSecret(sec, pm);

 	rsapubfree(pub);

 	if(epm != nil)

 		return epm;

@@ -2713,7 +2728,7 @@

 static void

 tlsSecClose(TlsSec *sec)

 {

-	if(!sec)

+	if(sec == nil)

 		return;

 	factotum_rsa_close(sec->rpc);

 	free(sec->server);

@@ -2785,41 +2800,6 @@

 }

 static void

-serverMasterSecret(TlsSec *sec, Bytes *epm)

-{

-	Bytes *pm;

-

-	pm = pkcs1_decrypt(sec, epm);

-

-	// if the client messed up, just continue as if everything is ok,

-	// to prevent attacks to check for correctly formatted messages.

-	// Hence the fprint(2,) can't be replaced by tlsError(), which sends an Alert msg to the client.

-	if(sec->ok < 0 || pm == nil || get16(pm->data) != sec->clientVers){

-		fprint(2, "serverMasterSecret failed ok=%d pm=%p pmvers=%x cvers=%x nepm=%d\n",

-			sec->ok, pm, pm != nil ? get16(pm->data) : -1, sec->clientVers, epm->len);

-		sec->ok = -1;

-		freebytes(pm);

-		pm = newbytes(MasterSecretSize);

-		genrandom(pm->data, MasterSecretSize);

-	}

-	assert(pm->len == MasterSecretSize);

-	setMasterSecret(sec, pm);

-}

-

-static Bytes*

-clientMasterSecret(TlsSec *sec, RSApub *pub)

-{

-	Bytes *pm, *epm;

-

-	pm = newbytes(MasterSecretSize);

-	put16(pm->data, sec->clientVers);

-	genrandom(pm->data+2, MasterSecretSize - 2);

-	epm = pkcs1_encrypt(pm, pub, 2);

-	setMasterSecret(sec, pm);

-	return epm;

-}

-

-static void

 sslSetFinished(TlsSec *sec, HandshakeHash hsh, uchar *finished, int isClient)

 {

 	DigestState *s;

@@ -3010,7 +2990,7 @@

 static Bytes*

 pkcs1_decrypt(TlsSec *sec, Bytes *cipher)

 {

-	Bytes *eb, *ans = nil;

+	Bytes *eb;

 	int i, modlen;

 	mpint *x, *y;

@@ -3021,24 +3001,21 @@

 	y = factotum_rsa_decrypt(sec->rpc, x);

 	if(y == nil)

 		return nil;

-	eb = mptobytes(y);

+	eb = newbytes(modlen);

+	mptober(y, eb->data, eb->len);

 	mpfree(y);

-	if(eb->len < modlen){ // pad on left with zeros

-		ans = newbytes(modlen);

-		memset(ans->data, 0, modlen-eb->len);

-		memmove(ans->data+modlen-eb->len, eb->data, eb->len);

-		freebytes(eb);

-		eb = ans;

-	}

 	if(eb->data[0] == 0 && eb->data[1] == 2) {

-		for(i = 2; i < modlen; i++)

+		for(i = 2; i < eb->len; i++)

 			if(eb->data[i] == 0)

 				break;

-		if(i < modlen - 1)

-			ans = makebytes(eb->data+i+1, modlen-(i+1));

+		if(i < eb->len - 1){

+			eb->len -= i+1;

+			memmove(eb->data, eb->data+i+1, eb->len);

+			return eb;

+		}

 	}

 	freebytes(eb);

-	return ans;

+	return nil;

 }