signify untrusted comments
A little background: Among the many ways a signature can fail to
verify is the user trying to verify with the wrong public key. In a
setup like signify where users may be expected (required) at times to
specify the public key, I expect this to be one of the leading failure
modes. (Somewhere on this list is also using the wrong private key to
create the signature.)
This can be a frustrating error because the only message you get back
is signature failed. The obvious solution is to encode some
identifier information into the key and signature files. That provides
a quick check where we can easily say this public key belongs to X,
but the signature was created by Y.
Why doesn't signify do this? Because that identity isn't trusted. Just
because a public key says it came from root@openbsd doesn't mean it
did. I didn't want to open up a social engineering avenue whereby
somebody provides a user with a backdoor pkg and matching public key,
allegedly from root@openbsd. Hey, openbsd is using a new key now, but
as you can see, it comes from them. Nothing in a signify key or
signature should be mistaken as a voucher for its own integrity.
As a concession to usability, I did include a comment line in the
files. If you accidentally mix up some key files, for whatever reason,
maybe that can help set you straight. The signify tool itself ignores
this line, as documented. Now we're back to asking, can this be used
to trick the user? I was hoping that users would know the comment
isn't to be trusted, but that may be asking too much.
To that end, I think the comment should be marked as untrusted, and
signify should even check that it says untrusted. Hopefully this makes
it a little harder to con somebody into believing the comment actually
should be trusted. Is this really necessary? Honestly, I think so. The
weakest link here is an adversary's ability to trick the user into using
the wrong key; I don't want to assist them in any way.
(I'm also open to reconsidering whether keys should include
identifiers. Perhaps a random id created during key generation? Just
enough to say you're using the wrong key.)
Index: signify.c
===
RCS file: /cvs/src/usr.bin/signify/signify.c,v
retrieving revision 1.12
diff -u -p -r1.12 signify.c
--- signify.c 6 Jan 2014 01:50:54 - 1.12
+++ signify.c 30 Dec 2013 16:14:25 -
@@ -38,6 +38,9 @@
#define PKALG Ed
#define KDFALG BK
+#define COMMENTHDR untrusted comment:
+#define COMMENTHDRLEN 18
+
struct enckey {
uint8_t pkalg[2];
uint8_t kdfalg[2];
@@ -118,8 +121,9 @@ readb64file(const char *filename, void *
if (rv == -1)
err(1, read from %s, filename);
commentend = strchr(b64, '\n');
- if (!commentend)
- errx(1, no newline in %s, filename);
+ if (!commentend || commentend - b64 = COMMENTHDRLEN ||
+ memcmp(b64, COMMENTHDR, COMMENTHDRLEN))
+ errx(1, invalid comment in %s, filename);
rv = b64_pton(commentend + 1, buf, len);
if (rv != len)
errx(1, invalid b64 encoding in %s, filename);
@@ -172,7 +176,8 @@ writeb64file(const char *filename, const
int fd, rv;
fd = xopen(filename, O_CREAT|O_EXCL|O_NOFOLLOW|O_RDWR, mode);
- snprintf(header, sizeof(header), signify -- %s\n, comment);
+ snprintf(header, sizeof(header), %s signify %s\n, COMMENTHDR,
+ comment);
writeall(fd, header, strlen(header), filename);
if ((rv = b64_ntop(buf, len, b64, sizeof(b64)-1)) == -1)
errx(1, b64 encode failed);
Re: signify untrusted comments
Ted Unangst t...@tedunangst.com wrote: To that end, I think the comment should be marked as untrusted, and signify should even check that it says untrusted. Hopefully this makes it a little harder to con somebody into believing the comment actually should be trusted. I think somebody who can be conned into accepting a key will not understand or think about untrusted comment. (I'm also open to reconsidering whether keys should include identifiers. Perhaps a random id created during key generation? Just enough to say you're using the wrong key.) I'm in favor. -- Christian naddy Weisgerber na...@mips.inka.de
Re: signify untrusted comments
On Tue, Jan 07, 2014 at 04:54:59PM +, Christian Weisgerber wrote: (I'm also open to reconsidering whether keys should include identifiers. Perhaps a random id created during key generation? Just enough to say you're using the wrong key.) I'm in favor. Yeah, looks like a good idea. 64 bits random number ?
Re: signify untrusted comments
On Tue, Jan 07, 2014 at 16:54, Christian Weisgerber wrote:
Ted Unangst t...@tedunangst.com wrote:
To that end, I think the comment should be marked as untrusted, and
signify should even check that it says untrusted. Hopefully this makes
it a little harder to con somebody into believing the comment actually
should be trusted.
I think somebody who can be conned into accepting a key will not
understand or think about untrusted comment.
hmm.
1. people who don't trust strangers. we don't need to protect them.
2. people who do trust strangers. we can't protect them.
3. people who are initially suspicious, but have a mistaken belief
that they can spot mischief. maybe they will be fooled regardless, but
I don't want to provide anything that can be used as leverage against
them.
(I'm also open to reconsidering whether keys should include
identifiers. Perhaps a random id created during key generation? Just
enough to say you're using the wrong key.)
I'm in favor.
Here's a diff with that too. I print out the fingerprints after a
mismatch, but the fingerprints are in the opaque part of the file, so
I'm not sure how useful that is. At least it provides a hint to check
the command line arguments.
Index: signify.c
===
RCS file: /cvs/src/usr.bin/signify/signify.c,v
retrieving revision 1.12
diff -u -p -r1.12 signify.c
--- signify.c 6 Jan 2014 01:50:54 - 1.12
+++ signify.c 30 Dec 2013 18:11:45 -
@@ -37,6 +37,10 @@
#define PKALG Ed
#define KDFALG BK
+#define FPLEN 8
+
+#define COMMENTHDR untrusted comment:
+#define COMMENTHDRLEN 18
struct enckey {
uint8_t pkalg[2];
@@ -44,16 +48,19 @@ struct enckey {
uint32_t kdfrounds;
uint8_t salt[16];
uint8_t checksum[8];
+ uint8_t fingerprint[FPLEN];
uint8_t seckey[SECRETBYTES];
};
struct pubkey {
uint8_t pkalg[2];
+ uint8_t fingerprint[FPLEN];
uint8_t pubkey[PUBLICBYTES];
};
struct sig {
uint8_t pkalg[2];
+ uint8_t fingerprint[FPLEN];
uint8_t sig[SIGBYTES];
};
@@ -118,8 +125,10 @@ readb64file(const char *filename, void *
if (rv == -1)
err(1, read from %s, filename);
commentend = strchr(b64, '\n');
- if (!commentend)
- errx(1, no newline in %s, filename);
+ if (!commentend || commentend - b64 = COMMENTHDRLEN ||
+ memcmp(b64, COMMENTHDR, COMMENTHDRLEN))
+ errx(1, invalid comment in %s; must start with '%s',
+ filename, COMMENTHDR);
rv = b64_pton(commentend + 1, buf, len);
if (rv != len)
errx(1, invalid b64 encoding in %s, filename);
@@ -172,7 +181,8 @@ writeb64file(const char *filename, const
int fd, rv;
fd = xopen(filename, O_CREAT|O_EXCL|O_NOFOLLOW|O_RDWR, mode);
- snprintf(header, sizeof(header), signify -- %s\n, comment);
+ snprintf(header, sizeof(header), %s signify %s\n, COMMENTHDR,
+ comment);
writeall(fd, header, strlen(header), filename);
if ((rv = b64_ntop(buf, len, b64, sizeof(b64)-1)) == -1)
errx(1, b64 encode failed);
@@ -239,10 +249,12 @@ generate(const char *pubkeyfile, const c
struct pubkey pubkey;
struct enckey enckey;
uint8_t xorkey[sizeof(enckey.seckey)];
+ uint8_t fingerprint[FPLEN];
SHA2_CTX ctx;
int i;
crypto_sign_ed25519_keypair(pubkey.pubkey, enckey.seckey);
+ arc4random_buf(fingerprint, sizeof(fingerprint));
SHA512Init(ctx);
SHA512Update(ctx, enckey.seckey, sizeof(enckey.seckey));
@@ -251,6 +263,7 @@ generate(const char *pubkeyfile, const c
memcpy(enckey.pkalg, PKALG, 2);
memcpy(enckey.kdfalg, KDFALG, 2);
enckey.kdfrounds = htonl(rounds);
+ memcpy(enckey.fingerprint, fingerprint, FPLEN);
arc4random_buf(enckey.salt, sizeof(enckey.salt));
kdf(enckey.salt, sizeof(enckey.salt), rounds, xorkey, sizeof(xorkey));
memcpy(enckey.checksum, digest, sizeof(enckey.checksum));
@@ -264,6 +277,7 @@ generate(const char *pubkeyfile, const c
memset(enckey, 0, sizeof(enckey));
memcpy(pubkey.pkalg, PKALG, 2);
+ memcpy(pubkey.fingerprint, fingerprint, FPLEN);
writeb64file(pubkeyfile, public key, pubkey,
sizeof(pubkey), 0666);
}
@@ -299,6 +313,7 @@ sign(const char *seckeyfile, const char
msg = readmsg(inputfile, msglen);
signmsg(enckey.seckey, msg, msglen, sig.sig);
+ memcpy(sig.fingerprint, enckey.fingerprint, FPLEN);
memset(enckey, 0, sizeof(enckey));
memcpy(sig.pkalg, PKALG, 2);
@@ -317,6 +332,14 @@ verify(const char *pubkeyfile, const cha
readb64file(pubkeyfile, pubkey, sizeof(pubkey));
readb64file(sigfile, sig, sizeof(sig));
+
+ if (memcmp(pubkey.fingerprint, sig.fingerprint, FPLEN)) {
+ char fp1[(FPLEN + 2) / 3 * 4 +
Re: signify untrusted comments
On Tue, Jan 07, 2014 at 01:17:12PM -0500, Ted Unangst wrote: On Tue, Jan 07, 2014 at 16:54, Christian Weisgerber wrote: Ted Unangst t...@tedunangst.com wrote: To that end, I think the comment should be marked as untrusted, and signify should even check that it says untrusted. Hopefully this makes it a little harder to con somebody into believing the comment actually should be trusted. I think somebody who can be conned into accepting a key will not understand or think about untrusted comment. hmm. 1. people who don't trust strangers. we don't need to protect them. 2. people who do trust strangers. we can't protect them. 3. people who are initially suspicious, but have a mistaken belief that they can spot mischief. maybe they will be fooled regardless, but I don't want to provide anything that can be used as leverage against them. (I'm also open to reconsidering whether keys should include identifiers. Perhaps a random id created during key generation? Just enough to say you're using the wrong key.) I'm in favor. Here's a diff with that too. I print out the fingerprints after a mismatch, but the fingerprints are in the opaque part of the file, so I'm not sure how useful that is. At least it provides a hint to check the command line arguments. I think the fingerprint mismatch error message is too technical. errx(1, Signature failed: checked against the wrong key); looks like enough for me. Apart from that, I'm okay with that code. If you want to display fingerprints, well, you have to make it possible to display fingerprints for any key-like message. Not that it's much code, but is it really necessary ?
