Skip to content

Commit

Permalink
security: don't generate zero keys when CURVE support is missing
Browse files Browse the repository at this point in the history
If CURVE support is missing at runtime, zcert_new() will unfortunately
generate zeroed keys instead of failing. These zeroed keys will later
cause assertions in the zeromq code.

Instead replace zcert_new() with zmq_curve_keypair()/zcert_new_from().
zmq_curve_keypair() should fail with errno == ENOTSUP if CURVE
support is missing from libzmq, so we can generate a reasonable
error instead of silently failing.

Fixes #7
  • Loading branch information
grondo committed Sep 29, 2014
1 parent 298e81d commit 8970b23
Showing 1 changed file with 26 additions and 2 deletions.
28 changes: 26 additions & 2 deletions src/common/libflux/security.c
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,28 @@ static char * ctime_iso8601_now (char *buf, size_t sz)
return (buf);
}

static zcert_t *zcert_curve_new (flux_sec_t c)
{
zcert_t *new;
char sec[41];
char pub[41];

if (zmq_curve_keypair (pub, sec) < 0) {
if (errno == ENOTSUP)
seterrstr (c,
"No CURVE support in libzmq (not compiled with libsodium?)");
else
seterrstr (c,
"Unknown error generating CURVE keypair");
return NULL;
}

if (!(new = zcert_new_from ((byte *)pub, (byte *)sec)))
oom ();

return new;
}

static int gencurve (flux_sec_t c, const char *role, bool force, bool verbose)
{
char *path = NULL, *priv = NULL;;
Expand Down Expand Up @@ -551,8 +573,10 @@ static int gencurve (flux_sec_t c, const char *role, bool force, bool verbose)
errno = EEXIST;
goto done;
}
if (!(cert = zcert_new ()))
oom ();

if (!(cert = zcert_curve_new (c)))
goto done; /* error message set in zcert_curve_new() */

zcert_set_meta (cert, "time", "%s", ctime_iso8601_now (buf, sizeof (buf)));
zcert_set_meta (cert, "role", (char *)role);
if (verbose) {
Expand Down

1 comment on commit 8970b23

@garlick
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks entirely reasonable. Thanks for running this down Mark.

Please sign in to comment.