From 78dce192009bf5ee7511cc9fc848a725603a9099 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 09:53:07 -0400 Subject: [PATCH 01/30] Use PK11_TraverseCertsForNicknameInSlot after all. As of 76bc13c it doesn't appear to be leaky any more, and it does a better job of disinguishing between certificates with the same nickname than we did when doing it by hand. Signed-off-by: Peter Jones --- src/cms_common.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/cms_common.c b/src/cms_common.c index 644b44c..2d51979 100644 --- a/src/cms_common.c +++ b/src/cms_common.c @@ -465,23 +465,23 @@ err_slots: goto err_slots_errmsg; } + SECItem nickname = { + .data = (void *)cms->certname, + .len = strlen(cms->certname) + 1, + .type = siUTF8String, + }; struct cbdata cbdata = { .cert = NULL, .psle = psle, .pwdata = pwdata, }; - CERTCertListNode *node = NULL; - for (node = CERT_LIST_HEAD(certlist); !CERT_LIST_END(node,certlist); - node = CERT_LIST_NEXT(node)) { - if (strcmp(cms->certname, node->cert->nickname)) - continue; + status = PK11_TraverseCertsForNicknameInSlot(&nickname, psle->slot, + is_valid_cert, &cbdata); + if (cbdata.cert == NULL) + goto err_slots; - if (is_valid_cert(node->cert, &cbdata) == SECSuccess) { - cms->cert = CERT_DupCertificate(cbdata.cert); - break; - } - } + cms->cert = CERT_DupCertificate(cbdata.cert); PK11_DestroySlotListElement(slots, &psle); PK11_FreeSlotList(slots); -- 1.7.10.4 From 4fa59c775eb083a9df1b417a9eefe4ba01f2fc7f Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 09:54:10 -0400 Subject: [PATCH 02/30] Remove an unused field. Signed-off-by: Peter Jones --- src/pesign_context.c | 1 - src/pesign_context.h | 1 - 2 files changed, 2 deletions(-) diff --git a/src/pesign_context.c b/src/pesign_context.c index b4b201d..c6afda6 100644 --- a/src/pesign_context.c +++ b/src/pesign_context.c @@ -88,7 +88,6 @@ pesign_context_fini(pesign_context *ctx) ctx->cms_ctx = NULL; } - xfree(ctx->certname); xfree(ctx->privkeyfile); if (ctx->outpe) { diff --git a/src/pesign_context.h b/src/pesign_context.h index cabccf3..8f4e45a 100644 --- a/src/pesign_context.h +++ b/src/pesign_context.h @@ -58,7 +58,6 @@ typedef struct { Pe *outpe; char *privkeyfile; - char *certname; cms_context *cms_ctx; int flags; -- 1.7.10.4 From 22a46c4c83f73e02c93eac6bfe314e56d5854f2c Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 09:55:02 -0400 Subject: [PATCH 03/30] Make sure we actually look up the certificate when not in daemon mode. Signed-off-by: Peter Jones --- src/pesign.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/pesign.c b/src/pesign.c index 108994e..4ddf636 100644 --- a/src/pesign.c +++ b/src/pesign.c @@ -500,12 +500,6 @@ main(int argc, char *argv[]) POPT_TABLEEND }; - tokenname = strdup(tokenname); - if (!tokenname) { - fprintf(stderr, "could not allocate memory: %m\n"); - exit(1); - } - if (!daemon) { SECStatus status = NSS_Init("/etc/pki/pesign"); if (status != SECSuccess) { @@ -521,8 +515,6 @@ main(int argc, char *argv[]) exit(1); } - ctx.cms_ctx->certname = certname ? strdup(certname) : NULL; - optCon = poptGetContext("pesign", argc, (const char **)argv, options,0); rc = poptReadDefaultConfig(optCon, 0); @@ -559,7 +551,21 @@ main(int argc, char *argv[]) exit(!is_help); } - ctx.cms_ctx->tokenname = tokenname; + ctx.cms_ctx->tokenname = tokenname ? + PORT_ArenaStrdup(ctx.cms_ctx->arena, tokenname) : NULL; + if (!ctx.cms_ctx->tokenname) { + fprintf(stderr, "could not allocate token name: %s\n", + PORT_ErrorToString(PORT_GetError())); + exit(1); + } + + ctx.cms_ctx->certname = certname ? + PORT_ArenaStrdup(ctx.cms_ctx->arena, certname) : NULL; + if (!ctx.cms_ctx->certname) { + fprintf(stderr, "could not allocate certificate name: %s\n", + PORT_ErrorToString(PORT_GetError())); + exit(1); + } int action = 0; if (daemon) -- 1.7.10.4 From 830e67df9da53def8086d9854068af01b437a553 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 09:54:37 -0400 Subject: [PATCH 04/30] Free the certificate list we make once we're done using it. Signed-off-by: Peter Jones --- src/wincert.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wincert.c b/src/wincert.c index fe666c5..b487dc5 100644 --- a/src/wincert.c +++ b/src/wincert.c @@ -74,6 +74,7 @@ finalize_signatures(cms_context *cms, Pe *pe) free(clist); return -1; } + free(clist); return 0; } -- 1.7.10.4 From a588656e3af22e63822c7a8b2afae5f0c3eefe2c Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 10:35:41 -0400 Subject: [PATCH 05/30] Fix check for allocations on tokenname,certname. If we didn't have anything to start with, we won't have anything when we're done... Signed-off-by: Peter Jones --- src/pesign.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pesign.c b/src/pesign.c index 4ddf636..c7b23cf 100644 --- a/src/pesign.c +++ b/src/pesign.c @@ -553,7 +553,7 @@ main(int argc, char *argv[]) ctx.cms_ctx->tokenname = tokenname ? PORT_ArenaStrdup(ctx.cms_ctx->arena, tokenname) : NULL; - if (!ctx.cms_ctx->tokenname) { + if (tokenname && !ctx.cms_ctx->tokenname) { fprintf(stderr, "could not allocate token name: %s\n", PORT_ErrorToString(PORT_GetError())); exit(1); @@ -561,7 +561,7 @@ main(int argc, char *argv[]) ctx.cms_ctx->certname = certname ? PORT_ArenaStrdup(ctx.cms_ctx->arena, certname) : NULL; - if (!ctx.cms_ctx->certname) { + if (certname && !ctx.cms_ctx->certname) { fprintf(stderr, "could not allocate certificate name: %s\n", PORT_ErrorToString(PORT_GetError())); exit(1); -- 1.7.10.4 From f30c6298941f70e9d8b02833b90660ae7a6e4351 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 10:57:20 -0400 Subject: [PATCH 06/30] Free the pid string once we're done writing it. Signed-off-by: Peter Jones --- src/daemon.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/daemon.c b/src/daemon.c index daa2dbf..245491f 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -842,6 +842,7 @@ err: if (rc < 0) goto err; + free(pidstr); close(fd); } -- 1.7.10.4 From 87b8175925d217abd1ec91e800cbcd969a3e0786 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 11:26:04 -0400 Subject: [PATCH 07/30] Only try to register OIDs once. Signed-off-by: Peter Jones --- src/cms_common.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/cms_common.c b/src/cms_common.c index 2d51979..6219a2a 100644 --- a/src/cms_common.c +++ b/src/cms_common.c @@ -179,11 +179,15 @@ cms_common_log(cms_context *ctx, int priority, char *fmt, ...) int cms_context_init(cms_context *cms) { + static int first_time = 1; memset(cms, '\0', sizeof (*cms)); - SECStatus status = register_oids(cms); - if (status != SECSuccess) - return -1; + if (first_time) { + SECStatus status = register_oids(cms); + if (status != SECSuccess) + return -1; + first_time = 0; + } cms->log = cms_common_log; -- 1.7.10.4 From e7a2d0d1fd1f1811fe48ee8b96c2d2fc12238ac7 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 14:33:35 -0400 Subject: [PATCH 08/30] Don't set up digests in cms_context_init. Move digest setup out of cms_context_init, so we can avoid leaking the reference to the digests by not having them in ctx->backup_cms in the daemon. Signed-off-by: Peter Jones --- src/cms_common.c | 9 ++------- src/cms_common.h | 3 +++ src/daemon.c | 27 +++++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/cms_common.c b/src/cms_common.c index 6219a2a..a8e34dd 100644 --- a/src/cms_common.c +++ b/src/cms_common.c @@ -97,7 +97,7 @@ digest_get_digest_size(cms_context *cms) } -static int +int setup_digests(cms_context *cms) { struct digest *digests = NULL; @@ -133,7 +133,7 @@ err: return -1; } -static void +void teardown_digests(cms_context *ctx) { struct digest *digests = ctx->digests; @@ -199,11 +199,6 @@ cms_context_init(cms_context *cms) return -1; } - int rc = setup_digests(cms); - if (rc < 0) { - PORT_FreeArena(cms->arena, PR_TRUE); - return -1; - } cms->selected_digest = -1; return 0; diff --git a/src/cms_common.h b/src/cms_common.h index fc80fa3..830427e 100644 --- a/src/cms_common.h +++ b/src/cms_common.h @@ -86,6 +86,9 @@ extern int cms_context_alloc(cms_context **ctxp); extern int cms_context_init(cms_context *ctx); extern void cms_context_fini(cms_context *ctx); +extern int setup_digests(cms_context *cms); +extern void teardown_digests(cms_context *ctx); + extern int generate_octet_string(cms_context *ctx, SECItem *encoded, SECItem *original); extern int generate_object_id(cms_context *ctx, SECItem *encoded, diff --git a/src/daemon.c b/src/daemon.c index 245491f..645722e 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -143,6 +143,15 @@ handle_unlock_token(context *ctx, struct pollfd *pollfd, socklen_t size) return; } + rc = setup_digests(ctx->cms); + if (rc < 0) { + ctx->backup_cms->log(ctx->backup_cms, ctx->priority|LOG_NOTICE, + "Could not initialize digests: %s\n", + PORT_ErrorToString(PORT_GetError())); + send_response(ctx, ctx->backup_cms, pollfd, rc); + return; + } + steal_from_cms(ctx->backup_cms, ctx->cms); if (!buffer) { @@ -491,6 +500,15 @@ handle_sign_attached(context *ctx, struct pollfd *pollfd, socklen_t size) if (rc < 0) return; + rc = setup_digests(ctx->cms); + if (rc < 0) { + ctx->backup_cms->log(ctx->backup_cms, ctx->priority|LOG_NOTICE, + "Could not initialize digests: %s\n", + PORT_ErrorToString(PORT_GetError())); + send_response(ctx, ctx->backup_cms, pollfd, rc); + return; + } + steal_from_cms(ctx->backup_cms, ctx->cms); handle_signing(ctx, pollfd, size, 1); @@ -506,6 +524,15 @@ handle_sign_detached(context *ctx, struct pollfd *pollfd, socklen_t size) if (rc < 0) return; + rc = setup_digests(ctx->cms); + if (rc < 0) { + ctx->backup_cms->log(ctx->backup_cms, ctx->priority|LOG_NOTICE, + "Could not initialize digests: %s\n", + PORT_ErrorToString(PORT_GetError())); + send_response(ctx, ctx->backup_cms, pollfd, rc); + return; + } + steal_from_cms(ctx->backup_cms, ctx->cms); handle_signing(ctx, pollfd, size, 0); -- 1.7.10.4 From 1538adb13dc9bf4277fa7fd1b2d2c01a606b516c Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 12:05:29 -0400 Subject: [PATCH 09/30] Check for NSS_Shutdown() failure. Signed-off-by: Peter Jones --- src/daemon.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/daemon.c b/src/daemon.c index 645722e..c4f6fb7 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -994,6 +994,11 @@ daemonize(cms_context *cms_ctx, int do_fork) rc = handle_events(&ctx); - NSS_Shutdown(); + status = NSS_Shutdown(); + if (status != SECSuccess) { + fprintf(stderr, "NSS_Shutdown failed: %s\n", + PORT_ErrorToString(PORT_GetError())); + exit(1); + } return rc; } -- 1.7.10.4 From 77f0fb3fbfab97ff3e768e2811c6d73ef49472f6 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 12:17:39 -0400 Subject: [PATCH 10/30] Don't destroy stdin/stdout/stderr if we don't fork. I like being able to read my error messages. Signed-off-by: Peter Jones --- src/daemon.c | 59 ++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/src/daemon.c b/src/daemon.c index c4f6fb7..f44f069 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -917,33 +917,38 @@ daemonize(cms_context *cms_ctx, int do_fork) exit(1); } - int fd = open("/dev/zero", O_RDONLY); - close(STDIN_FILENO); - rc = dup2(fd, STDIN_FILENO); - if (rc < 0) { - ctx.backup_cms->log(ctx.backup_cms, ctx.priority|LOG_ERR, - "pesignd: could not set up standard input: %m"); - exit(1); - } - close(fd); + if (do_fork) { + int fd = open("/dev/zero", O_RDONLY); + close(STDIN_FILENO); + rc = dup2(fd, STDIN_FILENO); + if (rc < 0) { + ctx.backup_cms->log(ctx.backup_cms, + ctx.priority|LOG_ERR, + "pesignd: could not set up standard input: %m"); + exit(1); + } + close(fd); - fd = open("/dev/null", O_WRONLY); - close(STDOUT_FILENO); - rc = dup2(fd, STDOUT_FILENO); - if (rc < 0) { - ctx.backup_cms->log(ctx.backup_cms, ctx.priority|LOG_ERR, - "pesignd: could not set up standard output: %m"); - exit(1); - } + fd = open("/dev/null", O_WRONLY); + close(STDOUT_FILENO); + rc = dup2(fd, STDOUT_FILENO); + if (rc < 0) { + ctx.backup_cms->log(ctx.backup_cms, + ctx.priority|LOG_ERR, + "pesignd: could not set up standard output: %m"); + exit(1); + } - close(STDERR_FILENO); - rc = dup2(fd, STDERR_FILENO); - if (rc < 0) { - ctx.backup_cms->log(ctx.backup_cms, ctx.priority|LOG_ERR, - "pesignd: could not set up standard error: %m"); - exit(1); + close(STDERR_FILENO); + rc = dup2(fd, STDERR_FILENO); + if (rc < 0) { + ctx.backup_cms->log(ctx.backup_cms, + ctx.priority|LOG_ERR, + "pesignd: could not set up standard error: %m"); + exit(1); + } + close(fd); } - close(fd); prctl(PR_SET_NAME, "pesignd", 0, 0, 0); @@ -990,13 +995,15 @@ daemonize(cms_context *cms_ctx, int do_fork) cms_set_pw_callback(ctx.backup_cms, get_password_fail); cms_set_pw_data(ctx.backup_cms, NULL); - ctx.backup_cms->log = daemon_logger; + if (do_fork) + ctx.backup_cms->log = daemon_logger; rc = handle_events(&ctx); status = NSS_Shutdown(); if (status != SECSuccess) { - fprintf(stderr, "NSS_Shutdown failed: %s\n", + ctx.backup_cms->log(ctx.backup_cms, ctx.priority|LOG_ERR, + "NSS_Shutdown failed: %s\n", PORT_ErrorToString(PORT_GetError())); exit(1); } -- 1.7.10.4 From dc142c3ee418ba55b2d86d39c81c867b8a07447b Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 14:41:18 -0400 Subject: [PATCH 11/30] Do register_oids() where we're doing NSS_Init() Signed-off-by: Peter Jones --- src/cms_common.c | 8 -------- src/daemon.c | 7 +++++++ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/cms_common.c b/src/cms_common.c index a8e34dd..6188e6e 100644 --- a/src/cms_common.c +++ b/src/cms_common.c @@ -179,16 +179,8 @@ cms_common_log(cms_context *ctx, int priority, char *fmt, ...) int cms_context_init(cms_context *cms) { - static int first_time = 1; memset(cms, '\0', sizeof (*cms)); - if (first_time) { - SECStatus status = register_oids(cms); - if (status != SECSuccess) - return -1; - first_time = 0; - } - cms->log = cms_common_log; cms->arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE); diff --git a/src/daemon.c b/src/daemon.c index f44f069..085cfec 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -917,6 +917,13 @@ daemonize(cms_context *cms_ctx, int do_fork) exit(1); } + status = register_oids(ctx.backup_cms); + if (status != SECSuccess) { + ctx.backup_cms->log(ctx.backup_cms, ctx.priority|LOG_NOTICE, + "Could not register OIDs\n"); + exit(1); + } + if (do_fork) { int fd = open("/dev/zero", O_RDONLY); close(STDIN_FILENO); -- 1.7.10.4 From 02e9940ee774b71ad899222737f58ef435badda3 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 14:43:58 -0400 Subject: [PATCH 12/30] Make daemon shutdown actually close the NSS databases and whatnot. Signed-off-by: Peter Jones --- src/daemon.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/daemon.c b/src/daemon.c index 085cfec..dd215f8 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -125,8 +125,7 @@ handle_kill_daemon(context *ctx, struct pollfd *pollfd, socklen_t size) ctx->backup_cms->log(ctx->backup_cms, ctx->priority|LOG_NOTICE, "pesignd exiting (pid %d)", getpid()); - cms_context_fini(ctx->backup_cms); - exit(0); + should_exit = 1; } static void @@ -625,6 +624,16 @@ handle_event(context *ctx, struct pollfd *pollfd) return 0; } +static void +do_shutdown(context *ctx, int nsockets, struct pollfd *pollfds) +{ + for (int i = 0; i < nsockets; i++) + close(pollfds[i].fd); + free(pollfds); + + xfree(ctx->errstr); +} + static int handle_events(context *ctx) { @@ -643,9 +652,14 @@ handle_events(context *ctx) pollfds[0].events = POLLIN|POLLPRI|POLLHUP; while (1) { + if (should_exit != 0) { +shutdown: + do_shutdown(ctx, nsockets, pollfds); + return 0; + } rc = ppoll(pollfds, nsockets, NULL, NULL); if (should_exit != 0) - exit(0); + goto shutdown; if (rc < 0) { ctx->backup_cms->log(ctx->backup_cms, ctx->priority|LOG_WARNING, -- 1.7.10.4 From 05776781dd3bfb30558803b3fc740f59ab5f546c Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 15:26:23 -0400 Subject: [PATCH 13/30] Use PORT_ArenaStrdup() where appropriate. Signed-off-by: Peter Jones --- src/daemon.c | 15 ++++++--------- src/pesign.c | 12 ++++++------ 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/daemon.c b/src/daemon.c index dd215f8..794b6b5 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -205,9 +205,8 @@ malformed: "pesignd: unlocking token \"%s\"", tn->value); /* authenticating with nss frees this ... best API ever. */ - ctx->cms->tokenname = PORT_ArenaZAlloc(ctx->cms->arena, - strlen((char *)tn->value)); - strcpy(ctx->cms->tokenname, (char *)tn->value); + ctx->cms->tokenname = PORT_ArenaStrdup(ctx->cms->arena, + (char *)tn->value); if (!ctx->cms->tokenname) goto oom; @@ -393,9 +392,8 @@ malformed: n -= tn->size; /* authenticating with nss frees these ... best API ever. */ - ctx->cms->tokenname = PORT_ArenaZAlloc(ctx->cms->arena, - strlen((char *)tn->value)); - strcpy(ctx->cms->tokenname, (char *)tn->value); + ctx->cms->tokenname = PORT_ArenaStrdup(ctx->cms->arena, + (char *)tn->value); if (!ctx->cms->tokenname) goto oom; @@ -406,9 +404,8 @@ malformed: if (n < cn->size) goto malformed; - ctx->cms->certname = PORT_ArenaZAlloc(ctx->cms->arena, - strlen((char *)cn->value)); - strcpy(ctx->cms->certname, (char *)cn->value); + ctx->cms->certname = PORT_ArenaStrdup(ctx->cms->arena, + (char *)cn->value); if (!ctx->cms->certname) goto oom; diff --git a/src/pesign.c b/src/pesign.c index c7b23cf..819cee0 100644 --- a/src/pesign.c +++ b/src/pesign.c @@ -551,17 +551,17 @@ main(int argc, char *argv[]) exit(!is_help); } - ctx.cms_ctx->tokenname = tokenname ? - PORT_ArenaStrdup(ctx.cms_ctx->arena, tokenname) : NULL; - if (tokenname && !ctx.cms_ctx->tokenname) { + ctxp->cms_ctx->tokenname = tokenname ? + PORT_ArenaStrdup(ctxp->cms_ctx->arena, tokenname) : NULL; + if (tokenname && !ctxp->cms_ctx->tokenname) { fprintf(stderr, "could not allocate token name: %s\n", PORT_ErrorToString(PORT_GetError())); exit(1); } - ctx.cms_ctx->certname = certname ? - PORT_ArenaStrdup(ctx.cms_ctx->arena, certname) : NULL; - if (certname && !ctx.cms_ctx->certname) { + ctxp->cms_ctx->certname = certname ? + PORT_ArenaStrdup(ctxp->cms_ctx->arena, certname) : NULL; + if (certname && !ctxp->cms_ctx->certname) { fprintf(stderr, "could not allocate certificate name: %s\n", PORT_ErrorToString(PORT_GetError())); exit(1); -- 1.7.10.4 From b47c02b5b67b9d66cd67897767a831530594dce4 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 15:27:03 -0400 Subject: [PATCH 14/30] [daemon] Make sure inpe is initialized before all error handling. find_certificate() and set_up_inpe() errors wind up being at the same place, which means when find_certificate is called, inpe already must be NULL. Signed-off-by: Peter Jones --- src/daemon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/daemon.c b/src/daemon.c index 794b6b5..4259a3a 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -358,6 +358,7 @@ handle_signing(context *ctx, struct pollfd *pollfd, socklen_t size, struct iovec iov; ssize_t n; char *buffer = malloc(size); + Pe *inpe = NULL; if (!buffer) { oom: @@ -429,7 +430,6 @@ malformed: goto finish; } - Pe *inpe = NULL; rc = set_up_inpe(ctx, infd, &inpe); if (rc < 0) goto finish; -- 1.7.10.4 From ed7eeb208562019489a28ec485fa85b5c2e08bfc Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 15:31:15 -0400 Subject: [PATCH 15/30] Allocate pesign_context rather than having it on the stack. This way it won't try to re-initialize cms_context when it's cleaned up. Signed-off-by: Peter Jones --- src/pesign.c | 152 +++++++++++++++++++++++++++++----------------------------- 1 file changed, 77 insertions(+), 75 deletions(-) diff --git a/src/pesign.c b/src/pesign.c index 819cee0..4c18b95 100644 --- a/src/pesign.c +++ b/src/pesign.c @@ -426,7 +426,7 @@ main(int argc, char *argv[]) { int rc; - pesign_context ctx, *ctxp = &ctx; + pesign_context *ctxp; int list = 0; int remove = 0; @@ -437,40 +437,47 @@ main(int argc, char *argv[]) char *tokenname = "NSS Certificate DB"; char *certname = NULL; + rc = pesign_context_new(&ctxp); + if (rc < 0) { + fprintf(stderr, "Could not initialize context: %m\n"); + exit(1); + } + poptContext optCon; struct poptOption options[] = { {NULL, '\0', POPT_ARG_INTL_DOMAIN, "pesign" }, - {"in", 'i', POPT_ARG_STRING, &ctx.infile, 0, + {"in", 'i', POPT_ARG_STRING, &ctxp->infile, 0, "specify input file", ""}, - {"out", 'o', POPT_ARG_STRING, &ctx.outfile, 0, + {"out", 'o', POPT_ARG_STRING, &ctxp->outfile, 0, "specify output file", "" }, {"certficate", 'c', POPT_ARG_STRING, &certname, 0, "specify certificate nickname", "" }, - {"privkey", 'p', POPT_ARG_STRING, &ctx.privkeyfile, 0, + {"privkey", 'p', POPT_ARG_STRING, &ctxp->privkeyfile, 0, "specify private key file", "" }, - {"force", 'f', POPT_ARG_VAL, &ctx.force, 1, + {"force", 'f', POPT_ARG_VAL, &ctxp->force, 1, "force overwriting of output file", NULL }, - {"sign", 's', POPT_ARG_VAL, &ctx.sign, 1, + {"sign", 's', POPT_ARG_VAL, &ctxp->sign, 1, "create a new signature", NULL }, - {"hash", 'h', POPT_ARG_VAL, &ctx.hash, 1, "hash binary", NULL }, + {"hash", 'h', POPT_ARG_VAL, &ctxp->hash, 1, "hash binary", NULL }, {"digest_type", 'd', POPT_ARG_STRING|POPT_ARGFLAG_SHOW_DEFAULT, &digest_name, 0, "digest type to use for pe hash" }, {"import-signed-certificate", 'm', POPT_ARG_STRING|POPT_ARGFLAG_DOC_HIDDEN, - &ctx.insig, 0,"import signature from file", "" }, + &ctxp->insig, 0,"import signature from file", "" }, {"export-signed-attributes", 'E', POPT_ARG_STRING|POPT_ARGFLAG_DOC_HIDDEN, - &ctx.outsattrs, 0, "export signed attributes to file", + &ctxp->outsattrs, 0, "export signed attributes to file", "" }, {"import-signed-attributes", 'I', POPT_ARG_STRING|POPT_ARGFLAG_DOC_HIDDEN, - &ctx.insattrs, 0, "import signed attributes from file", + &ctxp->insattrs, 0, + "import signed attributes from file", "" }, {"import-raw-signature", 'R', - POPT_ARG_STRING|POPT_ARGFLAG_DOC_HIDDEN, &ctx.rawsig, + POPT_ARG_STRING|POPT_ARGFLAG_DOC_HIDDEN, &ctxp->rawsig, 0, "import raw signature from file", "" }, - {"signature-number", 'u', POPT_ARG_INT, &ctx.signum, -1, + {"signature-number", 'u', POPT_ARG_INT, &ctxp->signum, -1, "specify which signature to operate on",""}, {"list-signatures", 'l', POPT_ARG_VAL|POPT_ARGFLAG_DOC_HIDDEN, @@ -483,13 +490,14 @@ main(int argc, char *argv[]) "remove signature" }, {"export-signature", 'e', POPT_ARG_STRING|POPT_ARGFLAG_DOC_HIDDEN, - &ctx.outsig, 0,"export signature to file", "" }, + &ctxp->outsig, 0, + "export signature to file", "" }, {"export-pubkey", 'K', POPT_ARG_STRING, - &ctx.outkey, 0, "export pubkey to file", "" }, + &ctxp->outkey, 0, "export pubkey to file", "" }, {"export-cert", 'C', POPT_ARG_STRING, - &ctx.outcert, 0, "export signing cert to file", + &ctxp->outcert, 0, "export signing cert to file", "" }, - {"ascii-armor", 'a', POPT_ARG_VAL, &ctx.ascii, 1, + {"ascii-armor", 'a', POPT_ARG_VAL, &ctxp->ascii, 1, "use ascii armoring", NULL }, {"daemonize", 'D', POPT_ARG_VAL, &daemon, 1, "run as a daemon process", NULL }, @@ -509,12 +517,6 @@ main(int argc, char *argv[]) } } - rc = pesign_context_init(ctxp); - if (rc < 0) { - fprintf(stderr, "Could not initialize context: %m\n"); - exit(1); - } - optCon = poptGetContext("pesign", argc, (const char **)argv, options,0); rc = poptReadDefaultConfig(optCon, 0); @@ -571,25 +573,25 @@ main(int argc, char *argv[]) if (daemon) action |= DAEMONIZE; - if (ctx.rawsig) + if (ctxp->rawsig) action |= IMPORT_RAW_SIGNATURE; - if (ctx.insattrs) + if (ctxp->insattrs) action |= IMPORT_SATTRS; - if (ctx.outsattrs) + if (ctxp->outsattrs) action |= EXPORT_SATTRS; - - if (ctx.insig) + + if (ctxp->insig) action |= IMPORT_SIGNATURE; - if (ctx.outkey) + if (ctxp->outkey) action |= EXPORT_PUBKEY; - if (ctx.outcert) + if (ctxp->outcert) action |= EXPORT_CERT; - if (ctx.outsig) + if (ctxp->outsig) action |= EXPORT_SIGNATURE; if (remove != 0) @@ -598,13 +600,13 @@ main(int argc, char *argv[]) if (list != 0) action |= LIST_SIGNATURES; - if (ctx.sign) { + if (ctxp->sign) { action |= GENERATE_SIGNATURE; if (!(action & EXPORT_SIGNATURE)) action |= IMPORT_SIGNATURE; } - if (ctx.hash) + if (ctxp->hash) action |= GENERATE_DIGEST|PRINT_DIGEST; ssize_t sigspace = 0; @@ -620,11 +622,11 @@ main(int argc, char *argv[]) */ case IMPORT_RAW_SIGNATURE|IMPORT_SATTRS: check_inputs(ctxp); - rc = find_certificate(ctx.cms_ctx); + rc = find_certificate(ctxp->cms_ctx); if (rc < 0) { fprintf(stderr, "pesign: Could not find " "certificate %s\n", - ctx.cms_ctx->certname); + ctxp->cms_ctx->certname); exit(1); } open_rawsig_input(ctxp); @@ -636,19 +638,19 @@ main(int argc, char *argv[]) open_input(ctxp); open_output(ctxp); close_input(ctxp); - generate_digest(ctx.cms_ctx, ctx.outpe); - sigspace = calculate_signature_space(ctx.cms_ctx, - ctx.outpe); - allocate_signature_space(ctx.outpe, sigspace); - generate_signature(ctx.cms_ctx); - insert_signature(ctx.cms_ctx, ctx.signum); - finalize_signatures(ctx.cms_ctx, ctx.outpe); + generate_digest(ctxp->cms_ctx, ctxp->outpe); + sigspace = calculate_signature_space(ctxp->cms_ctx, + ctxp->outpe); + allocate_signature_space(ctxp->outpe, sigspace); + generate_signature(ctxp->cms_ctx); + insert_signature(ctxp->cms_ctx, ctxp->signum); + finalize_signatures(ctxp->cms_ctx, ctxp->outpe); close_output(ctxp); break; case EXPORT_SATTRS: open_input(ctxp); open_sattr_output(ctxp); - generate_digest(ctx.cms_ctx, ctx.inpe); + generate_digest(ctxp->cms_ctx, ctxp->inpe); generate_sattr_blob(ctxp); close_sattr_output(ctxp); close_input(ctxp); @@ -666,22 +668,22 @@ main(int argc, char *argv[]) close_output(ctxp); break; case EXPORT_PUBKEY: - rc = find_certificate(ctx.cms_ctx); + rc = find_certificate(ctxp->cms_ctx); if (rc < 0) { fprintf(stderr, "pesign: Could not find " "certificate %s\n", - ctx.cms_ctx->certname); + ctxp->cms_ctx->certname); exit(1); } open_pubkey_output(ctxp); export_pubkey(ctxp); break; case EXPORT_CERT: - rc = find_certificate(ctx.cms_ctx); + rc = find_certificate(ctxp->cms_ctx); if (rc < 0) { fprintf(stderr, "pesign: Could not find " "certificate %s\n", - ctx.cms_ctx->certname); + ctxp->cms_ctx->certname); exit(1); } open_cert_output(ctxp); @@ -691,21 +693,21 @@ main(int argc, char *argv[]) case EXPORT_SIGNATURE: open_input(ctxp); open_sig_output(ctxp); - if (ctx.signum > ctx.cms_ctx->num_signatures) { + if (ctxp->signum > ctxp->cms_ctx->num_signatures) { fprintf(stderr, "Invalid signature number.\n"); exit(1); } - if (ctx.signum < 0) - ctx.signum = 0; - if (ctx.signum >= ctx.cms_ctx->num_signatures) { + if (ctxp->signum < 0) + ctxp->signum = 0; + if (ctxp->signum >= ctxp->cms_ctx->num_signatures) { fprintf(stderr, "No valid signature #%d.\n", - ctx.signum); + ctxp->signum); exit(1); } - memcpy(&ctx.cms_ctx->newsig, - ctx.cms_ctx->signatures[ctx.signum], - sizeof (ctx.cms_ctx->newsig)); - export_signature(ctx.cms_ctx, ctx.outsigfd, ctx.ascii); + memcpy(&ctxp->cms_ctx->newsig, + ctxp->cms_ctx->signatures[ctxp->signum], + sizeof (ctxp->cms_ctx->newsig)); + export_signature(ctxp->cms_ctx, ctxp->outsigfd, ctxp->ascii); close_input(ctxp); close_sig_output(ctxp); break; @@ -715,11 +717,11 @@ main(int argc, char *argv[]) open_input(ctxp); open_output(ctxp); close_input(ctxp); - if (ctx.signum > ctx.cms_ctx->num_signatures) { + if (ctxp->signum > ctxp->cms_ctx->num_signatures) { fprintf(stderr, "Invalid signature number.\n"); exit(1); } - remove_signature(&ctx); + remove_signature(ctxp); close_output(ctxp); break; /* list signatures in the binary */ @@ -729,49 +731,49 @@ main(int argc, char *argv[]) break; case GENERATE_DIGEST|PRINT_DIGEST: open_input(ctxp); - generate_digest(ctx.cms_ctx, ctx.inpe); + generate_digest(ctxp->cms_ctx, ctxp->inpe); print_digest(ctxp); break; /* generate a signature and save it in a separate file */ case EXPORT_SIGNATURE|GENERATE_SIGNATURE: - rc = find_certificate(ctx.cms_ctx); + rc = find_certificate(ctxp->cms_ctx); if (rc < 0) { fprintf(stderr, "pesign: Could not find " "certificate %s\n", - ctx.cms_ctx->certname); + ctxp->cms_ctx->certname); exit(1); } open_input(ctxp); open_sig_output(ctxp); - generate_digest(ctx.cms_ctx, ctx.inpe); - generate_signature(ctx.cms_ctx); - export_signature(ctx.cms_ctx, ctx.outsigfd, ctx.ascii); + generate_digest(ctxp->cms_ctx, ctxp->inpe); + generate_signature(ctxp->cms_ctx); + export_signature(ctxp->cms_ctx, ctxp->outsigfd, ctxp->ascii); break; /* generate a signature and embed it in the binary */ case IMPORT_SIGNATURE|GENERATE_SIGNATURE: check_inputs(ctxp); - rc = find_certificate(ctx.cms_ctx); + rc = find_certificate(ctxp->cms_ctx); if (rc < 0) { fprintf(stderr, "pesign: Could not find " "certificate %s\n", - ctx.cms_ctx->certname); + ctxp->cms_ctx->certname); exit(1); } open_input(ctxp); open_output(ctxp); close_input(ctxp); - generate_digest(ctx.cms_ctx, ctx.outpe); - sigspace = calculate_signature_space(ctx.cms_ctx, - ctx.outpe); - allocate_signature_space(ctx.outpe, sigspace); - generate_digest(ctx.cms_ctx, ctx.outpe); - generate_signature(ctx.cms_ctx); - insert_signature(ctx.cms_ctx, ctx.signum); - finalize_signatures(ctx.cms_ctx, ctx.outpe); + generate_digest(ctxp->cms_ctx, ctxp->outpe); + sigspace = calculate_signature_space(ctxp->cms_ctx, + ctxp->outpe); + allocate_signature_space(ctxp->outpe, sigspace); + generate_digest(ctxp->cms_ctx, ctxp->outpe); + generate_signature(ctxp->cms_ctx); + insert_signature(ctxp->cms_ctx, ctxp->signum); + finalize_signatures(ctxp->cms_ctx, ctxp->outpe); close_output(ctxp); break; case DAEMONIZE: - rc = daemonize(ctx.cms_ctx, fork); + rc = daemonize(ctxp->cms_ctx, fork); break; default: fprintf(stderr, "Incompatible flags (0x%08x): ", action); @@ -782,7 +784,7 @@ main(int argc, char *argv[]) fprintf(stderr, "\n"); exit(1); } - pesign_context_fini(&ctx); + pesign_context_free(ctxp); NSS_Shutdown(); return (rc < 0); -- 1.7.10.4 From 0466bb97d6ba8fb55fbeccaef86769c0c2bbd642 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 15:32:57 -0400 Subject: [PATCH 16/30] [pesign] initialize nss only if we're not a daemon. If it's a deamon, NSS_Init, register_oids, and setup_digests will be done in the daemon code, not in the normal tool code. Signed-off-by: Peter Jones --- src/pesign.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/pesign.c b/src/pesign.c index 4c18b95..23aa992 100644 --- a/src/pesign.c +++ b/src/pesign.c @@ -508,15 +508,6 @@ main(int argc, char *argv[]) POPT_TABLEEND }; - if (!daemon) { - SECStatus status = NSS_Init("/etc/pki/pesign"); - if (status != SECSuccess) { - fprintf(stderr, "Could not initialize nss: %s\n", - PORT_ErrorToString(PORT_GetError())); - exit(1); - } - } - optCon = poptGetContext("pesign", argc, (const char **)argv, options,0); rc = poptReadDefaultConfig(optCon, 0); @@ -543,7 +534,29 @@ main(int argc, char *argv[]) poptFreeContext(optCon); - rc = set_digest_parameters(ctx.cms_ctx, digest_name); + if (!daemon) { + SECStatus status = NSS_Init("/etc/pki/pesign"); + if (status != SECSuccess) { + fprintf(stderr, "Could not initialize nss: %s\n", + PORT_ErrorToString(PORT_GetError())); + exit(1); + } + + status = register_oids(ctxp->cms_ctx); + if (status != SECSuccess) { + fprintf(stderr, "Could not register OIDs\n"); + exit(1); + } + + rc = setup_digests(ctxp->cms_ctx); + if (rc < 0) { + fprintf(stderr, "Could not initialize digests: %s\n", + PORT_ErrorToString(PORT_GetError())); + exit(1); + } + } + + rc = set_digest_parameters(ctxp->cms_ctx, digest_name); int is_help = strcmp(digest_name, "help") ? 0 : 1; if (rc < 0) { if (!is_help) { -- 1.7.10.4 From a73cfcd17f4d1bfc2055c973f06142443c072336 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 15:34:00 -0400 Subject: [PATCH 17/30] Handle errors on pesign_context_init() Signed-off-by: Peter Jones --- src/pesign_context.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/pesign_context.c b/src/pesign_context.c index c6afda6..cbd929f 100644 --- a/src/pesign_context.c +++ b/src/pesign_context.c @@ -37,7 +37,9 @@ pesign_context_new(pesign_context **ctx) if (!context) return -1; - pesign_context_init(context); + rc = pesign_context_init(context); + if (rc < 0) + return rc; context->flags |= PESIGN_C_ALLOCATED; *ctx = context; -- 1.7.10.4 From a80f9aac4458caa32fae745824b2c73161dcd66f Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 16:35:43 -0400 Subject: [PATCH 18/30] Add sanity checking to make sure we don't emit uninitialized hashes. Signed-off-by: Peter Jones --- src/cms_common.c | 15 ++++++++++++++- src/content_info.c | 11 +++++++++++ src/signer_info.c | 5 +++++ src/util.h | 13 +++++++++++++ 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/cms_common.c b/src/cms_common.c index 6188e6e..0c8ad8c 100644 --- a/src/cms_common.c +++ b/src/cms_common.c @@ -532,6 +532,10 @@ generate_empty_sequence(cms_context *cms, SECItem *encoded) int generate_octet_string(cms_context *cms, SECItem *encoded, SECItem *original) { + if (content_is_empty(original->data, original->len)) { + cms->log(cms, LOG_ERR, "content is empty, not encoding"); + return -1; + } if (SEC_ASN1EncodeItem(cms->arena, encoded, original, SEC_OctetStringTemplate) == NULL) return -1; @@ -942,7 +946,16 @@ generate_signature(cms_context *cms) { int rc = 0; - assert(cms->digests[cms->selected_digest].pe_digest != NULL); + if (cms->digests[cms->selected_digest].pe_digest == NULL) { + cms->log(cms, LOG_ERR, "pe digest has not been allocated"); + return -1; + } + + if (content_is_empty(cms->digests[cms->selected_digest].pe_digest->data, + cms->digests[cms->selected_digest].pe_digest->len)) { + cms->log(cms, LOG_ERR, "pe binary has not been digested"); + return -1; + } SECItem sd_der; memset(&sd_der, '\0', sizeof(sd_der)); diff --git a/src/content_info.c b/src/content_info.c index 044e85e..7246d20 100644 --- a/src/content_info.c +++ b/src/content_info.c @@ -197,6 +197,11 @@ generate_spc_digest_info(cms_context *cms, SECItem *dip) int i = cms->selected_digest; memcpy(&di.digest, cms->digests[i].pe_digest, sizeof (di.digest)); + if (content_is_empty(di.digest.data, di.digest.len)) { + cms->log(cms, LOG_ERR, "got empty digest"); + return -1; + } + if (SEC_ASN1EncodeItem(cms->arena, dip, &di, DigestInfoTemplate) == NULL) { cms->log(cms, LOG_ERR, "could not encode DigestInfo: %s", @@ -327,6 +332,12 @@ generate_cinfo_digest(cms_context *cms, SpcContentInfo *cip) &cms->ci_digest->len, digest_get_digest_size(cms)) != SECSuccess) goto err; + + if (content_is_empty(cms->ci_digest->data, cms->ci_digest->len)) { + cms->log(cms, LOG_ERR, "generated empty digest"); + goto err; + } + if (cms->ci_digest->len > digest_get_digest_size(cms)) goto err; diff --git a/src/signer_info.c b/src/signer_info.c index 7a73c26..932b896 100644 --- a/src/signer_info.c +++ b/src/signer_info.c @@ -207,6 +207,11 @@ sign_blob(cms_context *cms, SECItem *sigitem, SECItem *sign_content) if (!sign_content) return -1; + if (content_is_empty(sign_content->data, sign_content->len)) { + cms->log(cms, LOG_ERR, "not signing empty digest"); + return -1; + } + SECOidData *oid = SECOID_FindOIDByTag(digest_get_signature_oid(cms)); if (!oid) goto err; diff --git a/src/util.h b/src/util.h index f495a0b..5e0ea34 100644 --- a/src/util.h +++ b/src/util.h @@ -110,6 +110,19 @@ free_poison(void *addrv, ssize_t len) addr[x] = poison_pills[x % 2]; } +static int +__attribute__ ((unused)) +content_is_empty(uint8_t *data, ssize_t len) +{ + if (len < 1) + return 1; + + for (int i = 0; i < len; i++) + if (data[i] != 0) + return 0; + return 1; +} + #if defined(DAEMON_H) static inline uint32_t __attribute__ ((unused)) -- 1.7.10.4 From 8311bc85a38fd4c9184ba79146662acceeb58909 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 17:47:49 -0400 Subject: [PATCH 19/30] Make sure we free the token/cert we get from the command line. This probably needs some further examination, but valgrind likes what's here currently. Signed-off-by: Peter Jones --- src/pesign.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/pesign.c b/src/pesign.c index 23aa992..acf0cee 100644 --- a/src/pesign.c +++ b/src/pesign.c @@ -435,6 +435,7 @@ main(int argc, char *argv[]) char *digest_name = "sha256"; char *tokenname = "NSS Certificate DB"; + char *origtoken = tokenname; char *certname = NULL; rc = pesign_context_new(&ctxp); @@ -573,6 +574,8 @@ main(int argc, char *argv[]) PORT_ErrorToString(PORT_GetError())); exit(1); } + if (tokenname != origtoken) + free(tokenname); ctxp->cms_ctx->certname = certname ? PORT_ArenaStrdup(ctxp->cms_ctx->arena, certname) : NULL; @@ -581,6 +584,8 @@ main(int argc, char *argv[]) PORT_ErrorToString(PORT_GetError())); exit(1); } + if (certname) + free(certname); int action = 0; if (daemon) -- 1.7.10.4 From eb9ae0e9e00e752f12f622a7b2b55f9e2abbff0c Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 17:48:44 -0400 Subject: [PATCH 20/30] [pesign] Only shut down nss in pesign.c if we're not the daemon. The daemon does its own init and shutdown. Signed-off-by: Peter Jones --- src/pesign.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/pesign.c b/src/pesign.c index acf0cee..581c327 100644 --- a/src/pesign.c +++ b/src/pesign.c @@ -804,6 +804,14 @@ main(int argc, char *argv[]) } pesign_context_free(ctxp); - NSS_Shutdown(); + if (!daemon) { + SECStatus status = NSS_Shutdown(); + if (status != SECSuccess) { + fprintf(stderr, "could not shut down NSS: %s", + PORT_ErrorToString(PORT_GetError())); + exit(1); + } + } + return (rc < 0); } -- 1.7.10.4 From 93cf7af5ac86adc9ba1345bbf55d6c2e70519627 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 17:49:17 -0400 Subject: [PATCH 21/30] Rework setup_digests() and teardown_digests() This fixes the problem I was seeing with empty content_info digests, and makes the code a /little/ bit cleaner in some ways. Signed-off-by: Peter Jones --- src/cms_common.c | 92 ++++++++++++++++++++++++++++++++---------------------- src/cms_common.h | 1 - src/daemon.c | 28 +---------------- src/pesign.c | 7 ----- 4 files changed, 55 insertions(+), 73 deletions(-) diff --git a/src/cms_common.c b/src/cms_common.c index 0c8ad8c..db134d7 100644 --- a/src/cms_common.c +++ b/src/cms_common.c @@ -96,43 +96,6 @@ digest_get_digest_size(cms_context *cms) return digest_params[i].size; } - -int -setup_digests(cms_context *cms) -{ - struct digest *digests = NULL; - - digests = calloc(n_digest_params, sizeof (*digests)); - if (!digests) { - cms->log(cms, LOG_ERR, "cannot allocate memory: %m"); - return -1; - } - - for (int i = 0; i < n_digest_params; i++) { - digests[i].pk11ctx = PK11_CreateDigestContext( - digest_params[i].digest_tag); - if (!digests[i].pk11ctx) { - cms->log(cms, LOG_ERR, "could not create digest " - "context: %s", - PORT_ErrorToString(PORT_GetError())); - goto err; - } - - PK11_DigestBegin(digests[i].pk11ctx); - } - - cms->digests = digests; - return 0; -err: - for (int i = 0; i < n_digest_params; i++) { - if (digests[i].pk11ctx) - PK11_DestroyContext(digests[i].pk11ctx, PR_TRUE); - } - - free(digests); - return -1; -} - void teardown_digests(cms_context *ctx) { @@ -733,6 +696,46 @@ check_pointer_and_size(Pe *pe, void *ptr, size_t size) return 1; } +int +generate_digest_begin(cms_context *cms) +{ + struct digest *digests = NULL; + + if (cms->digests) { + digests = cms->digests; + } else { + digests = calloc(n_digest_params, sizeof (*digests)); + if (!digests) { + cms->log(cms, LOG_ERR, "cannot allocate memory: %m"); + return -1; + } + } + + for (int i = 0; i < n_digest_params; i++) { + digests[i].pk11ctx = PK11_CreateDigestContext( + digest_params[i].digest_tag); + if (!digests[i].pk11ctx) { + cms->log(cms, LOG_ERR, "could not create digest " + "context: %s", + PORT_ErrorToString(PORT_GetError())); + goto err; + } + + PK11_DigestBegin(digests[i].pk11ctx); + } + + cms->digests = digests; + return 0; +err: + for (int i = 0; i < n_digest_params; i++) { + if (digests[i].pk11ctx) + PK11_DestroyContext(digests[i].pk11ctx, PR_TRUE); + } + + free(digests); + return -1; +} + void generate_digest_step(cms_context *cms, void *data, size_t len) { @@ -762,6 +765,12 @@ generate_digest_finish(cms_context *cms) PK11_DigestFinal(cms->digests[i].pk11ctx, digest->data, &digest->len, digest_params[i].size); + PK11_Finalize(cms->digests[i].pk11ctx); + PK11_DestroyContext(cms->digests[i].pk11ctx, PR_TRUE); + cms->digests[i].pk11ctx = NULL; + if (cms->digests[i].pe_digest) + free_poison(cms->digests[i].pe_digest->data, + cms->digests[i].pe_digest->len); cms->digests[i].pe_digest = digest; } @@ -791,7 +800,14 @@ generate_digest(cms_context *cms, Pe *pe) if (!pe) { cms->log(cms, LOG_ERR, "no output pe ready"); - exit(1); + return -1; + } + + rc = generate_digest_begin(cms); + if (rc < 0) { + cms->log(cms, LOG_ERR, "could not initialize digests: %s", + PORT_ErrorToString(PORT_GetError())); + return rc; } struct pe_hdr pehdr; diff --git a/src/cms_common.h b/src/cms_common.h index 830427e..5cbda62 100644 --- a/src/cms_common.h +++ b/src/cms_common.h @@ -86,7 +86,6 @@ extern int cms_context_alloc(cms_context **ctxp); extern int cms_context_init(cms_context *ctx); extern void cms_context_fini(cms_context *ctx); -extern int setup_digests(cms_context *cms); extern void teardown_digests(cms_context *ctx); extern int generate_octet_string(cms_context *ctx, SECItem *encoded, diff --git a/src/daemon.c b/src/daemon.c index 4259a3a..29ac1dd 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -142,15 +142,6 @@ handle_unlock_token(context *ctx, struct pollfd *pollfd, socklen_t size) return; } - rc = setup_digests(ctx->cms); - if (rc < 0) { - ctx->backup_cms->log(ctx->backup_cms, ctx->priority|LOG_NOTICE, - "Could not initialize digests: %s\n", - PORT_ErrorToString(PORT_GetError())); - send_response(ctx, ctx->backup_cms, pollfd, rc); - return; - } - steal_from_cms(ctx->backup_cms, ctx->cms); if (!buffer) { @@ -487,6 +478,7 @@ finish: close(outfd); send_response(ctx, ctx->cms, pollfd, rc); + teardown_digests(ctx->cms); } static void @@ -496,15 +488,6 @@ handle_sign_attached(context *ctx, struct pollfd *pollfd, socklen_t size) if (rc < 0) return; - rc = setup_digests(ctx->cms); - if (rc < 0) { - ctx->backup_cms->log(ctx->backup_cms, ctx->priority|LOG_NOTICE, - "Could not initialize digests: %s\n", - PORT_ErrorToString(PORT_GetError())); - send_response(ctx, ctx->backup_cms, pollfd, rc); - return; - } - steal_from_cms(ctx->backup_cms, ctx->cms); handle_signing(ctx, pollfd, size, 1); @@ -520,15 +503,6 @@ handle_sign_detached(context *ctx, struct pollfd *pollfd, socklen_t size) if (rc < 0) return; - rc = setup_digests(ctx->cms); - if (rc < 0) { - ctx->backup_cms->log(ctx->backup_cms, ctx->priority|LOG_NOTICE, - "Could not initialize digests: %s\n", - PORT_ErrorToString(PORT_GetError())); - send_response(ctx, ctx->backup_cms, pollfd, rc); - return; - } - steal_from_cms(ctx->backup_cms, ctx->cms); handle_signing(ctx, pollfd, size, 0); diff --git a/src/pesign.c b/src/pesign.c index 581c327..eed9264 100644 --- a/src/pesign.c +++ b/src/pesign.c @@ -548,13 +548,6 @@ main(int argc, char *argv[]) fprintf(stderr, "Could not register OIDs\n"); exit(1); } - - rc = setup_digests(ctxp->cms_ctx); - if (rc < 0) { - fprintf(stderr, "Could not initialize digests: %s\n", - PORT_ErrorToString(PORT_GetError())); - exit(1); - } } rc = set_digest_parameters(ctxp->cms_ctx, digest_name); -- 1.7.10.4 From 97bd823fd0f557c2cbabe917e4358a57c3e7af1d Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 17 Oct 2012 19:59:49 -0400 Subject: [PATCH 22/30] Fix errors found by coverity. Signed-off-by: Peter Jones --- src/actions.c | 4 ++-- src/cms_common.c | 17 ++++++++++------- src/daemon.c | 16 +++++++++++++++- src/password.c | 1 + src/pesign_context.c | 4 +++- src/wincert.c | 2 +- 6 files changed, 32 insertions(+), 12 deletions(-) diff --git a/src/actions.c b/src/actions.c index 76a311c..9cf4f45 100644 --- a/src/actions.c +++ b/src/actions.c @@ -70,7 +70,7 @@ insert_signature(cms_context *cms, int signum) if (signum != cms->num_signatures) { memmove(cms->signatures[signum+1], cms->signatures[signum], - sizeof(SECItem *) * (cms->num_signatures - signum)); + sizeof(SECItem) * (cms->num_signatures - signum)); } cms->signatures[signum] = sig; cms->num_signatures++; @@ -430,7 +430,7 @@ remove_signature(pesign_context *p_ctx) if (p_ctx->signum != ctx->num_signatures - 1) memmove(ctx->signatures[p_ctx->signum], ctx->signatures[p_ctx->signum+1], - sizeof(SECItem *) * + sizeof(SECItem) * (ctx->num_signatures - p_ctx->signum)); ctx->num_signatures--; diff --git a/src/cms_common.c b/src/cms_common.c index db134d7..e3c647d 100644 --- a/src/cms_common.c +++ b/src/cms_common.c @@ -598,16 +598,19 @@ generate_spc_string(cms_context *cms, SECItem *ssp, char *str, int len) memset(&ss, '\0', sizeof (ss)); SECITEM_AllocItem(cms->arena, &ss.unicode, len); - if (!ss.unicode.data && len != 0) { - cms->log(cms, LOG_ERR, "could not allocate memory: %s", - PORT_ErrorToString(PORT_GetError())); - return -1; + if (len != 0) { + if (!ss.unicode.data) { + cms->log(cms, LOG_ERR, "could not allocate memory: %s", + PORT_ErrorToString(PORT_GetError())); + return -1; + } + + memcpy(ss.unicode.data, str, len); } - - memcpy(ss.unicode.data, str, len); ss.unicode.type = siBMPString; - if (SEC_ASN1EncodeItem(cms->arena, ssp, &ss, SpcStringTemplate) == NULL) { + if (SEC_ASN1EncodeItem(cms->arena, ssp, &ss, SpcStringTemplate) + == NULL) { cms->log(cms, LOG_ERR, "could not encode SpcString: %s", PORT_ErrorToString(PORT_GetError())); return -1; diff --git a/src/daemon.c b/src/daemon.c index 29ac1dd..6d92136 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -134,7 +134,6 @@ handle_unlock_token(context *ctx, struct pollfd *pollfd, socklen_t size) struct msghdr msg; struct iovec iov; ssize_t n; - char *buffer = malloc(size); int rc = cms_context_alloc(&ctx->cms); if (rc < 0) { @@ -144,6 +143,7 @@ handle_unlock_token(context *ctx, struct pollfd *pollfd, socklen_t size) steal_from_cms(ctx->backup_cms, ctx->cms); + char *buffer = malloc(size); if (!buffer) { oom: ctx->cms->log(ctx->cms, ctx->priority|LOG_ERR, @@ -790,6 +790,7 @@ check_socket(context *ctx) rc = connect(sd, (struct sockaddr *)&addr_un, len); if (rc < 0) { + close(sd); unlink(SOCKPATH); return; } @@ -798,6 +799,7 @@ check_socket(context *ctx) socklen_t size = sizeof(remote); rc = getpeername(sd, &remote, &size); if (rc < 0) { + close(sd); return; } else { fprintf(stderr, "pesignd: already running"); @@ -911,6 +913,12 @@ daemonize(cms_context *cms_ctx, int do_fork) if (do_fork) { int fd = open("/dev/zero", O_RDONLY); + if (fd < 0) { + ctx.backup_cms->log(ctx.backup_cms, + ctx.priority|LOG_ERR, + "could not open /dev/zero: %m"); + exit(1); + } close(STDIN_FILENO); rc = dup2(fd, STDIN_FILENO); if (rc < 0) { @@ -922,6 +930,12 @@ daemonize(cms_context *cms_ctx, int do_fork) close(fd); fd = open("/dev/null", O_WRONLY); + if (fd < 0) { + ctx.backup_cms->log(ctx.backup_cms, + ctx.priority|LOG_ERR, + "could not open /dev/null: %m"); + exit(1); + } close(STDOUT_FILENO); rc = dup2(fd, STDOUT_FILENO); if (rc < 0) { diff --git a/src/password.c b/src/password.c index 5ee15f8..100c584 100644 --- a/src/password.c +++ b/src/password.c @@ -114,6 +114,7 @@ SECU_GetPasswordString(void *arg, char *prompt) output = fopen(consoleName, "w"); if (output == NULL) { + fclose(input); fprintf(stderr, "Error opening output terminal for write\n"); return NULL; } diff --git a/src/pesign_context.c b/src/pesign_context.c index cbd929f..033e8de 100644 --- a/src/pesign_context.c +++ b/src/pesign_context.c @@ -38,8 +38,10 @@ pesign_context_new(pesign_context **ctx) return -1; rc = pesign_context_init(context); - if (rc < 0) + if (rc < 0) { + free(context); return rc; + } context->flags |= PESIGN_C_ALLOCATED; *ctx = context; diff --git a/src/wincert.c b/src/wincert.c index b487dc5..4b5ba45 100644 --- a/src/wincert.c +++ b/src/wincert.c @@ -257,7 +257,7 @@ parse_signatures(cms_context *cms, Pe *pe) if (rc <= 0) break; - signatures[i] = calloc(1, sizeof (SECItem *)); + signatures[i] = calloc(1, sizeof (SECItem)); if (!signatures[i]) goto err; -- 1.7.10.4 From f4b3b20cc5a8697743f0ed5c24bf04c72e02ba11 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 18 Oct 2012 09:12:25 -0400 Subject: [PATCH 23/30] Don't keep the DEPS list twice. Signed-off-by: Peter Jones --- src/Makefile | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Makefile b/src/Makefile index cd06158..cb74c12 100644 --- a/src/Makefile +++ b/src/Makefile @@ -39,14 +39,20 @@ client_OBJECTS = $(foreach source,$(client_SOURCES),$(patsubst %.c,%,$(source)). client_DEPS = $(foreach source,$(client_SOURCES),.$(patsubst %.c,%,$(source)).P) client : $(client_OBJECTS) $(STATIC_LIBS) -deps : $(generic_DEPS)$(authvar_DEPS) $(pesign_DEPS) $(client_DEPS) \ +fuzzsocket_SOURCES = fuzzsocket.c +fuzzsocket_OBJECTS = $(foreach source,$(fuzzsocket_SOURCES),$(patsubst %.c,%,$(source)).o) +fuzzsocket_DEPS = $(foreach source,$(fuzzsocket_SOURCES),.$(patsubst %.c,%,$(source)).P) +fuzzsocket : $(fuzzsocket_OBJECTS) + +DEPS = $(generic_DEPS)$(authvar_DEPS) $(pesign_DEPS) $(client_DEPS) \ $(peverify_DEPS) +deps : $(DEPS) + depclean : @rm -fv .*.P --include $(generic_DEPS) $(authvar_DEPS) $(pesign_DEPS) $(client_DEPS) \ - $(peverify_DEPS) +-include $(DEPS) clean : depclean @rm -rfv *.o *.a *.so $(TARGETS) -- 1.7.10.4 From 8308daea94818c7603149a7aefd3df5f0aebbcc9 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 18 Oct 2012 13:09:58 -0400 Subject: [PATCH 24/30] Make "install_systemd" and "install_sysvinit" separate targets Signed-off-by: Peter Jones --- Makefile | 6 ++++ src/Makefile | 16 ++++++---- src/pesign.sysvinit | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 src/pesign.sysvinit diff --git a/Makefile b/Makefile index 531c865..ddaf4c5 100644 --- a/Makefile +++ b/Makefile @@ -19,6 +19,12 @@ install : $(INSTALL) -d -m 755 $(INSTALLROOT)$(PREFIX)$(DOCDIR)/pesign-$(VERSION)/ $(INSTALL) -m 644 COPYING $(INSTALLROOT)$(PREFIX)$(DOCDIR)/pesign-$(VERSION)/ +install_systemd: + @for x in $(SUBDIRS) ; do $(MAKE) -C $${x} TOPDIR=$(TOPDIR) SRCDIR=$(TOPDIR)/$@/ ARCH=$(ARCH) $@ ; done + +install_sysvinit: + @for x in $(SUBDIRS) ; do $(MAKE) -C $${x} TOPDIR=$(TOPDIR) SRCDIR=$(TOPDIR)/$@/ ARCH=$(ARCH) $@ ; done + .PHONY: $(SUBDIRS) clean install include $(TOPDIR)/Make.rules diff --git a/src/Makefile b/src/Makefile index cb74c12..7e611c8 100644 --- a/src/Makefile +++ b/src/Makefile @@ -42,7 +42,7 @@ client : $(client_OBJECTS) $(STATIC_LIBS) fuzzsocket_SOURCES = fuzzsocket.c fuzzsocket_OBJECTS = $(foreach source,$(fuzzsocket_SOURCES),$(patsubst %.c,%,$(source)).o) fuzzsocket_DEPS = $(foreach source,$(fuzzsocket_SOURCES),.$(patsubst %.c,%,$(source)).P) -fuzzsocket : $(fuzzsocket_OBJECTS) +fuzzsocket : $(fuzzsocket_OBJECTS) -lrt DEPS = $(generic_DEPS)$(authvar_DEPS) $(pesign_DEPS) $(client_DEPS) \ $(peverify_DEPS) @@ -57,6 +57,16 @@ depclean : clean : depclean @rm -rfv *.o *.a *.so $(TARGETS) +install_systemd: + $(INSTALL) -d -m 755 $(INSTALLROOT)/usr/lib/tmpfiles.d/ + $(INSTALL) -m 644 tmpfiles.conf $(INSTALLROOT)/usr/lib/tmpfiles.d/pesign.conf + $(INSTALL) -d -m 755 $(INSTALLROOT)/usr/lib/systemd/system/ + $(INSTALL) -m 644 pesign.service $(INSTALLROOT)/usr/lib/systemd/system/ + +install_sysvinit: + $(INSTALL) -d -m 755 $(INSTALLROOT)/etc/rc.d/init.d/ + $(INSTALL) -m 755 pesign.sysvinit $(INSTALLROOT)/etc/rc.d/init.d/pesign + install : $(INSTALL) -d -m 700 $(INSTALLROOT)/etc/pki/pesign/ $(INSTALL) -d -m 770 $(INSTALLROOT)/var/run/pesign/ @@ -72,10 +82,6 @@ install : #$(INSTALL) -m 644 peverify.1 $(INSTALLROOT)/usr/share/man/man1/ $(INSTALL) -d -m 755 $(INSTALLROOT)/etc/rpm/ $(INSTALL) -m 644 macros.pesign $(INSTALLROOT)/etc/rpm/ - $(INSTALL) -d -m 755 $(INSTALLROOT)/usr/lib/tmpfiles.d/ - $(INSTALL) -m 644 tmpfiles.conf $(INSTALLROOT)/usr/lib/tmpfiles.d/pesign.conf - $(INSTALL) -d -m 755 $(INSTALLROOT)/usr/lib/systemd/system/ - $(INSTALL) -m 644 pesign.service $(INSTALLROOT)/usr/lib/systemd/system/ .PHONY: all deps clean install diff --git a/src/pesign.sysvinit b/src/pesign.sysvinit new file mode 100644 index 0000000..f955e01 --- /dev/null +++ b/src/pesign.sysvinit @@ -0,0 +1,82 @@ +#! /bin/sh +# +# pesign This starts the pesign PE signing daemon +# +# chkconfig: - 50 50 +# processname: /usr/bin/pesign +# pidfile: /var/run/pesign.pid +### BEGIN INIT INFO +# Provides: pesign +# Default-Start: +# Default-Stop: +# Short-Description: The pesign PE signing daemon +# Description: The pesign PE signing daemon +### END INIT INFO + +. /etc/init.d/functions +[ -f /usr/bin/pesign ] || exit 1 + +RETVAL=0 + +start(){ + echo -n "Starting pesign: " + daemon /usr/bin/pesign --daemonize + RETVAL=$? + echo + touch /var/lock/subsys/pesign +} + +stop(){ + echo -n "Stopping pesign: " + killproc -p /var/run/pesign.pid pesignd + RETVAL=$? + echo + rm -f /var/lock/subsys/pesign +} + +restart(){ + stop + start +} + +reload(){ + stop + start +} + +condrestart(){ + [ -e /var/lock/subsys/pesign ] && restart +} + +# See how we were called. +case "$1" in + start) + start + ;; + stop) + stop + ;; + status) + status /usr/bin/pesign + ;; + restart) + restart + ;; + reload) + reload + ;; + force-reload) + reload + ;; + condrestart) + condrestart + ;; + try-restart) + condrestart + ;; + *) + echo "Usage: pesign {start|stop|status|restart|condrestart|reload}" + RETVAL=1 +esac + +exit $RETVAL -- 1.7.10.4 From e23f28bc9e114ab99414023dbe45db4ea057b3a1 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 18 Oct 2012 13:10:28 -0400 Subject: [PATCH 25/30] Get rid of an unnecessary allocation. Signed-off-by: Peter Jones --- src/client.c | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/client.c b/src/client.c index 8336749..df1c8f2 100644 --- a/src/client.c +++ b/src/client.c @@ -223,25 +223,17 @@ unlock_token(int sd, char *tokenname, char *pin) { struct msghdr msg; struct iovec iov[2]; - pesignd_msghdr *pm; + pesignd_msghdr pm; uint32_t size0 = pesignd_string_size(tokenname); uint32_t size1 = pesignd_string_size(pin); - pm = calloc(1, sizeof(*pm)); - if (!pm) { -oom: - fprintf(stderr, "pesign-client: could not allocate memory: " - "%m\n"); - exit(1); - } - - pm->version = PESIGND_VERSION; - pm->command = CMD_UNLOCK_TOKEN; - pm->size = size0 + size1; - iov[0].iov_base = pm; - iov[0].iov_len = sizeof (*pm); + pm.version = PESIGND_VERSION; + pm.command = CMD_UNLOCK_TOKEN; + pm.size = size0 + size1; + iov[0].iov_base = ± + iov[0].iov_len = sizeof (pm); memset(&msg, '\0', sizeof(msg)); msg.msg_iov = iov; @@ -257,8 +249,11 @@ oom: uint8_t *buffer = NULL; buffer = calloc(1, size0 + size1); - if (!buffer) - goto oom; + if (!buffer) { + fprintf(stderr, "pesign-client: could not allocate memory: " + "%m\n"); + exit(1); + } pesignd_string *tn = (pesignd_string *)buffer; pesignd_string_set(tn, tokenname); @@ -478,8 +473,9 @@ main(int argc, char *argv[]) rc = poptReadDefaultConfig(optCon, 0); if (rc < 0) { - fprintf(stderr, "pesign: poprReadDefaultConfig failed: %s\n", - poptStrerror(rc)); + fprintf(stderr, + "pesign-client: poptReadDefaultConfig failed: %s\n", + poptStrerror(rc)); exit(1); } -- 1.7.10.4 From 5c6bd285201bb8f574c96d563ddf6e9478c041c6 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 18 Oct 2012 14:28:36 -0400 Subject: [PATCH 26/30] Allow use of -e from rpm macro. Signed-off-by: Peter Jones --- src/macros.pesign | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/macros.pesign b/src/macros.pesign index 703edbb..7706050 100644 --- a/src/macros.pesign +++ b/src/macros.pesign @@ -11,9 +11,9 @@ %_pesign /usr/bin/pesign -%pesign(i:o:C:s) \ +%pesign(i:o:C:e:s) \ if [ -x %{_pesign} -a "%{_target_cpu}" == "x86_64" ]; then \ - %{_pesign} %{__pesign_token} %{__pesign_cert} %{-i} %{-o} %{-s} \ + %{_pesign} %{__pesign_token} %{__pesign_cert} %{-i} %{-o} %{-e} %{-s} \ else \ if [ -n "%{-i*}" -a -n "%{-o*}" ]; then \ mv %{-i*} %{-o*} \ -- 1.7.10.4 From 606eeb10b6a5ffb3dd36d362e96a92c8f9fe595f Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 18 Oct 2012 14:55:07 -0400 Subject: [PATCH 27/30] Make client use -e like pesign does, rather than --detached. This way we can use the same macros for them. Signed-off-by: Peter Jones --- src/client.c | 22 ++++++++++++++++++++-- src/pesign-client.1 | 3 ++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/client.c b/src/client.c index df1c8f2..5e5399d 100644 --- a/src/client.c +++ b/src/client.c @@ -434,6 +434,7 @@ main(int argc, char *argv[]) int action; char *infile = NULL; char *outfile = NULL; + char *exportfile = NULL; int attached = 1; int pinfd = -1; char *pinfile = NULL; @@ -456,8 +457,9 @@ main(int argc, char *argv[]) &infile, 0, "input filename", "" }, {"outfile", 'o', POPT_ARG_STRING, &outfile, 0, "output filename", "" }, - {"detached", 'd', POPT_ARG_VAL, &attached, 0, - "create detached signature", NULL }, + {"export", 'e', POPT_ARG_STRING, + &exportfile, 0, "create detached signature", + "" }, {"pinfd", 'f', POPT_ARG_INT, &pinfd, -1, "read file descriptor for pin information", "" }, @@ -494,6 +496,22 @@ main(int argc, char *argv[]) exit(1); } + if (!outfile && !exportfile) { + fprintf(stderr, "pesign-client: neither --outfile nor --export " + "specified\n"); + exit(1); + } + + if (outfile && exportfile) { + fprintf(stderr, "pesign-client: both --outfile and --export " + "specified\n"); + exit(1); + } + if (exportfile) { + outfile = exportfile; + attached = 0; + } + poptFreeContext(optCon); int sd = connect_to_server(); diff --git a/src/pesign-client.1 b/src/pesign-client.1 index 686383e..1ccfbb3 100644 --- a/src/pesign-client.1 +++ b/src/pesign-client.1 @@ -5,10 +5,11 @@ pesign-client \- command line tool for signing UEFI applications .SH SYNOPSIS \fBpesign\fR [--in=\fIinfile\fR | -i \fIinfile\fR] [--out=\fIoutfile\fR | -o \fIoutfile\fR] + [--export=\fIexportfile\fR | -e \fIexportfile\fR] [--token=\fItoken\fR | -t \fItoken\fR] [--certificate=\fInickname\fR | -c \fInickname\fR] [--unlock | -u] [--kill | -k] [--sign | -s] - [--detached | -d] [--pinfd=\fIpinfd\fR | -f \fIpinfd\fR] + [--pinfd=\fIpinfd\fR | -f \fIpinfd\fR] [--pinfile=\fIpinfile\fR | -F \fIpinfile\fR] .SH DESCRIPTION -- 1.7.10.4 From 2d1816cf43fa04e4454a65bfd0121c037368ead0 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Fri, 19 Oct 2012 10:08:26 -0400 Subject: [PATCH 28/30] Add support to read the pin from stdin in client. Signed-off-by: Peter Jones --- src/client.c | 10 +++++++--- src/password.c | 41 +++++++++++++++++++++++++++++++++++++++++ src/password.h | 1 + src/signer_info.c | 45 +-------------------------------------------- 4 files changed, 50 insertions(+), 47 deletions(-) diff --git a/src/client.c b/src/client.c index 5e5399d..561db19 100644 --- a/src/client.c +++ b/src/client.c @@ -212,10 +212,14 @@ get_token_pin(int pinfd, char *pinfile, char *envname) fclose(pinf); return pin; - } else - return strdup(getenv(envname)); + } else { + pin = getenv(envname); + if (pin) + return strdup(pin); + } - return NULL; + pin = readpw(NULL, PR_FALSE, NULL); + return pin; } static void diff --git a/src/password.c b/src/password.c index 100c584..c663955 100644 --- a/src/password.c +++ b/src/password.c @@ -17,6 +17,7 @@ * Author(s): Peter Jones */ +#include #include #include #include @@ -289,4 +290,44 @@ SECU_GetModulePassword(PK11SlotInfo *slot, PRBool retry, void *arg) return NULL; } +#if 0 +#warning investigate killing readpw +#endif +char * +readpw(PK11SlotInfo *slot, PRBool retry, void *arg) +{ + struct termios sio, tio; + char line[LINE_MAX], *p; + if (tcgetattr(fileno(stdin), &sio) < 0) { + fprintf(stderr, "Could not read password from standard input.\n"); + return NULL; + } + tio = sio; + tio.c_lflag &= ~ECHO; + if (tcsetattr(fileno(stdin), 0, &tio) < 0) { + fprintf(stderr, "Could not read password from standard input.\n"); + return NULL; + } + + fprintf(stdout, "Enter passphrase for private key: "); + if (fgets(line, sizeof(line), stdin) == NULL) { + fprintf(stdout, "\n"); + tcsetattr(fileno(stdin), 0, &sio); + return NULL; + } + fprintf(stdout, "\n"); + tcsetattr(fileno(stdin), 0, &sio); + + p = line + strcspn(line, "\r\n"); + if (p != NULL) + *p = '\0'; + + char *ret = strdup(line); + memset(line, '\0', sizeof (line)); + if (!ret) { + fprintf(stderr, "Could not read passphrase.\n"); + return NULL; + } + return ret; +} diff --git a/src/password.h b/src/password.h index 853bd5a..bcbac44 100644 --- a/src/password.h +++ b/src/password.h @@ -22,5 +22,6 @@ extern char *SECU_GetModulePassword(PK11SlotInfo *slot, PRBool retry, void *arg); extern char *get_password_passthrough(PK11SlotInfo *slot, PRBool retry, void *arg); extern char *get_password_fail(PK11SlotInfo *slot, PRBool retry, void *arg); +extern char *readpw(PK11SlotInfo *slot, PRBool retry, void *arg); #endif /* PASSWORD_H */ diff --git a/src/signer_info.c b/src/signer_info.c index 932b896..f755bf6 100644 --- a/src/signer_info.c +++ b/src/signer_info.c @@ -19,10 +19,8 @@ #include "pesign.h" -#include #include #include -#include #include #include @@ -159,47 +157,6 @@ err: return -1; } -#if 0 -#warning investigate killing getpw -#endif -static char *getpw(PK11SlotInfo *slot, PRBool retry, void *arg) -{ - struct termios sio, tio; - char line[LINE_MAX], *p; - - if (tcgetattr(fileno(stdin), &sio) < 0) { - fprintf(stderr, "Could not read password from standard input.\n"); - return NULL; - } - tio = sio; - tio.c_lflag &= ~ECHO; - if (tcsetattr(fileno(stdin), 0, &tio) < 0) { - fprintf(stderr, "Could not read password from standard input.\n"); - return NULL; - } - - fprintf(stdout, "Enter passphrase for private key: "); - if (fgets(line, sizeof(line), stdin) == NULL) { - fprintf(stdout, "\n"); - tcsetattr(fileno(stdin), 0, &sio); - return NULL; - } - fprintf(stdout, "\n"); - tcsetattr(fileno(stdin), 0, &sio); - - p = line + strcspn(line, "\r\n"); - if (p != NULL) - *p = '\0'; - - char *ret = strdup(line); - memset(line, '\0', sizeof (line)); - if (!ret) { - fprintf(stderr, "Could not read passphrase.\n"); - return NULL; - } - return ret; -} - static int sign_blob(cms_context *cms, SECItem *sigitem, SECItem *sign_content) { @@ -216,7 +173,7 @@ sign_blob(cms_context *cms, SECItem *sigitem, SECItem *sign_content) if (!oid) goto err; - PK11_SetPasswordFunc(cms->func ? cms->func : getpw); + PK11_SetPasswordFunc(cms->func ? cms->func : readpw); SECKEYPrivateKey *privkey = PK11_FindKeyByAnyCert(cms->cert, cms->pwdata ? cms->pwdata : NULL); if (!privkey) { -- 1.7.10.4 From 98ada084e649204ad49bfe1fcccd50eae593196a Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Mon, 22 Oct 2012 10:44:30 -0400 Subject: [PATCH 29/30] Complain if no certname is provided. Signed-off-by: Peter Jones --- src/pesign.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/pesign.c b/src/pesign.c index eed9264..a7befb0 100644 --- a/src/pesign.c +++ b/src/pesign.c @@ -615,6 +615,12 @@ main(int argc, char *argv[]) action |= GENERATE_SIGNATURE; if (!(action & EXPORT_SIGNATURE)) action |= IMPORT_SIGNATURE; + + if (!ctxp->cms_ctx->certname) { + fprintf(stderr, "pesign: signing requested but no " + "certificate nickname provided\n"); + exit(1); + } } if (ctxp->hash) -- 1.7.10.4 From c790c5c2b121506e6ac1ba7f4906b8f0eb74051b Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Fri, 19 Oct 2012 10:07:40 -0400 Subject: [PATCH 30/30] Fix command line checking for -s. Accidentally applied when not using -s. Woops. Signed-off-by: Peter Jones --- src/client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client.c b/src/client.c index 561db19..1ec582b 100644 --- a/src/client.c +++ b/src/client.c @@ -500,7 +500,7 @@ main(int argc, char *argv[]) exit(1); } - if (!outfile && !exportfile) { + if (action & SIGN_BINARY && (!outfile && !exportfile)) { fprintf(stderr, "pesign-client: neither --outfile nor --export " "specified\n"); exit(1); -- 1.7.10.4