ref: 54c49284e03e46f6e3a5d41bfc9fbc98c6f0b214
parent: 294e08fa1e2481a3b01b815c34f458999d2e782c
author: cinap_lenrek <[email protected]>
date: Sat Apr 16 19:36:55 EDT 2016
libsec: fix memory leak of RSApub, avoid parsing certificate twice to extract rsa public key instead of letting factotum_rsa_open() parse the certificate, we pass in the rsa public key which is then matched against the factotum keyring. this avoids parsing the x509 certificate twice. the sec->rsapub was not freed, so free it in tlsSecClose()
--- a/sys/src/libsec/port/tlshand.c
+++ b/sys/src/libsec/port/tlshand.c
@@ -72,7 +72,7 @@
int ver2hi; // server got a version 2 hello
int isClient; // is this the client or server?
Bytes *sid; // SessionID
- Bytes *cert; // only last - no chain
+ Bytes *cert; // server certificate; only last - no chain
Lock statelk;
int state; // must be set using setstate
@@ -428,9 +428,9 @@
uchar *seed0, int nseed0, uchar *seed1, int nseed1);
static int setVers(TlsSec *sec, int version);
-static AuthRpc* factotum_rsa_open(uchar *cert, int certlen);
+static AuthRpc* factotum_rsa_open(RSApub *rsapub);
static mpint* factotum_rsa_decrypt(AuthRpc *rpc, mpint *cipher);
-static void factotum_rsa_close(AuthRpc*rpc);
+static void factotum_rsa_close(AuthRpc *rpc);
static void* emalloc(int);
static void* erealloc(void*, int);
@@ -715,14 +715,10 @@
memmove(c->crandom, m.u.clientHello.random, RandomSize);
cipher = okCipher(m.u.clientHello.ciphers, psklen > 0);
- if(cipher < 0) {
+ if(cipher < 0 || !setAlgs(c, cipher)) {
tlsError(c, EHandshakeFailure, "no matching cipher suite");
goto Err;
}
- if(!setAlgs(c, cipher)){
- tlsError(c, EHandshakeFailure, "no matching cipher suite");
- goto Err;
- }
compressor = okCompression(m.u.clientHello.compressors);
if(compressor < 0) {
tlsError(c, EHandshakeFailure, "no matching compressor");
@@ -733,25 +729,22 @@
if(trace)
trace(" cipher %x, compressor %x, csidlen %d\n", cipher, compressor, csid->len);
c->sec = tlsSecInits(c->clientVersion, csid->data, csid->len, c->crandom, sid, &nsid, c->srandom);
- if(c->sec == nil){
- tlsError(c, EHandshakeFailure, "can't initialize security: %r");
- goto Err;
- }
if(psklen > 0){
c->sec->psk = psk;
c->sec->psklen = psklen;
}
if(certlen > 0){
- c->sec->rpc = factotum_rsa_open(cert, certlen);
- if(c->sec->rpc == nil){
- tlsError(c, EHandshakeFailure, "factotum_rsa_open: %r");
- goto Err;
- }
+ /* server certificate */
c->sec->rsapub = X509toRSApub(cert, certlen, nil, 0);
if(c->sec->rsapub == nil){
tlsError(c, EHandshakeFailure, "invalid X509/rsa certificate");
goto Err;
}
+ c->sec->rpc = factotum_rsa_open(c->sec->rsapub);
+ if(c->sec->rpc == nil){
+ tlsError(c, EHandshakeFailure, "factotum_rsa_open: %r");
+ goto Err;
+ }
}
msgClear(&m);
@@ -1123,13 +1116,23 @@
c->cert = nil;
c->sec = tlsSecInitc(c->clientVersion, c->crandom);
- if(c->sec == nil)
- goto Err;
-
if(psklen > 0){
c->sec->psk = psk;
c->sec->psklen = psklen;
}
+ if(certlen > 0){
+ /* client certificate */
+ c->sec->rsapub = X509toRSApub(cert, certlen, nil, 0);
+ if(c->sec->rsapub == nil){
+ tlsError(c, EHandshakeFailure, "invalid X509/rsa certificate");
+ goto Err;
+ }
+ c->sec->rpc = factotum_rsa_open(c->sec->rsapub);
+ if(c->sec->rpc == nil){
+ tlsError(c, EHandshakeFailure, "factotum_rsa_open: %r");
+ goto Err;
+ }
+ }
/* client hello */
memset(&m, 0, sizeof(m));
@@ -1267,7 +1270,7 @@
}
if(creq) {
- if(cert != nil && certlen > 0){
+ if(certlen > 0){
m.u.certificate.ncert = 1;
m.u.certificate.certs = emalloc(m.u.certificate.ncert * sizeof(Bytes*));
m.u.certificate.certs[0] = makebytes(cert, certlen);
@@ -1293,23 +1296,12 @@
msgClear(&m);
/* certificate verify */
- if(creq && cert != nil && certlen > 0) {
+ if(creq && certlen > 0) {
mpint *signedMP, *paddedHashes;
HandshakeHash hsave;
uchar buf[512];
int buflen;
- c->sec->rpc = factotum_rsa_open(cert, certlen);
- if(c->sec->rpc == nil){
- tlsError(c, EHandshakeFailure, "factotum_rsa_open: %r");
- goto Err;
- }
- c->sec->rsapub = X509toRSApub(cert, certlen, nil, 0);
- if(c->sec->rsapub == nil){
- tlsError(c, EHandshakeFailure, "invalid X509/rsa certificate");
- goto Err;
- }
-
/* save the state for the Finish message */
hsave = c->handhash;
if(c->version >= TLS12Version){
@@ -1396,7 +1388,7 @@
free(epm);
msgClear(&m);
tlsConnectionFree(c);
- return 0;
+ return nil;
}
@@ -2369,15 +2361,14 @@
//================= security functions ========================
-// given X.509 certificate, set up connection to factotum
-// for using corresponding private key
+// given a public key, set up connection to factotum
+// for using corresponding private key
static AuthRpc*
-factotum_rsa_open(uchar *cert, int certlen)
+factotum_rsa_open(RSApub *rsapub)
{
int afd;
char *s;
- mpint *pub = nil;
- RSApub *rsapub;
+ mpint *n;
AuthRpc *rpc;
// start talking to factotum
@@ -2388,28 +2379,20 @@
return nil;
}
s = "proto=rsa service=tls role=client";
- if(auth_rpc(rpc, "start", s, strlen(s)) != ARok){
- factotum_rsa_close(rpc);
- return nil;
- }
-
- // roll factotum keyring around to match certificate
- rsapub = X509toRSApub(cert, certlen, nil, 0);
- while(1){
- if(auth_rpc(rpc, "read", nil, 0) != ARok){
- factotum_rsa_close(rpc);
- rpc = nil;
- goto done;
+ if(auth_rpc(rpc, "start", s, strlen(s)) == ARok){
+ // roll factotum keyring around to match public key
+ n = mpnew(0);
+ while(auth_rpc(rpc, "read", nil, 0) == ARok){
+ if(strtomp(rpc->arg, nil, 16, n) != nil
+ && mpcmp(n, rsapub->n) == 0){
+ mpfree(n);
+ return rpc;
+ }
}
- pub = strtomp(rpc->arg, nil, 16, nil);
- assert(pub != nil);
- if(mpcmp(pub,rsapub->n) == 0)
- break;
+ mpfree(n);
}
-done:
- mpfree(pub);
- rsapubfree(rsapub);
- return rpc;
+ factotum_rsa_close(rpc);
+ return nil;
}
static mpint*
@@ -2593,15 +2576,12 @@
}
// 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);
+ genrandom(pm->data, pm->len);
}
setMasterSecret(sec, pm);
return 0;
@@ -2702,6 +2682,7 @@
if(sec == nil)
return;
factotum_rsa_close(sec->rpc);
+ rsapubfree(sec->rsapub);
free(sec->server);
free(sec);
}