Changeset View
Standalone View
lib/libve/openpgp/opgp_key.c
- This file was added.
Property | Old Value | New Value |
---|---|---|
svn:eol-style | null | native \ No newline at end of property |
svn:keywords | null | FreeBSD=%H \ No newline at end of property |
svn:mime-type | null | text/plain \ No newline at end of property |
/*- | |||||
* Copyright (c) 2018, Juniper Networks, Inc. | |||||
* | |||||
* Redistribution and use in source and binary forms, with or without | |||||
* modification, are permitted provided that the following conditions | |||||
* are met: | |||||
* 1. Redistributions of source code must retain the above copyright | |||||
* notice, this list of conditions and the following disclaimer. | |||||
* 2. Redistributions in binary form must reproduce the above copyright | |||||
* notice, this list of conditions and the following disclaimer in the | |||||
* documentation and/or other materials provided with the distribution. | |||||
* | |||||
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS | |||||
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT | |||||
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR | |||||
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT | |||||
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, | |||||
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT | |||||
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, | |||||
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY | |||||
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT | |||||
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE | |||||
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. | |||||
*/ | |||||
#include <sys/cdefs.h> | |||||
__FBSDID("$FreeBSD$"); | |||||
#include "../libve-priv.h" | |||||
#include "decode.h" | |||||
#include "packet.h" | |||||
/** | |||||
* @brief decode user-id packet | |||||
* | |||||
* This is trivial | |||||
* | |||||
* @sa rfc4880:5.11 | |||||
*/ | |||||
ssize_t | |||||
decode_user(int tag, unsigned char **pptr, size_t len, OpenPGP_user *user) | |||||
{ | |||||
char *cp; | |||||
if (tag == 13) { | |||||
user->id = malloc(len + 1); | |||||
strncpy(user->id, (char *)*pptr, len); | |||||
user->id[len] = '\0'; | |||||
user->name = user->id; | |||||
cp = strchr(user->id, '<'); | |||||
if (cp > user->id) { | |||||
user->id = strdup(user->id); | |||||
cp[-1] = '\0'; | |||||
} | |||||
} | |||||
*pptr += len; | |||||
return ((ssize_t)len); | |||||
} | |||||
/** | |||||
* @brief decode a key packet | |||||
* | |||||
* We only really support v4 and RSA | |||||
* | |||||
* @sa rfc4880:5.5.1.1 | |||||
*/ | |||||
ssize_t | |||||
decode_key(int tag, unsigned char **pptr, size_t len, OpenPGP_key *key) | |||||
{ | |||||
unsigned char *ptr; | |||||
int version; | |||||
#ifdef USE_BEARSSL | |||||
br_sha1_context mctx; | |||||
unsigned char mdata[br_sha512_SIZE]; | |||||
#else | |||||
RSA *rsa = NULL; | |||||
const EVP_MD *md = NULL; | |||||
EVP_MD_CTX mctx; | |||||
unsigned char mdata[EVP_MAX_MD_SIZE]; | |||||
#endif | |||||
size_t mlen; | |||||
if (tag != 6) | |||||
return (-1); | |||||
key->key = NULL; | |||||
ptr = *pptr; | |||||
version = *ptr; | |||||
if (version == 4) { /* all we support really */ | |||||
/* comput key fingerprint and id @sa rfc4880:12.2 */ | |||||
mdata[0] = 0x99; /* rfc4880: 12.2.a.1 */ | |||||
mdata[1] = (len >> 8) & 0xff; | |||||
mdata[2] = len & 0xff; | |||||
#ifdef USE_BEARSSL | |||||
br_sha1_init(&mctx); | |||||
br_sha1_update(&mctx, mdata, 3); | |||||
br_sha1_update(&mctx, ptr, len); | |||||
br_sha1_out(&mctx, mdata); | |||||
mlen = br_sha1_SIZE; | |||||
#else | |||||
md = EVP_get_digestbyname("sha1"); | |||||
EVP_DigestInit(&mctx, md); | |||||
EVP_DigestUpdate(&mctx, mdata, 3); | |||||
EVP_DigestUpdate(&mctx, ptr, len); | |||||
mlen = sizeof(mdata); | |||||
EVP_DigestFinal(&mctx,mdata,(unsigned int *)&mlen); | |||||
#endif | |||||
cem: DigestFinal's 3rd argument is `unsigned int*` — this cast is a NOP.
(If it were not a nop… | |||||
Not Done Inline Actionspretty sure the cast is/was there because one of the toolchains complained sjg: pretty sure the cast is/was there because one of the toolchains complained | |||||
Not Done Inline ActionsAh, the cast is only a no-op if size_t is unsigned int which is certainly not the case for all the architectures that code is compiled for. Note: This is currently dead code for FreeBSD - this being in the not-BEARSSL case, sjg: Ah, the cast is only a no-op if `size_t` is `unsigned int` which is certainly not the case for… | |||||
Not Done Inline ActionsAh, I thought mlen was unsigned int for some reason. I think the dead code should be fixed or removed. If the idea is it may be used in the future, fixing it is the way to go. Casting size_t* to unsigned* is totally bogus. cem: Ah, I thought mlen was unsigned int for some reason. I think the dead code should be fixed or… | |||||
Not Done Inline ActionsSo what is your suggestion for passing a size_t * to an API that wants unsigned int * ? sjg: So what is your suggestion for passing a size_t * to an API that wants unsigned int * ? | |||||
Not Done Inline ActionsIt cannot be done. Use an unsigned stack variable and pass its address instead. cem: It cannot be done. Use an `unsigned` stack variable and pass its address instead. | |||||
Not Done Inline Actionsok will do sjg: ok will do | |||||
key->id = octets2hex(&mdata[mlen - 8], 8); | |||||
} | |||||
Not Done Inline ActionsThe truncated hash key->id is subsequently used for lookup in trusted keys. This is dubious (I don't know whether it does or does not affect the overall security of the system as specifically constructed, but it's not a good practice regardless). Similar truncated truncated hash attacks have been used to attack the PGP/GPG ecosystem in the past, e.g., this from 2014: https://evil32.com/ . The truncated hash is effectively 32 bits, which is far weaker than even SHA1's 160 bits. (Also octets2hex is returning a pointer to its internal static buffer here so in general the key lifetime is questionable.) cem: The truncated hash key->id is subsequently used for lookup in trusted keys. This is dubious (I… | |||||
Not Done Inline ActionsThe key id usage is from the RFC. And yes the the id life time is transitory. sjg: The key id usage is from the RFC.
OpenPGP is obviously not perfect - but it is still in wide… | |||||
Not Done Inline ActionsRFCs aren't intended to be a substitute for thought. What's wrong with identifying keys using the full fingerprint instead of just the last 32 bits? cem: RFCs aren't intended to be a substitute for thought.
What's wrong with identifying keys using… | |||||
Not Done Inline ActionsHave a look at the RFC - or opgp_sig.c - the signature contains the truncated SHA1 hash as key-id - which is how we need to find the key to verify the signature. Using full hash thus would serve no purpose. RFC's are about interoperability as much as anything else. sjg: Have a look at the RFC - or opgp_sig.c - the signature contains the truncated SHA1 hash as key… | |||||
Not Done Inline ActionsHow does such a signature scheme protect against forgery? Can an attacker just generate a key that collides the 32 bit key-id and sign anything as if they are the first key? (I don't think interoperability with a broken scheme is useful.) cem: How does such a signature scheme protect against forgery? Can an attacker just generate a key… | |||||
Not Done Inline ActionsNo, of course you cannot create a key that collides the id, and thus cheat the system. sjg: No, of course you cannot create a key that collides the id, and thus cheat the system.
The key… | |||||
ptr += 1; /* done with version */ | |||||
ptr += 4; /* skip ctime */ | |||||
if (version == 3) | |||||
ptr += 2; /* valid days */ | |||||
key->sig_alg = *ptr++; | |||||
if (key->sig_alg == 1) { /* RSA */ | |||||
#ifdef USE_BEARSSL | |||||
key->key = NEW(br_rsa_public_key); | |||||
if (!key->key) | |||||
goto oops; | |||||
key->key->n = mpi2bn(&ptr, &key->key->nlen); | |||||
key->key->e = mpi2bn(&ptr, &key->key->elen); | |||||
#else | |||||
rsa = RSA_new(); | |||||
if (!rsa) | |||||
goto oops; | |||||
rsa->n = mpi2bn(&ptr); | |||||
rsa->e = mpi2bn(&ptr); | |||||
key->key = EVP_PKEY_new(); | |||||
if (!key->key || !rsa->n || !rsa->e) { | |||||
goto oops; | |||||
} | |||||
if (!EVP_PKEY_set1_RSA(key->key, rsa)) | |||||
goto oops; | |||||
#endif | |||||
} | |||||
/* we are done */ | |||||
return ((ssize_t)len); | |||||
oops: | |||||
#ifdef USE_BEARSSL | |||||
free(key->key); | |||||
key->key = NULL; | |||||
#else | |||||
if (rsa) | |||||
RSA_free(rsa); | |||||
if (key->key) { | |||||
EVP_PKEY_free(key->key); | |||||
key->key = NULL; | |||||
} | |||||
#endif | |||||
return (-1); | |||||
} | |||||
static OpenPGP_key * | |||||
load_key_buf(unsigned char *buf, size_t nbytes) | |||||
{ | |||||
unsigned char *data = NULL; | |||||
unsigned char *ptr; | |||||
ssize_t rc; | |||||
int tag; | |||||
OpenPGP_key *key; | |||||
if (!buf) | |||||
return (NULL); | |||||
initialize(); | |||||
if ((buf[0] & 0200) == 0) { | |||||
data = dearmor((char *)buf, nbytes, &nbytes); | |||||
Not Done Inline ActionsI'm not really pleased to see arbitrary octal constants throughout the source cem: I'm not really pleased to see arbitrary octal constants throughout the source | |||||
Not Done Inline ActionsSuggestions welcome - the goal is to test if 8th bit is set - would a comment help? sjg: Suggestions welcome - the goal is to test if 8th bit is set - would a comment help? | |||||
Not Done Inline ActionsUsually significant bits can be named with macros. Like #define ARMORED 0x80 or whatever makes the most sense here. (And in general hex constants are more common and easier (for me) to understand, but naming is best.) cem: Usually significant bits can be named with macros. Like `#define ARMORED 0x80` or whatever… | |||||
Not Done Inline ActionsARMORED is not really correct - will see if I can think of something suitable. sjg: ARMORED is not really correct - will see if I can think of something suitable.
Octal masks btw… | |||||
ptr = data; | |||||
} else | |||||
ptr = buf; | |||||
key = NEW(OpenPGP_key); | |||||
if (key) { | |||||
rc = decode_packet(0, &ptr, nbytes, (decoder_t)decode_key, | |||||
key); | |||||
if (rc < 0) { | |||||
free(key); | |||||
key = NULL; | |||||
} else if (rc > 8) { | |||||
int isnew, ltype; | |||||
tag = decode_tag(ptr, &isnew, <ype); | |||||
if (tag == 13) { | |||||
key->user = NEW(OpenPGP_user); | |||||
rc = decode_packet(0, &ptr, (size_t)rc, | |||||
(decoder_t)decode_user, key->user); | |||||
} | |||||
} | |||||
} | |||||
free(data); | |||||
return (key); | |||||
} | |||||
static LIST_HEAD(, OpenPGP_key_) trust_list; | |||||
/** | |||||
* @brief add a key to our list | |||||
*/ | |||||
void | |||||
openpgp_trust_add(OpenPGP_key *key) | |||||
{ | |||||
static int once = 0; | |||||
if (!once) { | |||||
once = 1; | |||||
LIST_INIT(&trust_list); | |||||
} | |||||
if (key) | |||||
LIST_INSERT_HEAD(&trust_list, key, entries); | |||||
} | |||||
/** | |||||
* @brief if keyID is in our list return the key | |||||
* | |||||
* @return key or NULL | |||||
*/ | |||||
OpenPGP_key * | |||||
openpgp_trust_get(const char *keyID) | |||||
{ | |||||
OpenPGP_key *key; | |||||
openpgp_trust_add(NULL); /* initialize if needed */ | |||||
LIST_FOREACH(key, &trust_list, entries) { | |||||
if (strcmp(key->id, keyID) == 0) | |||||
return (key); | |||||
} | |||||
return (NULL); | |||||
} | |||||
/** | |||||
* @brief load a key from file | |||||
*/ | |||||
OpenPGP_key * | |||||
load_key_file(const char *kfile) | |||||
{ | |||||
unsigned char *data = NULL; | |||||
size_t n; | |||||
OpenPGP_key *key; | |||||
data = read_file(kfile, &n); | |||||
key = load_key_buf(data, n); | |||||
free(data); | |||||
openpgp_trust_add(key); | |||||
return (key); | |||||
} | |||||
#include <ta_asc.h> | |||||
#ifndef _STANDALONE | |||||
/* we can lookup keyID in filesystem */ | |||||
static const char *trust_store[] = { | |||||
"/var/db/trust", | |||||
"/etc/db/trust", | |||||
NULL, | |||||
}; | |||||
/** | |||||
* @brief lookup key id in trust store | |||||
* | |||||
*/ | |||||
static OpenPGP_key * | |||||
load_trusted_key_id(const char *keyID) | |||||
{ | |||||
char kfile[MAXPATHLEN]; | |||||
const char **tp; | |||||
size_t n; | |||||
for (tp = trust_store; *tp; tp++) { | |||||
n = (size_t)snprintf(kfile, sizeof(kfile), "%s/%s", *tp, keyID); | |||||
if (n >= sizeof(kfile)) | |||||
return (NULL); | |||||
if (access(kfile, R_OK) == 0) { | |||||
return (load_key_file(kfile)); | |||||
} | |||||
Not Done Inline ActionsTOCTOU cem: TOCTOU | |||||
Not Done Inline ActionsSorry what does that mean? sjg: Sorry what does that mean? | |||||
Not Done Inline ActionsThere appears to be a race between Time Of Check and Time Of Use. https://en.wikipedia.org/wiki/Time_of_check_to_time_of_use This is in the section #ifndef standalone so I guess it may be used in ordinary userspace (not just the constrained loader environment). cem: There appears to be a race between Time Of Check and Time Of Use.
https://en.wikipedia. | |||||
Not Done Inline ActionsSince the check is not a verification, I'm not sure what the concern is. sjg: Since the check is not a verification, I'm not sure what the concern is.
If someone plays… | |||||
Not Done Inline ActionsIf the check serves no purpose, why have it? cem: If the check serves no purpose, why have it? | |||||
Not Done Inline ActionsIt serves the purpose of avoiding an attempt to open a file that does not exist. sjg: It serves the purpose of avoiding an attempt to open a file that does not exist. | |||||
} | |||||
return (NULL); | |||||
} | |||||
#endif | |||||
/** | |||||
* @brief return key if trusted | |||||
*/ | |||||
OpenPGP_key * | |||||
load_key_id(const char *keyID) | |||||
{ | |||||
static int once = 0; | |||||
OpenPGP_key *key; | |||||
if (!once) { | |||||
#ifdef ta_ASC | |||||
char *cp; | |||||
size_t n; | |||||
if ((cp = strdup(ta_ASC))) { | |||||
n = strlen(cp); | |||||
key = load_key_buf((unsigned char *)cp, n); | |||||
free(cp); | |||||
openpgp_trust_add(key); | |||||
} | |||||
#endif | |||||
once = 1; | |||||
} | |||||
key = openpgp_trust_get(keyID); | |||||
#ifndef _STANDALONE | |||||
if (!key) | |||||
key = load_trusted_key_id(keyID); | |||||
#endif | |||||
return (key); | |||||
} | |||||
/** | |||||
* @brief test that we can verify a signature | |||||
*/ | |||||
int | |||||
openpgp_self_tests(void) | |||||
{ | |||||
int rc = -1; | |||||
#ifdef ta_ASC | |||||
char *fdata, *sdata = NULL; | |||||
size_t fbytes, sbytes; | |||||
if ((fdata = strdup(ta_ASC)) && | |||||
(sdata = strdup(vc_ASC))) { | |||||
fbytes = strlen(fdata); | |||||
sbytes = strlen(sdata); | |||||
rc = openpgp_verify("ta_ASC", | |||||
(unsigned char *)fdata, fbytes, | |||||
(unsigned char *)sdata, sbytes, 0); | |||||
if (!rc) { | |||||
printf("Testing verify OpenPGP signature:\t\tPassed\n"); | |||||
} | |||||
} | |||||
free(fdata); | |||||
free(sdata); | |||||
#endif | |||||
return (rc); | |||||
} |
DigestFinal's 3rd argument is unsigned int* — this cast is a NOP.
(If it were not a nop, casting pointer out parameters to different sizes is totally bogus in general.)
Style(9) nit: spaces after commas in arguments list