From cb1dc365c292e82a17b2e0a231b248b84773a14c Mon Sep 17 00:00:00 2001 From: cinap_lenrek Date: Thu, 16 Mar 2017 00:05:08 +0100 Subject: [PATCH] upas/fs: fix memory leaks in tls code, handle tls in a common wraptls() function --- sys/src/cmd/upas/fs/dat.h | 2 ++ sys/src/cmd/upas/fs/imap.c | 74 +++----------------------------------- sys/src/cmd/upas/fs/mkfile | 1 + sys/src/cmd/upas/fs/pop3.c | 64 +++++++-------------------------- sys/src/cmd/upas/fs/tls.c | 39 ++++++++++++++++++++ 5 files changed, 59 insertions(+), 121 deletions(-) create mode 100644 sys/src/cmd/upas/fs/tls.c diff --git a/sys/src/cmd/upas/fs/dat.h b/sys/src/cmd/upas/fs/dat.h index f955fd23c..a40e2b437 100644 --- a/sys/src/cmd/upas/fs/dat.h +++ b/sys/src/cmd/upas/fs/dat.h @@ -216,6 +216,8 @@ char* delmessages(int, char**); char *flagmessages(int, char**); void digestmessage(Mailbox*, Message*); +int wraptls(int); + void eprint(char*, ...); void iprint(char *, ...); int newid(void); diff --git a/sys/src/cmd/upas/fs/imap.c b/sys/src/cmd/upas/fs/imap.c index 4a3f74467..a877b2c37 100644 --- a/sys/src/cmd/upas/fs/imap.c +++ b/sys/src/cmd/upas/fs/imap.c @@ -60,8 +60,6 @@ struct Imap { int nuid; int muid; - Thumbprint *thumb; - /* open network connection */ Biobuf bin; Biobuf bout; @@ -760,44 +758,6 @@ imaperrstr(char *host, char *port) return buf; } -static int -starttls(Imap *imap, TLSconn *tls) -{ - char buf[Pathlen]; - uchar digest[SHA1dlen]; - int sfd, fd; - - memset(tls, 0, sizeof *tls); - sfd = tlsClient(imap->fd, tls); - if(sfd < 0){ - werrstr("tlsClient: %r"); - return -1; - } - if(tls->cert == nil || tls->certlen <= 0){ - close(sfd); - werrstr("server did not provide TLS certificate"); - return -1; - } - sha1(tls->cert, tls->certlen, digest, nil); - if(!imap->thumb || !okThumbprint(digest, imap->thumb)){ - close(sfd); - werrstr("server certificate %.*H not recognized", - SHA1dlen, digest); - return -1; - } - close(imap->fd); - imap->fd = sfd; - - if(imap->flags & Fdebug){ - snprint(buf, sizeof buf, "%s/ctl", tls->dir); - fd = open(buf, OWRITE); - fprint(fd, "debug"); - close(fd); - } - - return 1; -} - static void imap4disconnect(Imap *imap) { @@ -806,8 +766,10 @@ imap4disconnect(Imap *imap) Bterm(&imap->bout); imap->binit = 0; } - close(imap->fd); - imap->fd = -1; + if(imap->fd >= 0){ + close(imap->fd); + imap->fd = -1; + } } char* @@ -827,7 +789,6 @@ static char* imap4dial(Imap *imap) { char *err, *port; - TLSconn conn; if(imap->fd >= 0){ imap4cmd(imap, "noop"); @@ -841,9 +802,8 @@ imap4dial(Imap *imap) port = "imap"; if((imap->fd = dial(netmkaddr(imap->host, "net", port), 0, 0, 0)) < 0) return imaperrstr(imap->host, port); - if(imap->flags & Fssl && starttls(imap, &conn) == -1){ + if(imap->flags & Fssl && (imap->fd = wraptls(imap->fd)) < 0){ err = imaperrstr(imap->host, port); - free(conn.cert); imap4disconnect(imap); return err; } @@ -1074,7 +1034,6 @@ out: static char* imap4ctl(Mailbox *mb, int argc, char **argv) { - char *a, *b; Imap *imap; imap = mb->aux; @@ -1085,26 +1044,6 @@ imap4ctl(Mailbox *mb, int argc, char **argv) imap->flags ^= Fdebug; return nil; } - if(strcmp(argv[0], "thumbprint") == 0){ - if(imap->thumb){ - freeThumbprints(imap->thumb); - imap->thumb = 0; - } - a = "/sys/lib/tls/mail"; - b = "/sys/lib/tls/mail.exclude"; - switch(argc){ - default: - return Eimap4ctl; - case 4: - b = argv[2]; - case 3: - a = argv[1]; - case 2: - break; - } - imap->thumb = initThumbprints(a, b); - return nil; - } if(argc == 2 && strcmp(argv[0], "uid") == 0){ uvlong l; Message *m; @@ -1253,9 +1192,6 @@ imap4mbox(Mailbox *mb, char *path) else imap->mbox = strdup(f[4]); mkmbox(imap, mb->path, mb->path + sizeof mb->path); - if(imap->flags & Fssl) - imap->thumb = initThumbprints("/sys/lib/tls/mail", "/sys/lib/tls/mail.exclude"); - mb->aux = imap; mb->sync = imap4sync; mb->close = imap4close; diff --git a/sys/src/cmd/upas/fs/mkfile b/sys/src/cmd/upas/fs/mkfile index a09222c05..765b4a66a 100644 --- a/sys/src/cmd/upas/fs/mkfile +++ b/sys/src/cmd/upas/fs/mkfile @@ -18,6 +18,7 @@ OFILES=\ remove.$O\ rename.$O\ strtotm.$O\ + tls.$O\ LIB=../common/libcommon.a$O\ diff --git a/sys/src/cmd/upas/fs/pop3.c b/sys/src/cmd/upas/fs/pop3.c index 750646d9b..cbe6082d2 100644 --- a/sys/src/cmd/upas/fs/pop3.c +++ b/sys/src/cmd/upas/fs/pop3.c @@ -32,7 +32,6 @@ struct Pop { Biobuf bout; int fd; char *lastline; /* from Brdstr */ - Thumbprint *thumb; }; static int @@ -120,38 +119,6 @@ pop3log(char *fmt, ...) return 0; } -static char* -pop3pushtls(Pop *pop) -{ - int fd; - uchar digest[SHA1dlen]; - TLSconn conn; - - memset(&conn, 0, sizeof conn); - // conn.trace = pop3log; - fd = tlsClient(pop->fd, &conn); - if(fd < 0) - return "tls error"; - if(conn.cert==nil || conn.certlen <= 0){ - close(fd); - return "server did not provide TLS certificate"; - } - sha1(conn.cert, conn.certlen, digest, nil); - if(!pop->thumb || !okThumbprint(digest, pop->thumb)){ - close(fd); - free(conn.cert); - eprint("pop3: server certificate %.*H not recognized\n", SHA1dlen, digest); - return "bad server certificate"; - } - free(conn.cert); - close(pop->fd); - pop->fd = fd; - pop->encrypted = 1; - Binit(&pop->bin, pop->fd, OREAD); - Binit(&pop->bout, pop->fd, OWRITE); - return nil; -} - /* * get capability list, possibly start tls */ @@ -182,8 +149,13 @@ pop3capa(Pop *pop) pop3cmd(pop, "STLS"); if(!isokay(s = pop3resp(pop))) return s; - if((s = pop3pushtls(pop)) != nil) - return s; + Bterm(&pop->bin); + Bterm(&pop->bout); + if((pop->fd = wraptls(pop->fd)) < 0) + return geterrstr(); + pop->encrypted = 1; + Binit(&pop->bin, pop->fd, OREAD); + Binit(&pop->bout, pop->fd, OWRITE); } return nil; } @@ -265,15 +237,11 @@ pop3dial(Pop *pop) if((pop->fd = dial(netmkaddr(pop->host, "net", pop->needssl ? "pop3s" : "pop3"), 0, 0, 0)) < 0) return geterrstr(); - - if(pop->needssl){ - if((err = pop3pushtls(pop)) != nil) - return err; - }else{ - Binit(&pop->bin, pop->fd, OREAD); - Binit(&pop->bout, pop->fd, OWRITE); - } - + if(pop->needssl && (pop->fd = wraptls(pop->fd)) < 0) + return geterrstr(); + pop->encrypted = pop->needssl; + Binit(&pop->bin, pop->fd, OREAD); + Binit(&pop->bout, pop->fd, OWRITE); if(err = pop3login(pop)) { close(pop->fd); return err; @@ -607,11 +575,6 @@ pop3ctl(Mailbox *mb, int argc, char **argv) return nil; } - if(argc==1 && strcmp(argv[0], "thumbprint")==0){ - if(pop->thumb) - freeThumbprints(pop->thumb); - pop->thumb = initThumbprints("/sys/lib/tls/mail", "/sys/lib/tls/mail.exclude"); - } if(strcmp(argv[0], "refresh")==0){ if(argc==1){ pop->refreshtime = 60; @@ -693,8 +656,6 @@ pop3mbox(Mailbox *mb, char *path) pop->needtls = poptls || apoptls; pop->refreshtime = 60; pop->notls = popnotls || apopnotls; - pop->thumb = initThumbprints("/sys/lib/tls/mail", "/sys/lib/tls/mail.exclude"); - mkmbox(pop, mb->path, mb->path + sizeof mb->path); mb->aux = pop; mb->sync = pop3sync; @@ -704,4 +665,3 @@ pop3mbox(Mailbox *mb, char *path) mb->addfrom = 1; return nil; } - diff --git a/sys/src/cmd/upas/fs/tls.c b/sys/src/cmd/upas/fs/tls.c new file mode 100644 index 000000000..2790d9cf1 --- /dev/null +++ b/sys/src/cmd/upas/fs/tls.c @@ -0,0 +1,39 @@ +#include "common.h" +#include +#include +#include "dat.h" + +int +wraptls(int ofd) +{ + uchar digest[SHA1dlen]; + Thumbprint *thumb; + TLSconn conn; + int fd; + + memset(&conn, 0, sizeof conn); + fd = tlsClient(ofd, &conn); + if(fd < 0){ + close(ofd); + return -1; + } + thumb = initThumbprints("/sys/lib/tls/mail", "/sys/lib/tls/mail.exclude"); + if(thumb != nil){ + if(conn.cert == nil || conn.certlen <= 0){ + werrstr("server did not provide TLS certificate"); + goto Err; + } + sha1(conn.cert, conn.certlen, digest, nil); + if(!okThumbprint(digest, thumb)){ + werrstr("server certificate %.*H not recognized", + SHA1dlen, digest); + Err: + close(fd); + fd = -1; + } + freeThumbprints(thumb); + } + free(conn.cert); + free(conn.sessionID); + return fd; +}