On Tue, Jan 07, 2014 at 16:54, Christian Weisgerber wrote:
> Ted Unangst <[email protected]> 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 -0000 1.12
+++ signify.c 30 Dec 2013 18:11:45 -0000
@@ -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 + 1];
+ char fp2[(FPLEN + 2) / 3 * 4 + 1];
+ b64_ntop(pubkey.fingerprint, FPLEN, fp1, sizeof(fp1));
+ b64_ntop(sig.fingerprint, FPLEN, fp2, sizeof(fp2));
+ errx(1, "fingerprint mismatch (%s vs %s)", fp1, fp2);
+ }
msg = readmsg(inputfile, &msglen);
