Differential D16335 Diff 46491 lib/libve/openpgp/opgp_key.c

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
				cemUnsubmitted Not Done Inline Actions 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 cem:* DigestFinal's 3rd argument is `unsigned int*` — this cast is a NOP. (If it were not a nop…
				sjgAuthorUnsubmitted Not Done Inline Actions pretty 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
				sjgAuthorUnsubmitted Not Done Inline Actions Ah, 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, but for `sbin/veriexec` D16575 it may be desirable to add back the OpenSSL support. sjg: Ah, the cast is only a no-op if `size_t` is `unsigned int` which is certainly not the case for…
				cemUnsubmitted Not Done Inline Actions Ah, 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…
				sjgAuthorUnsubmitted Not Done Inline Actions So 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 * ?
				cemUnsubmitted Not Done Inline Actions It 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.
				sjgAuthorUnsubmitted Not Done Inline Actions ok will do sjg: ok will do
				key->id = octets2hex(&mdata[mlen - 8], 8);
				}
				cemUnsubmitted Not Done Inline Actions The 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…
				sjgAuthorUnsubmitted Not Done Inline Actions The key id usage is from the RFC. OpenPGP is obviously not perfect - but it is still in wide use. 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…
				cemUnsubmitted Not Done Inline Actions RFCs 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…
				sjgAuthorUnsubmitted Not Done Inline Actions Have 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…
				cemUnsubmitted Not Done Inline Actions How 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…
				sjgAuthorUnsubmitted Not Done Inline Actions No, of course you cannot create a key that collides the id, and thus cheat the system. The key-id is for looking up a trusted key - if the wrong key is found (because you have a collision) then verification will fail. 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);
				cemUnsubmitted Not Done Inline Actions I'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
				sjgAuthorUnsubmitted Not Done Inline Actions Suggestions 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?
				cemUnsubmitted Not Done Inline Actions Usually 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…
				sjgAuthorUnsubmitted Not Done Inline Actions ARMORED is not really correct - will see if I can think of something suitable. Octal masks btw are much easier to translate to binary. 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, &ltype);
				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));
				}
				cemUnsubmitted Not Done Inline Actions TOCTOU cem: TOCTOU
				sjgAuthorUnsubmitted Not Done Inline Actions Sorry what does that mean? sjg: Sorry what does that mean?
				cemUnsubmitted Not Done Inline Actions There 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.
				sjgAuthorUnsubmitted Not Done Inline Actions Since the check is not a verification, I'm not sure what the concern is. If someone plays games with replacing/removing the key during that window, signature verification will fail so a denial of service at worst? If someone is playing such games you are already in trouble. sjg: Since the check is not a verification, I'm not sure what the concern is. If someone plays…
				cemUnsubmitted Not Done Inline Actions If the check serves no purpose, why have it? cem: If the check serves no purpose, why have it?
				sjgAuthorUnsubmitted Not Done Inline Actions It 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 HAVE_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 HAVE_VC_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);
				}