libsec: revert asn1mpint(), rewrite rsa signature validation, cleanups

reverting asn1mpint() as all users really just expect
unsigned integers here. also openssl seems to interpret
rsa modulus as unsigned no matter what... so keeping
it as it was before.

handle nil cipher bytes in factotum_rsa_decrypt() due
to pkcs1padbuf() failing.

apply some lessions from intels berzerk paper:

instead of parsing the decrypted digest info blob, we
generate the *expected* blob's for all digest algorithms
that match the digest size and compare the results.

provide pkcs1 pad and unpad functions that consistently
enforce minimum padding size and handles block types 1
and 2.

2 files changed, 115 insertions, 197 deletions

diff --git a/sys/src/libsec/port/tlshand.c b/sys/src/libsec/port/tlshand.c
index 1c59ddf67..9aec9fb63 100644
--- a/sys/src/libsec/port/tlshand.c
+++ b/sys/src/libsec/port/tlshand.c
@@ -431,8 +431,8 @@ static void	setMasterSecret(TlsSec *sec, Bytes *pm);
 static int	digestDHparams(TlsSec *sec, Bytes *par, uchar digest[MAXdlen], int sigalg);
 static char*	verifyDHparams(TlsSec *sec, Bytes *par, Bytes *cert, Bytes *sig, int sigalg);
 
-static Bytes*	pkcs1_encrypt(Bytes* data, RSApub* key, int blocktype);
-static Bytes*	pkcs1_decrypt(TlsSec *sec, Bytes *cipher);
+static Bytes*	pkcs1_encrypt(Bytes* data, RSApub* key);
+static Bytes*	pkcs1_decrypt(TlsSec *sec, Bytes *data);
 static Bytes*	pkcs1_sign(TlsSec *sec, uchar *digest, int digestlen, int sigalg);
 
 static void* emalloc(int);
@@ -452,7 +452,8 @@ static void freeints(Ints* b);
 static int lookupid(Ints* b, int id);
 
 /* x509.c */
-extern mpint*	pkcs1padbuf(uchar *buf, int len, mpint *modulus);
+extern mpint*	pkcs1padbuf(uchar *buf, int len, mpint *modulus, int blocktype);
+extern int	pkcs1unpadbuf(uchar *buf, int len, mpint *modulus, int blocktype);
 extern int	asn1encodedigest(DigestState* (*fun)(uchar*, ulong, uchar*, DigestState*), uchar *digest, uchar *buf, int len);
 
 //================= client/server ========================
@@ -2323,6 +2324,8 @@ factotum_rsa_decrypt(AuthRpc *rpc, mpint *cipher)
 	char *p;
 	int rv;
 
+	if(cipher == nil)
+		return nil;
 	p = mptoa(cipher, 16, nil, 0);
 	mpfree(cipher);
 	if(p == nil)
@@ -2660,7 +2663,7 @@ tlsSecRSAc(TlsSec *sec, uchar *cert, int ncert)
 	pm = newbytes(MasterSecretSize);
 	put16(pm->data, sec->clientVers);
 	genrandom(pm->data+2, MasterSecretSize - 2);
-	epm = pkcs1_encrypt(pm, pub, 2);
+	epm = pkcs1_encrypt(pm, pub);
 	setMasterSecret(sec, pm);
 	rsapubfree(pub);
 	return epm;
@@ -2833,105 +2836,40 @@ verifyDHparams(TlsSec *sec, Bytes *par, Bytes *cert, Bytes *sig, int sigalg)
 	return err;
 }
 
-
-// Do RSA computation on block according to key, and pad
-// result on left with zeros to make it modlen long.
+// encrypt data according to PKCS#1, /lib/rfc/rfc2437 9.1.2.1
 static Bytes*
-rsacomp(Bytes* block, RSApub* key, int modlen)
+pkcs1_encrypt(Bytes* data, RSApub* key)
 {
 	mpint *x, *y;
-	Bytes *a, *ybytes;
-	int ylen;
 
-	x = bytestomp(block);
+	x = pkcs1padbuf(data->data, data->len, key->n, 2);
+	if(x == nil)
+		return nil;
 	y = rsaencrypt(key, x, nil);
 	mpfree(x);
-	ybytes = mptobytes(y);
-	ylen = ybytes->len;
+	data = newbytes((mpsignif(key->n)+7)/8);
+	mptober(y, data->data, data->len);
 	mpfree(y);
-
-	if(ylen < modlen) {
-		a = newbytes(modlen);
-		memset(a->data, 0, modlen-ylen);
-		memmove(a->data+modlen-ylen, ybytes->data, ylen);
-		freebytes(ybytes);
-		ybytes = a;
-	}
-	else if(ylen > modlen) {
-		// assume it has leading zeros (mod should make it so)
-		a = newbytes(modlen);
-		memmove(a->data, ybytes->data, modlen);
-		freebytes(ybytes);
-		ybytes = a;
-	}
-	return ybytes;
-}
-
-// encrypt data according to PKCS#1, /lib/rfc/rfc2437 9.1.2.1
-static Bytes*
-pkcs1_encrypt(Bytes* data, RSApub* key, int blocktype)
-{
-	Bytes *pad, *eb, *ans;
-	int i, dlen, padlen, modlen;
-
-	modlen = (mpsignif(key->n)+7)/8;
-	dlen = data->len;
-	if(modlen < 12 || dlen > modlen - 11)
-		return nil;
-	padlen = modlen - 3 - dlen;
-	pad = newbytes(padlen);
-	genrandom(pad->data, padlen);
-	for(i = 0; i < padlen; i++) {
-		if(blocktype == 0)
-			pad->data[i] = 0;
-		else if(blocktype == 1)
-			pad->data[i] = 255;
-		else if(pad->data[i] == 0)
-			pad->data[i] = 1;
-	}
-	eb = newbytes(modlen);
-	eb->data[0] = 0;
-	eb->data[1] = blocktype;
-	memmove(eb->data+2, pad->data, padlen);
-	eb->data[padlen+2] = 0;
-	memmove(eb->data+padlen+3, data->data, dlen);
-	ans = rsacomp(eb, key, modlen);
-	freebytes(eb);
-	freebytes(pad);
-	return ans;
+	return data;
 }
 
 // decrypt data according to PKCS#1, with given key.
-// expect a block type of 2.
 static Bytes*
-pkcs1_decrypt(TlsSec *sec, Bytes *cipher)
+pkcs1_decrypt(TlsSec *sec, Bytes *data)
 {
-	Bytes *eb;
-	int i, modlen;
-	mpint *x, *y;
+	mpint *y;
 
-	modlen = (mpsignif(sec->rsapub->n)+7)/8;
-	if(cipher->len != modlen)
+	if(data->len != (mpsignif(sec->rsapub->n)+7)/8)
 		return nil;
-	x = bytestomp(cipher);
-	y = factotum_rsa_decrypt(sec->rpc, x);
+	y = factotum_rsa_decrypt(sec->rpc, bytestomp(data));
 	if(y == nil)
 		return nil;
-	eb = newbytes(modlen);
-	mptober(y, eb->data, eb->len);
-	mpfree(y);
-	if(eb->data[0] == 0 && eb->data[1] == 2) {
-		for(i = 2; i < eb->len; i++)
-			if(eb->data[i] == 0)
-				break;
-		if(++i < eb->len){
-			eb->len -= i;
-			memmove(eb->data, eb->data+i, eb->len);
-			return eb;
-		}
+	data = mptobytes(y);
+	if((data->len = pkcs1unpadbuf(data->data, data->len, sec->rsapub->n, 2)) < 0){
+		freebytes(data);
+		return nil;
 	}
-	freebytes(eb);
-	return nil;
+	return data;
 }
 
 static Bytes*
@@ -2952,7 +2890,7 @@ pkcs1_sign(TlsSec *sec, uchar *digest, int digestlen, int sigalg)
 		werrstr("bad digest algorithm");
 		return nil;
 	}
-	signedMP = factotum_rsa_decrypt(sec->rpc, pkcs1padbuf(buf, digestlen, sec->rsapub->n));
+	signedMP = factotum_rsa_decrypt(sec->rpc, pkcs1padbuf(buf, digestlen, sec->rsapub->n, 1));
 	if(signedMP == nil)
 		return nil;
 	signature = mptobytes(signedMP);
diff --git a/sys/src/libsec/port/x509.c b/sys/src/libsec/port/x509.c
index 20cfb9a8a..da09b978d 100644
--- a/sys/src/libsec/port/x509.c
+++ b/sys/src/libsec/port/x509.c
@@ -1492,6 +1492,19 @@ freevalfields(Value* v)
 	}
 }
 
+static mpint*
+asn1mpint(Elem *e)
+{
+	Bytes *b;
+	int v;
+
+	if(is_int(e, &v))
+		return itomp(v, nil);
+	if(is_bigint(e, &b))
+		return betomp(b->data, b->len, nil);
+	return nil;
+}
+
 /* end of general ASN1 functions */
 
 
@@ -1695,6 +1708,8 @@ static DigestAlg *digestalg[NUMALGS+1] = {
 	nil
 };
 
+static Bytes* encode_digest(DigestAlg *da, uchar *digest);
+
 static Ints15 oid_secp256r1 = {7, 1, 2, 840, 10045, 3, 1, 7};
 static Ints15 oid_secp384r1 = {5, 1, 3, 132, 0, 34};
 
@@ -2121,54 +2136,6 @@ errret:
 	return nil;
 }
 
-static mpint*
-asn1mpint(Elem *e)
-{
-	Bytes *b;
-	int v;
-
-	if(is_int(e, &v))
-		return itomp(v, nil);
-	if(is_bigint(e, &b)){
-		mpint *s = betomp(b->data, b->len, nil);
-		if(b->len > 0 && (b->data[0] & 0x80) != 0)
-			mpxtend(s, b->len*8, s);
-		return s;
-	}
-	return nil;
-}
-
-mpint*
-pkcs1padbuf(uchar *buf, int len, mpint *modulus)
-{
-	int n = (mpsignif(modulus)+7)/8;
-	int pm1, i;
-	uchar *p;
-	mpint *mp;
-
-	pm1 = n - 1 - len;
-	if(pm1 <= 2){
-		werrstr("pkcs1padbuf: modulus too small");
-		return nil;
-	}
-	p = (uchar*)emalloc(n);
-	p[0] = 0;
-	p[1] = 1;
-	for(i = 2; i < pm1; i++)
-		p[i] = 0xFF;
-	p[pm1] = 0;
-	memcpy(&p[pm1+1], buf, len);
-	mp = betomp(p, n, nil);
-	free(p);
-	return mp;
-}
-
-static mpint*
-pkcs1pad(Bytes *b, mpint *modulus)
-{
-	return pkcs1padbuf(b->data, b->len, modulus);
-}
-
 RSApriv*
 asn1toRSApriv(uchar *kd, int kn)
 {
@@ -2226,79 +2193,92 @@ digest_certinfo(Bytes *cert, DigestAlg *da, uchar *digest)
 	return da->len;
 }
 
-static int
-pkcs1decryptsignature(uchar *sig, int siglen, RSApub *pk, uchar **pbuf)
+mpint*
+pkcs1padbuf(uchar *buf, int len, mpint *modulus, int blocktype)
 {
-	int nlen, buflen;
-	mpint *pkcs1;
-	uchar *buf;
-
-	*pbuf = nil;
+	int i, n = (mpsignif(modulus)-1)/8;
+	int pad = n - 2 - len;
+	uchar *p;
+	mpint *mp;
 
-	/* one less than the byte length of the modulus */
-	nlen = (mpsignif(pk->n)-1)/8;
+	if(pad < 8){
+		werrstr("rsa modulus too small");
+		return nil;
+	}
+	if((p = malloc(n)) == nil)
+		return nil;
+	p[0] = blocktype;
+	switch(blocktype){
+	default:
+	case 1:
+		memset(p+1, 0xFF, pad);
+		break;
+	case 2:
+		for(i=1; i <= pad; i++)
+			p[i] = 1 + nfastrand(255);
+		break;
+	}
+	p[1+pad] = 0;
+	memmove(p+2+pad, buf, len);
+	mp = betomp(p, n, nil);
+	free(p);
+	return mp;
+}
 
-	/* see 9.2.1 of rfc2437 */
-	pkcs1 = betomp(sig, siglen, nil);
-	mpexp(pkcs1, pk->ek, pk->n, pkcs1);
-	buflen = mptobe(pkcs1, nil, 0, pbuf);
-	mpfree(pkcs1);
+int
+pkcs1unpadbuf(uchar *buf, int len, mpint *modulus, int blocktype)
+{
+	uchar *p = buf + 1, *e = buf + len;
 
-	buf = *pbuf;
-	if(buflen != nlen || buf[0] != 1)
-		goto bad;
-	buf++, buflen--;
-	while(buflen > 0 && buf[0] == 0xff)
-		buf++, buflen--;
-	if(buflen < 1 || buf[0] != 0)
-		goto bad;
-	buf++, buflen--;
-	memmove(*pbuf, buf, buflen);
-	return buflen;
-bad:
-	free(*pbuf);
-	*pbuf = nil;
-	return -1;
+	if(len < 1 || len != (mpsignif(modulus)-1)/8 || buf[0] != blocktype)
+		return -1;
+	switch(blocktype){
+	default:
+	case 1:
+		while(p < e && *p == 0xFF)
+			p++;
+		break;
+	case 2:
+		while(p < e && *p != 0x00)
+			p++;
+		break;
+	}
+	if(p - buf <= 8 || p >= e || *p++ != 0x00)
+		return -1;
+	memmove(buf, p, len = e - p);
+	return len;
 }
 
+static char Ebadsig[] = "bad signature";
+
 char*
 X509rsaverifydigest(uchar *sig, int siglen, uchar *edigest, int edigestlen, RSApub *pk)
 {
-	Elem e;
-	Elist *el;
+	mpint *x, *y;
+	DigestAlg **dp;
 	Bytes *digest;
 	uchar *buf;
-	int alg, buflen;
+	int len;
 	char *err;
 
-	buflen = pkcs1decryptsignature(sig, siglen, pk, &buf);
-	if(buflen == edigestlen && tsmemcmp(buf, edigest, edigestlen) == 0){
-		free(buf);
-		return nil;
-	}
-	el = nil;
-	memset(&e, 0, sizeof(e));
-	if(buflen < 0 || decode(buf, buflen, &e) != ASN_OK
-	|| !is_seq(&e, &el) || elistlen(el) != 2 || !is_octetstring(&el->tl->hd, &digest)) {
-		err = "signature parse error";
-		goto end;
-	}
-	alg = parse_alg(&el->hd);
-	if(alg < 0){
-		err = "unknown signature algorithm";
-		goto end;
-	}
-	if(digest->len != edigestlen || digest->len != digestalg[alg]->len){
-		err = "bad digest length";
-		goto end;
-	}
-	if(tsmemcmp(digest->data, edigest, edigestlen) != 0){
-		err = "digest did not match";
-		goto end;
+	x = betomp(sig, siglen, nil);
+	y = rsaencrypt(pk, x, nil);
+	mpfree(x);
+	len = mptobe(y, nil, 0, &buf);
+	mpfree(y);	
+
+	err = Ebadsig;
+	len = pkcs1unpadbuf(buf, len, pk->n, 1);
+	if(len == edigestlen && tsmemcmp(buf, edigest, edigestlen) == 0)
+		err = nil;
+	for(dp = digestalg; err != nil && *dp != nil; dp++){
+		if((*dp)->len != edigestlen)
+			continue;
+		digest = encode_digest(*dp, edigest);
+		if(digest->len == len && tsmemcmp(digest->data, buf, len) == 0)
+			err = nil;
+		freebytes(digest);
 	}
-	err = nil;
-end:
-	freevalfields(&e.val);
 	free(buf);
 	return err;
 }
@@ -2312,7 +2292,7 @@ X509ecdsaverifydigest(uchar *sig, int siglen, uchar *edigest, int edigestlen, EC
 	char *err;
 
 	r = s = nil;
-	err = "bad signature";
+	err = Ebadsig;
 	if(decode(sig, siglen, &e) != ASN_OK)
 		goto end;
 	if(!is_seq(&e, &el) || elistlen(el) != 2)
@@ -2838,7 +2818,7 @@ X509rsagen(RSApriv *priv, char *subj, ulong valid[2], int *certlen)
 	sigbytes = encode_digest(da, digest);
 	if(sigbytes == nil)
 		goto errret;
-	pkcs1 = pkcs1pad(sigbytes, pk->n);
+	pkcs1 = pkcs1padbuf(sigbytes->data, sigbytes->len, pk->n, 1);
 	freebytes(sigbytes);
 	if(pkcs1 == nil)
 		goto errret;
@@ -2907,7 +2887,7 @@ X509rsareq(RSApriv *priv, char *subj, int *certlen)
 	sigbytes = encode_digest(da, digest);
 	if(sigbytes == nil)
 		goto errret;
-	pkcs1 = pkcs1pad(sigbytes, pk->n);
+	pkcs1 = pkcs1padbuf(sigbytes->data, sigbytes->len, pk->n, 1);
 	freebytes(sigbytes);
 	if(pkcs1 == nil)
 		goto errret;