From 0dd1b302b52138048b9f1205b0d8d9c20292e3f0 Mon Sep 17 00:00:00 2001 From: Andrew Bettison Date: Thu, 28 Nov 2013 17:44:37 +1030 Subject: [PATCH] Rewrite Rhizome manifest parsing Move validation checks into new function rhizome_manifest_validate() Remove rhizome_manifest 'errors' field Replace rhizome_manifest 'warnings' with 'malformed' Replace rhizome_manifest 'manifest_bytes' with 'manifest_body_bytes' and refactor to use 'manifest_all_bytes' in all manifest i/o Refactor rhizome_manifest_verify() and reverse sense of return value to match rhizome_manifest_validate() New function rhizome_manifest_inspect() -- lightweight manifest parser used when receiving Rhizome advertisements New 'rhizomeops' test case for invalid manifest "service" field values, now passes --- commandline.c | 28 +- overlay_buffer.c | 13 +- overlay_buffer.h | 3 +- rhizome.c | 9 +- rhizome.h | 40 +- rhizome_bundle.c | 792 +++++++++++++++++++++++----------------- rhizome_crypto.c | 102 ++---- rhizome_database.c | 43 +-- rhizome_direct.c | 11 +- rhizome_direct_http.c | 4 +- rhizome_fetch.c | 13 +- rhizome_packetformats.c | 74 ++-- tests/rhizomeops | 9 +- tests/rhizomeprotocol | 2 + 14 files changed, 619 insertions(+), 524 deletions(-) diff --git a/commandline.c b/commandline.c index b5baeb0b..e497dcb7 100644 --- a/commandline.c +++ b/commandline.c @@ -1338,8 +1338,9 @@ int app_rhizome_add_file(const struct cli_parsed *parsed, struct cli_context *co DEBUGF("reading manifest from %s", manifestpath); /* Don't verify the manifest, because it will fail if it is incomplete. This is okay, because we fill in any missing bits and sanity check before - trying to write it out. */ - if (rhizome_read_manifest_file(m, manifestpath, 0) == -1) { + trying to write it out. However, we do insist that whatever we load is + valid and not malformed. */ + if (rhizome_read_manifest_file(m, manifestpath, 0) == -1 || m->malformed) { rhizome_manifest_free(m); keyring_free(keyring); return WHY("Manifest file could not be loaded -- not added to rhizome"); @@ -1584,22 +1585,19 @@ int app_rhizome_append_manifest(const struct cli_parsed *parsed, struct cli_cont if ( cli_arg(parsed, "manifestpath", &manifestpath, NULL, "") == -1 || cli_arg(parsed, "filepath", &filepath, NULL, "") == -1) return -1; - rhizome_manifest *m = rhizome_new_manifest(); if (!m) return WHY("Out of manifests."); - - int ret=0; - if (rhizome_read_manifest_file(m, manifestpath, 0) == -1) - ret=-1; - // TODO why doesn't read manifest file set finalised??? - m->finalised=1; - - if (ret==0 && rhizome_write_manifest_file(m, filepath, 1) == -1) - ret = -1; - - if (m) - rhizome_manifest_free(m); + int ret = -1; + if ( rhizome_read_manifest_file(m, manifestpath, 0) != -1 + && rhizome_manifest_validate(m) + && rhizome_manifest_verify(m) + ) { + assert(m->finalised); + if (rhizome_write_manifest_file(m, filepath, 1) != -1) + ret = 0; + } + rhizome_manifest_free(m); return ret; } diff --git a/overlay_buffer.c b/overlay_buffer.c index 06686e3d..1d05dcb2 100644 --- a/overlay_buffer.c +++ b/overlay_buffer.c @@ -416,12 +416,17 @@ int test_offset(struct overlay_buffer *b,int start,int length) return 0; } -int ob_getbyte(struct overlay_buffer *b, int ofs) +// next byte without advancing +int ob_peek(struct overlay_buffer *b) { - if (test_offset(b, ofs, 1)) + if (test_offset(b, b->position, 1)) return -1; - - return b->bytes[ofs]; + return b->bytes[b->position]; +} + +void ob_skip(struct overlay_buffer *b, unsigned n) +{ + b->position += n; } int ob_get_bytes(struct overlay_buffer *b, unsigned char *buff, int len){ diff --git a/overlay_buffer.h b/overlay_buffer.h index dd7ba4a0..cf69a4b9 100644 --- a/overlay_buffer.h +++ b/overlay_buffer.h @@ -95,7 +95,8 @@ void _ob_append_rfs(struct __sourceloc whence, struct overlay_buffer *b,int l); #define ob_append_rfs(b, l) _ob_append_rfs(__WHENCE__, b, l) // get one byte, -ve number indicates failure -int ob_getbyte(struct overlay_buffer *b,int ofs); +int ob_peek(struct overlay_buffer *b); +void ob_skip(struct overlay_buffer *b, unsigned n); // get one byte from the current position, -ve number indicates failure int ob_get(struct overlay_buffer *b); int ob_get_bytes(struct overlay_buffer *b, unsigned char *buff, int len); diff --git a/rhizome.c b/rhizome.c index f063ee69..56378233 100644 --- a/rhizome.c +++ b/rhizome.c @@ -130,14 +130,11 @@ int rhizome_bundle_import_files(rhizome_manifest *m, const char *manifest_path, if (rhizome_read_manifest_file(m, manifest_path, buffer_len) == -1) return WHY("could not read manifest file"); - if (rhizome_manifest_verify(m)) + if (!rhizome_manifest_validate(m)) + return WHY("manifest is invalid"); + if (!rhizome_manifest_verify(m)) return WHY("could not verify manifest"); - /* Make sure we store signatures */ - // TODO, why do we need this? Why isn't the state correct from rhizome_read_manifest_file? - // This feels like a hack... - m->manifest_bytes=m->manifest_all_bytes; - /* Do we already have this manifest or newer? */ int64_t dbVersion = -1; if (sqlite_exec_int64(&dbVersion, "SELECT version FROM MANIFESTS WHERE id = ?;", RHIZOME_BID_T, &m->cryptoSignPublic, END) == -1) diff --git a/rhizome.h b/rhizome.h index 7b171a15..e2a284b9 100644 --- a/rhizome.h +++ b/rhizome.h @@ -124,7 +124,7 @@ extern time_ms_t rhizome_voice_timeout; typedef struct rhizome_signature { unsigned char signature[crypto_sign_edwards25519sha512batch_BYTES +crypto_sign_edwards25519sha512batch_PUBLICKEYBYTES+1]; - int signatureLength; + size_t signatureLength; } rhizome_signature; #define RHIZOME_BAR_BYTES 32 @@ -191,18 +191,15 @@ typedef struct rhizome_manifest unsigned char *signatories[MAX_MANIFEST_VARS]; uint8_t signatureTypes[MAX_MANIFEST_VARS]; - /* Imperfections. - * - Errors involve the correctness of fields that are mandatory for proper - * operation of the transport and storage layer. A manifest with errors > 0 - * must not be stored, transmitted or supplied via any API. - * - Warnings indicate a manifest that cannot be fully understood by this - * version of Rhizome (probably from a future or a very old past version - * of Rhizome). During add or import (local injection), the manifest - * should not be imported. During extract or export (local) a warning or - * error message should be logged. + /* Set to non-zero if a manifest has been parsed that cannot be fully + * understood by this version of Rhizome (probably from a future or a very + * old past version of Rhizome). During add (local injection), the manifest + * should not be imported. During extract (local decode) a warning or error + * message should be logged. Manifests marked as malformed are still + * transported, imported and exported normally, as long as their signature is + * valid. */ - unsigned short errors; - unsigned short warnings; + unsigned short malformed; /* Set non-zero after variables have been packed and signature blocks * appended. All fields below may not be valid until the manifest has been @@ -211,7 +208,7 @@ typedef struct rhizome_manifest bool_t finalised; /* Whether the manifest contains a signature that corresponds to the manifest - * id (ie public key). Caches the result of + * id (ie public key). */ bool_t selfSigned; @@ -219,6 +216,10 @@ typedef struct rhizome_manifest */ bool_t dataFileUnlinkOnFree; + /* Set if the ID field (cryptoSignPublic) contains a bundle ID. + */ + bool_t has_id; + /* Set if the tail field is valid, ie, the bundle is a journal. */ bool_t is_journal; @@ -315,8 +316,8 @@ typedef struct rhizome_manifest unsigned group_count; char *groups[MAX_MANIFEST_VARS]; - unsigned manifest_bytes; - unsigned manifest_all_bytes; + size_t manifest_body_bytes; + size_t manifest_all_bytes; unsigned char manifestdata[MAX_MANIFEST_BYTES]; unsigned char manifesthash[crypto_hash_sha512_BYTES]; @@ -448,11 +449,20 @@ sqlite_retry_state sqlite_retry_state_init(int serverLimit, int serverSleep, int #define SQLITE_RETRY_STATE_DEFAULT sqlite_retry_state_init(-1,-1,-1,-1) +struct rhizome_manifest_summary { + rhizome_bid_t bid; + int64_t version; + size_t body_len; +}; + +int rhizome_manifest_inspect(const char *buf, size_t len, struct rhizome_manifest_summary *summ); + int rhizome_write_manifest_file(rhizome_manifest *m, const char *filename, char append); int rhizome_manifest_selfsign(rhizome_manifest *m); int rhizome_drop_stored_file(const rhizome_filehash_t *hashp, int maximum_priority); int rhizome_manifest_priority(sqlite_retry_state *retry, const rhizome_bid_t *bidp); int rhizome_read_manifest_file(rhizome_manifest *m, const char *filename, size_t bufferPAndSize); +int rhizome_manifest_validate(rhizome_manifest *m); int rhizome_hash_file(rhizome_manifest *m, const char *path, rhizome_filehash_t *hash_out, uint64_t *size_out); void _rhizome_manifest_free(struct __sourceloc __whence, rhizome_manifest *m); diff --git a/rhizome_bundle.c b/rhizome_bundle.c index 8ecf5b51..9a7cb590 100644 --- a/rhizome_bundle.c +++ b/rhizome_bundle.c @@ -92,7 +92,7 @@ static const char *_rhizome_manifest_set(struct __sourceloc __whence, rhizome_ma m->finalised = 0; return ret; } - if (m->var_count >= MAX_MANIFEST_VARS) + if (m->var_count >= NELS(m->vars)) return WHYNULL("no more manifest vars"); if ((m->vars[m->var_count] = str_edup(var)) == NULL) return NULL; @@ -344,72 +344,52 @@ void _rhizome_manifest_del_author(struct __sourceloc __whence, rhizome_manifest } } +/* Compute the hash of the manifest's body, including the NUL byte that separates the body from + * the signature block, and verify that a signature is present and is correct. + * + * Returns 1 if the manifest signature is valid, ie, the signature is a self-signature using the + * manifest's own private key. Sets the m->finalised flag to 1. + * + * Returns 0 if there are no signatures or if the signature block does not verify. + * + * Only call this function on manifests for which rhizome_manifest_validate(m) has returned true. + */ int rhizome_manifest_verify(rhizome_manifest *m) { - unsigned end_of_text=0; - - /* find end of manifest body and start of signatures */ - while(m->manifestdata[end_of_text]&&end_of_textmanifest_all_bytes) - end_of_text++; - end_of_text++; /* include null byte in body for verification purposes */ - - /* Calculate hash of the text part of the file, as we need to couple this with - each signature block to */ - crypto_hash_sha512(m->manifesthash,m->manifestdata,end_of_text); - - /* Read signature blocks from file. */ - unsigned ofs = end_of_text; - while(ofsmanifest_all_bytes) { - if (config.debug.rhizome) - DEBUGF("ofs=0x%x, m->manifest_bytes=0x%x", ofs,m->manifest_all_bytes); - if (rhizome_manifest_extract_signature(m, &ofs)) + assert(m->manifest_body_bytes > 0); + assert(m->manifest_all_bytes > 0); + assert(m->manifest_body_bytes <= m->manifest_all_bytes); + if (m->manifest_body_bytes == m->manifest_all_bytes) + assert(m->manifestdata[m->manifest_body_bytes - 1] == '\0'); + // Hash the body + crypto_hash_sha512(m->manifesthash, m->manifestdata, m->manifest_body_bytes); + // Read signature blocks + unsigned ofs = m->manifest_body_bytes; + while (ofs < m->manifest_all_bytes) { + if (rhizome_manifest_extract_signature(m, &ofs) == -1) break; } + assert(ofs <= m->manifest_all_bytes); - if (m->sig_count==0) { - WHYF("Manifest has zero valid signatures"); - m->errors++; + // Make sure the first signatory's public key is the bundle ID + assert(m->has_id); + if (m->sig_count == 0) { + if (config.debug.rhizome) + DEBUG("Manifest has no signature blocks, but should have self-signature block"); + m->selfSigned = 0; + return 0; } - - /* Make sure that id variable is correct */ - { - rhizome_bid_t bid; - const char *id = rhizome_manifest_get(m,"id"); - if (!id) { - WHY("Manifest lacks 'id' field"); - m->errors++; - } else if (str_to_rhizome_bid_t(&bid, id) == -1) { - WHY("Invalid manifest 'id' field"); - m->errors++; - } else if (cmp_rhizome_bid_t(&bid, &m->cryptoSignPublic) != 0) { - WHYF("Manifest id field does not match cryptoSignPublic: id=%s, cryptoSignPublic=%s", - alloca_tohex_rhizome_bid_t(bid), - alloca_tohex_rhizome_bid_t(m->cryptoSignPublic) - ); - m->errors++; - } else if (m->sig_count == 0 || memcmp(m->signatories[0], bid.binary, sizeof bid.binary) != 0) { - if (config.debug.rhizome) { - if (m->sig_count>0) { - DEBUGF("Manifest id variable does not match first signature block (signature key is %s)", - alloca_tohex(m->signatories[0], crypto_sign_edwards25519sha512batch_PUBLICKEYBYTES) - ); - } else { - DEBUG("Manifest has no signature blocks, but should have self-signature block"); - } - } - m->errors++; - m->selfSigned = 0; - } else - m->selfSigned = 1; + if (memcmp(m->signatories[0], m->cryptoSignPublic.binary, sizeof m->cryptoSignPublic.binary) != 0) { + if (config.debug.rhizome) + DEBUGF("Manifest id does not match first signature block (signature key is %s)", + alloca_tohex(m->signatories[0], crypto_sign_edwards25519sha512batch_PUBLICKEYBYTES) + ); + m->selfSigned = 0; + return 0; } - - /* Mark as finalised, as it is all read and intact, - unless of course it has errors, or is lacking a self-signature. */ - if (!m->errors) m->finalised=1; - else WHY("Verified a manifest that has errors, so marking as not finalised"); - - if (m->errors) return WHY("Manifest verification failed"); - else return 0; + m->selfSigned = 1; + m->finalised = 1; + return 1; } ssize_t read_whole_file(const char *path, unsigned char *buffer, size_t buffer_size) @@ -425,259 +405,396 @@ ssize_t read_whole_file(const char *path, unsigned char *buffer, size_t buffer_s return ret; } -int rhizome_manifest_parse(rhizome_manifest *m) +static void rhizome_manifest_clear(rhizome_manifest *m) { - IN(); - m->manifest_all_bytes=m->manifest_bytes; - m->var_count = 0; + while (m->var_count) { + --m->var_count; + free((char *) m->vars[m->var_count]); + free((char *) m->values[m->var_count]); + m->vars[m->var_count] = m->values[m->var_count] = NULL; + } + while (m->sig_count) { + --m->sig_count; + free(m->signatories[m->sig_count]); + m->signatories[m->sig_count] = NULL; + } + m->malformed = 0; + m->has_id = 0; + m->is_journal = 0; m->filesize = RHIZOME_SIZE_UNSET; m->tail = RHIZOME_SIZE_UNSET; + m->version = 0; + // TODO initialise more fields +} - /* Parse out variables, signature etc */ - int have_id = 0; - int have_version = 0; - int have_filehash = 0; - - unsigned ofs = 0; - while (ofs < m->manifest_bytes && m->manifestdata[ofs]) { - char line[1024]; - unsigned limit = ofs + sizeof line - 1; - if (limit > m->manifest_bytes) - limit = m->manifest_bytes; - char *p = line; - while (ofs < limit && !(m->manifestdata[ofs] == '\0' || m->manifestdata[ofs] == '\n' || m->manifestdata[ofs] == '\r')) - *p++ = m->manifestdata[ofs++]; - *p = '\0'; - if (m->manifestdata[ofs] == '\r') - ++ofs; - if (m->manifestdata[ofs] == '\n') - ++ofs; - /* Ignore blank lines */ - if (line[0] == '\0') - continue; - /* Ignore comment lines */ - if (line[0] == '#' || line[0] == '!') - continue; - /* Parse field=value lines */ - size_t linelen = p - line; - p = strchr(line, '='); - if (p == NULL || p == line) { - m->errors++; - WARNF("Malformed manifest line: %s", alloca_toprint(80, line, linelen)); - } else { - *p++ = '\0'; - char *var = line; - char *value = p; - if (rhizome_manifest_get(m, var)) { - if (config.debug.rejecteddata) - DEBUGF("Ill formed manifest file, duplicate variable \"%s\"", var); - m->errors++; - } else if (m->var_count >= MAX_MANIFEST_VARS) { - if (config.debug.rejecteddata) - WARN("Ill formed manifest file, too many variables"); - m->errors++; - } else { - m->vars[m->var_count] = strdup(var); - m->values[m->var_count] = strdup(value); - /* The bundle ID is implicit in transit, but we need to store it in the manifest, so that - * reimporting manifests on receiver nodes works easily. We might implement something that - * strips the id variable out of the manifest when sending it, or some other scheme to avoid - * sending all the extra bytes. - */ - if (strcasecmp(var, "id") == 0) { - have_id = 1; - if (str_to_rhizome_bid_t(&m->cryptoSignPublic, value) == -1) { - if (config.debug.rejecteddata) - DEBUGF("Invalid manifest id: %s", value); - m->errors++; - } else { - if (config.debug.rhizome_manifest) - DEBUGF("PARSE manifest[%d].id = %s", m->manifest_record_number, alloca_tohex_sid_t(m->cryptoSignPublic)); +int rhizome_manifest_inspect(const char *buf, size_t len, struct rhizome_manifest_summary *summ) +{ + const char *const end = buf + len; + int has_bid = 0; + int has_version = 0; + const char *begin = buf; + enum { Label, Value, Error } state = Label; + const char *p; + for (p = buf; state != Error && p < end && *p; ++p) + switch (state) { + case Label: + if (*p == '=') { + if (p == begin) + state = Error; // nil field name + else { + int *has = NULL; + if (p == begin + 2 && strncmp(begin, "id", 2) == 0) + has = &has_bid; + else if (p == begin + 7 && strncmp(begin, "version", 7) == 0) + has = &has_version; + state = Value; + if (has) { + if (*has) + state = Error; // duplicate + else { + *has = 1; + begin = p + 1; + } + } } - } else if (strcasecmp(var, "filehash") == 0) { - have_filehash = 1; - if (str_to_rhizome_filehash_t(&m->filehash, value) == -1) { - if (config.debug.rejecteddata) - DEBUGF("Invalid filehash: %s", value); - m->errors++; - } else { - if (config.debug.rhizome_manifest) - DEBUGF("PARSE manifest[%d].filehash = %s", m->manifest_record_number, alloca_tohex_rhizome_filehash_t(m->filehash)); + } else if (!(p == begin ? isalpha(*p) : isalnum(*p))) + state = Error; // bad field name + break; + case Value: + if (*p == '\n') { + const char *eol = p[-1] == '\r' ? p - 1 : p; + if (has_bid == 1) { + const char *e; + if (strn_to_rhizome_bid_t(&summ->bid, begin, &e) == 0 && e == eol) + has_bid = 2; + else + state = Error; // invalid "id" field + } else if (has_version == 1) { + const char *e; + if (str_to_uint64(begin, 10, (uint64_t*)&summ->version, &e) && e == eol) + has_version = 2; + else + state = Error; // invalid "version" field } - } else if (strcasecmp(var, "filesize") == 0) { - uint64_t filesize; - if (!str_to_uint64(value, 10, &filesize, NULL) || filesize == RHIZOME_SIZE_UNSET) { - if (config.debug.rejecteddata) - DEBUGF("Invalid filesize: %s", value); - m->errors++; - } else { - m->filesize = filesize; - if (config.debug.rhizome_manifest) - DEBUGF("PARSE manifest[%d].filesize = %"PRIu64, m->manifest_record_number, m->filesize); + if (state == Value) { + state = Label; + begin = p + 1; } - } else if (strcasecmp(var, "version") == 0) { - have_version = 1; - uint64_t version; - if (!str_to_uint64(value, 10, &version, NULL)) { - if (config.debug.rejecteddata) - DEBUGF("Invalid version: %s", value); - m->errors++; - } else { - m->version = version; - if (config.debug.rhizome_manifest) - DEBUGF("PARSE manifest[%d].version = %"PRIu64, m->manifest_record_number, m->version); - } - // since rhizome *MUST* be able to carry future manifest versions - // if any of these fields are not well formed, the manifest can still be imported and exported - // but the bundle should not be added or exported - } else if (strcasecmp(var, "tail") == 0) { - uint64_t tail; - if (!str_to_uint64(value, 10, &tail, NULL) || tail == RHIZOME_SIZE_UNSET) { - if (config.debug.rejecteddata) - DEBUGF("Invalid tail: %s", value); - m->errors++; - } else { - m->tail = tail; - m->is_journal = 1; - if (config.debug.rhizome_manifest) - DEBUGF("PARSE manifest[%d].tail = %"PRIu64, m->manifest_record_number, m->tail); - } - } else if (strcasecmp(var, "BK") == 0) { - if (str_to_rhizome_bk_t(&m->bundle_key, value) == -1) { - if (config.debug.rejecteddata) - DEBUGF("Invalid BK: %s", value); - m->warnings++; - } else { - m->has_bundle_key = 1; - if (config.debug.rhizome_manifest) - DEBUGF("PARSE manifest[%d].BK = %s", m->manifest_record_number, alloca_tohex_rhizome_bk_t(m->bundle_key)); - } - } else if (strcasecmp(var, "service") == 0) { - if (rhizome_str_is_manifest_service(value)) { - m->service = m->values[m->var_count]; // will be free()d when vars[] and values[] are free()d - if (config.debug.rhizome_manifest) - DEBUGF("PARSE manifest[%d].service = %s", m->manifest_record_number, alloca_str_toprint(m->service)); - } else { - if (config.debug.rejecteddata) - DEBUGF("Invalid service: %s", value); - m->warnings++; - } - } else if (strcasecmp(var, "date") == 0) { - int64_t date; - if (!str_to_int64(value, 10, &date, NULL)) { - if (config.debug.rejecteddata) - DEBUGF("Invalid date: %s", value); - m->warnings++; - } else { - m->date = date; - m->has_date = 1; - if (config.debug.rhizome_manifest) - DEBUGF("PARSE manifest[%d].date = %"PRItime_ms_t, m->manifest_record_number, m->date); - } - } else if (strcasecmp(var, "sender") == 0) { - if (str_to_sid_t(&m->sender, value) == -1) { - if (config.debug.rejecteddata) - DEBUGF("Invalid sender: %s", value); - m->warnings++; - } else { - m->has_sender = 1; - if (config.debug.rhizome_manifest) - DEBUGF("PARSE manifest[%d].sender = %s", m->manifest_record_number, alloca_tohex_sid_t(m->sender)); - } - } else if (strcasecmp(var, "recipient") == 0) { - if (str_to_sid_t(&m->recipient, value) == -1) { - if (config.debug.rejecteddata) - DEBUGF("Invalid recipient: %s", value); - m->warnings++; - } else { - m->has_recipient = 1; - if (config.debug.rhizome_manifest) - DEBUGF("PARSE manifest[%d].recipient = %s", m->manifest_record_number, alloca_tohex_sid_t(m->recipient)); - } - } else if (strcasecmp(var, "name") == 0) { - if (value[0] == '\0') { - if (config.debug.rejecteddata) - WARN("Empty name"); - //m->warnings++; TODO Meshms code should set a name for its "conversations" bundle - } - m->name = m->values[m->var_count]; // will be free()d when vars[] and values[] are free()d - if (config.debug.rhizome_manifest) - DEBUGF("PARSE manifest[%d].name = %s", m->manifest_record_number, alloca_str_toprint(m->name)); - } else if (strcasecmp(var, "crypt") == 0) { - if (!(strcmp(value, "0") == 0 || strcmp(value, "1") == 0)) { - if (config.debug.rejecteddata) - DEBUGF("Invalid crypt: %s", value); - m->warnings++; - } else { - m->payloadEncryption = (value[0] == '1') ? PAYLOAD_ENCRYPTED : PAYLOAD_CLEAR; - if (config.debug.rhizome_manifest) - DEBUGF("PARSE manifest[%d].crypt = %u", m->manifest_record_number, m->payloadEncryption == PAYLOAD_ENCRYPTED ? 1 : 0); - } - } else { - // An unknown field is not an error... older rhizome nodes must carry newer manifests. - if (config.debug.rhizome_manifest) - DEBUGF("SKIP manifest[%d].%s = %s", m->manifest_record_number, var, alloca_str_toprint(value)); } - m->var_count++; - } + break; + default: + abort(); } - } - /* The null byte gets included in the check sum */ - if (ofs < m->manifest_bytes) - ++ofs; + if (p < end && *p == '\0') + ++p; + summ->body_len = p - buf; + return state == Label && has_bid == 2 && has_version == 2; +} - /* Remember where the text ends */ - unsigned end_of_text = ofs; - m->manifest_bytes = end_of_text; +/* Parse a Rhizome text manifest from its internal buffer up to and including the terminating NUL + * character which marks the start of the signature block. + * + * Prior to calling, the caller must set up m->manifest_all_bytes to the length of the manifest + * text, including the signature block, and set m->manifestdata[0..m->manifest_all_bytes-1] to + * contain the manifest text and signature block to be parsed. + * + * A "well formed" manifest consists of a series of zero or more lines with the form: + * + * LABEL "=" VALUE [ CR ] LF + * + * where LABEL matches the regular expression [A-Za-z][A-Za-z0-9]* (identifier without underscore) + * VALUE is any value that does not contain NUL, CR or LF (leading and trailing spaces are + * not stripped from VALUE) + * NUL is ASCII 0 + * CR is ASCII 13 + * LF is ASCII 10 + * + * Unpacks all parsed field labels and string values into the m->vars[] and m->values[] arrays, as + * pointers to malloc(3)ed NUL terminated strings, in the order they appear, and sets m->var_count + * to the number of fields unpacked. Sets m->manifest_body_bytes to the number of bytes in the text + * portion up to and including the optional NUL that starts the signature block (if present). + * + * Returns 1 if the manifest is not well formed (syntax violation), any essential field is + * malformed, or if there are any duplicate fields. In this case the m->vars[] and m->values[] + * arrays are not set and the manifest is returned to the state it was in prior to calling. + * + * Returns 0 if the manifest is well formed, if there are no duplicate fields, and if all essential + * fields are valid. Counts invalid non-essential fields and unrecognised fields in m->malformed. + * + * Returns -1 if there is an unrecoverable error (eg, malloc(3) returns NULL, out of memory). + * + * @author Andrew Bettison + */ +static int rhizome_manifest_parse(rhizome_manifest *m) +{ + IN(); + assert(m->manifest_all_bytes <= sizeof m->manifestdata); + assert(m->manifest_body_bytes == 0); + assert(m->var_count == 0); + assert(!m->malformed); + assert(!m->has_id); + assert(!m->is_journal); + assert(m->filesize == RHIZOME_SIZE_UNSET); + assert(m->tail == RHIZOME_SIZE_UNSET); + assert(m->version == 0); - // verify that all required fields are consistent. - if (!have_id) { - if (config.debug.rejecteddata) - DEBUG("Missing manifest id field"); - m->errors++; - } - if (!have_version) { - if (config.debug.rejecteddata) - DEBUG("Missing version field"); - m->errors++; - } - if (m->filesize == RHIZOME_SIZE_UNSET) { - if (config.debug.rejecteddata) - DEBUG("Missing filesize field"); - m->errors++; - } - if (!have_filehash && m->filesize > 0) { - if (config.debug.rejecteddata) - DEBUG("Missing filehash field"); - m->errors++; - } - if (have_filehash && m->filesize == 0) { - if (config.debug.rejecteddata) - DEBUG("Spurious filehash field"); - m->errors++; - } + unsigned has_invalid_essential = 0; + unsigned has_duplicate = 0; - // warn if expected fields are missing - if (m->service == NULL) { - if (config.debug.rejecteddata) - DEBUG("Missing service field"); - m->warnings++; + const char *const end = (const char *)m->manifestdata + m->manifest_all_bytes; + const char *p; + unsigned line_number = 0; + for (p = (const char *)m->manifestdata; p < end && *p; ++p) { + ++line_number; + if (!isalpha(*p)) { + if (config.debug.rhizome_manifest) + DEBUGF("Invalid manifest field name at line %u: %s", line_number, alloca_toprint(20, p, end - p)); + break; + } + const char *const plabel = p++; + while (p < end && isalnum(*p)) + ++p; + if (*p != '=') { + if (config.debug.rhizome_manifest) + DEBUGF("Invalid manifest field name at line %u: %s", line_number, alloca_toprint(-1, plabel, p - plabel + 1)); + break; + } + const char *const pvalue = ++p; + while (p < end && *p && *p != '\n') + ++p; + if (p >= end || *p != '\n') { + if (config.debug.rhizome_manifest) + DEBUGF("Missing manifest newline at line %u: %s", line_number, alloca_toprint(-1, plabel, p - plabel)); + break; + } + const char *const eol = (p > pvalue && p[-1] == '\r') ? p - 1 : p; + if (m->var_count >= NELS(m->vars)) { + if (config.debug.rhizome_manifest) + DEBUGF("Manifest field limit reached at line %u", line_number); + break; + } + assert(pvalue - plabel - 1 > 0); + const char *label = strn_edup(plabel, pvalue - plabel - 1); + const char *value = strn_edup(pvalue, eol - pvalue); + if (label == NULL || value == NULL) { + free((char *)label); + free((char *)value); + RETURN(-1); + } + enum { FIELD_UNKNOWN, FIELD_OK, FIELD_DUPLICATE, FIELD_MALFORMED, FIELD_INVALID } status = FIELD_UNKNOWN; + if (rhizome_manifest_get(m, label)) + status = FIELD_DUPLICATE; + else if (strcasecmp(label, "id") == 0) { + if (str_to_rhizome_bid_t(&m->cryptoSignPublic, value) != -1) { + status = FIELD_OK; + m->has_id = 1; + if (config.debug.rhizome_manifest) + DEBUGF("PARSE manifest[%d].id = %s", m->manifest_record_number, alloca_tohex_sid_t(m->cryptoSignPublic)); + } else + status = FIELD_INVALID; + } + else if (strcasecmp(label, "version") == 0) { + uint64_t version; + if (str_to_uint64(value, 10, &version, NULL) && version != 0) { + status = FIELD_OK; + m->version = version; + if (config.debug.rhizome_manifest) + DEBUGF("PARSE manifest[%d].version = %"PRIu64, m->manifest_record_number, m->version); + } else + status = FIELD_INVALID; + } + else if (strcasecmp(label, "filehash") == 0) { + if (str_to_rhizome_filehash_t(&m->filehash, value) != -1 && !rhizome_filehash_t_is_zero(m->filehash)) { + status = FIELD_OK; + if (config.debug.rhizome_manifest) + DEBUGF("PARSE manifest[%d].filehash = %s", m->manifest_record_number, alloca_tohex_rhizome_filehash_t(m->filehash)); + } else + status = FIELD_INVALID; + } + else if (strcasecmp(label, "filesize") == 0) { + uint64_t filesize; + if (str_to_uint64(value, 10, &filesize, NULL) && filesize != RHIZOME_SIZE_UNSET) { + status = FIELD_OK; + m->filesize = filesize; + if (config.debug.rhizome_manifest) + DEBUGF("PARSE manifest[%d].filesize = %"PRIu64, m->manifest_record_number, m->filesize); + } else + status = FIELD_INVALID; + } + else if (strcasecmp(label, "tail") == 0) { + uint64_t tail; + if (str_to_uint64(value, 10, &tail, NULL) && tail != RHIZOME_SIZE_UNSET) { + status = FIELD_OK; + m->tail = tail; + m->is_journal = 1; + if (config.debug.rhizome_manifest) + DEBUGF("PARSE manifest[%d].tail = %"PRIu64, m->manifest_record_number, m->tail); + } else + status = FIELD_INVALID; + } + // Since rhizome MUST be able to carry future manifest versions, if any of the following fields + // are not well formed, they are simply not unpacked into their respective struct elements and + // treated as an unrecognised field. The m->malformed flag is set so that the application API + // layer can refuse to add (or export?) the bundle. + else if (strcasecmp(label, "BK") == 0) { + if (str_to_rhizome_bk_t(&m->bundle_key, value) != -1) { + status = FIELD_OK; + m->has_bundle_key = 1; + if (config.debug.rhizome_manifest) + DEBUGF("PARSE manifest[%d].BK = %s", m->manifest_record_number, alloca_tohex_rhizome_bk_t(m->bundle_key)); + } else + status = FIELD_MALFORMED; + } + else if (strcasecmp(label, "service") == 0) { + if (rhizome_str_is_manifest_service(value)) { + status = FIELD_OK; + m->service = value; // will be free()d when vars[] and values[] are free()d + if (config.debug.rhizome_manifest) + DEBUGF("PARSE manifest[%d].service = %s", m->manifest_record_number, alloca_str_toprint(m->service)); + } else + status = FIELD_MALFORMED; + } + else if (strcasecmp(label, "date") == 0) { + int64_t date; + if (str_to_int64(value, 10, &date, NULL)) { + status = FIELD_OK; + m->date = date; + m->has_date = 1; + if (config.debug.rhizome_manifest) + DEBUGF("PARSE manifest[%d].date = %"PRItime_ms_t, m->manifest_record_number, m->date); + } else + status = FIELD_MALFORMED; + } + else if (strcasecmp(label, "sender") == 0) { + if (str_to_sid_t(&m->sender, value) != -1) { + status = FIELD_OK; + m->has_sender = 1; + if (config.debug.rhizome_manifest) + DEBUGF("PARSE manifest[%d].sender = %s", m->manifest_record_number, alloca_tohex_sid_t(m->sender)); + } else + status = FIELD_MALFORMED; + } + else if (strcasecmp(label, "recipient") == 0) { + if (str_to_sid_t(&m->recipient, value) != -1) { + status = FIELD_OK; + m->has_recipient = 1; + if (config.debug.rhizome_manifest) + DEBUGF("PARSE manifest[%d].recipient = %s", m->manifest_record_number, alloca_tohex_sid_t(m->recipient)); + } else + status = FIELD_MALFORMED; + } + else if (strcasecmp(label, "name") == 0) { + status = FIELD_OK; + m->name = value; // will be free()d when vars[] and values[] are free()d + if (config.debug.rhizome_manifest) + DEBUGF("PARSE manifest[%d].name = %s", m->manifest_record_number, alloca_str_toprint(m->name)); + } + else if (strcasecmp(label, "crypt") == 0) { + if (strcmp(value, "0") == 0 || strcmp(value, "1") == 0) { + status = FIELD_OK; + m->payloadEncryption = (value[0] == '1') ? PAYLOAD_ENCRYPTED : PAYLOAD_CLEAR; + if (config.debug.rhizome_manifest) + DEBUGF("PARSE manifest[%d].crypt = %u", m->manifest_record_number, m->payloadEncryption == PAYLOAD_ENCRYPTED ? 1 : 0); + } else + status = FIELD_MALFORMED; + } + const char *reason = NULL; + switch (status) { + case FIELD_OK: + m->vars[m->var_count] = label; + m->values[m->var_count] = value; + ++m->var_count; + break; + case FIELD_DUPLICATE: + ++has_duplicate; + reason = "duplicate"; + break; + case FIELD_INVALID: + ++has_invalid_essential; + reason = "invalid"; + break; + case FIELD_UNKNOWN: + ++m->malformed; + reason = "unsupported"; + break; + case FIELD_MALFORMED: + ++m->malformed; + reason = "invalid"; + break; + default: + abort(); + } + if (reason) { + if (config.debug.rhizome_manifest) + DEBUGF("SKIP manifest[%d].%s = %s (%s)", m->manifest_record_number, label, alloca_str_toprint(value), reason); + free((char *)label); + free((char *)value); + } + assert(p < end); + assert(*p == '\n'); } - if (!m->has_date) { - if (config.debug.rejecteddata) - DEBUG("Missing date field"); - m->warnings++; + if ((p < end && *p) || has_invalid_essential || has_duplicate) { + rhizome_manifest_clear(m); + RETURN(1); } - - if (m->errors || m->warnings) { - if (config.debug.rejecteddata) - dump("manifest body", m->manifestdata, (size_t) m->manifest_bytes); + // The null byte is included in the body (and checksum), not the signature block + if (p < end) { + assert(*p == '\0'); + ++p; } - + m->manifest_body_bytes = p - (const char *)m->manifestdata; RETURN(0); OUT(); } +/* Return 1 if all necessary fields are present, 0 if not. Increment m->malformed if any + * unnecessary fields are missing. + */ +int rhizome_manifest_validate(rhizome_manifest *m) +{ + int ret = 1; + if (!m->has_id) { + if (config.debug.rhizome_manifest) + DEBUG("Missing 'id' field"); + ret = 0; + } + if (m->version == 0) { + if (config.debug.rhizome_manifest) + DEBUG("Missing 'version' field"); + ret = 0; + } + if (m->filesize == RHIZOME_SIZE_UNSET) { + if (config.debug.rhizome_manifest) + DEBUG("Missing 'filesize' field"); + ret = 0; + } + if (rhizome_filehash_t_is_zero(m->filehash)) { + if (m->filesize > 0) { + if (config.debug.rhizome_manifest) + DEBUG("Missing 'filehash' field"); + ret = 0; + } + } else { + if (m->filesize == 0) { + if (config.debug.rhizome_manifest) + DEBUG("Spurious 'filehash' field"); + ret = 0; + } + } + // warn if expected fields are missing + if (m->service == NULL) { + if (config.debug.rhizome_manifest) + DEBUG("Missing 'service' field"); + ++m->malformed; + } + if (!m->has_date) { + if (config.debug.rhizome_manifest) + DEBUG("Missing 'date' field"); + ++m->malformed; + } + return ret; +} + int rhizome_read_manifest_file(rhizome_manifest *m, const char *filename, size_t bufferP) { if (!m) @@ -686,15 +803,19 @@ int rhizome_read_manifest_file(rhizome_manifest *m, const char *filename, size_t return WHY("Buffer too big"); if (bufferP) { - m->manifest_bytes=bufferP; - memcpy(m->manifestdata, filename, m->manifest_bytes); + m->manifest_all_bytes=bufferP; + memcpy(m->manifestdata, filename, m->manifest_all_bytes); } else { ssize_t bytes = read_whole_file(filename, m->manifestdata, sizeof m->manifestdata); if (bytes == -1) return -1; - m->manifest_bytes = bytes; + m->manifest_all_bytes = (size_t) bytes; + } + switch (rhizome_manifest_parse(m)) { + case 0: return 0; + case -1: return -1; + default: return WHY("Invalid manifest"); } - return rhizome_manifest_parse(m); } int rhizome_hash_file(rhizome_manifest *m, const char *path, rhizome_filehash_t *hash_out, uint64_t *size_out) @@ -804,8 +925,7 @@ rhizome_manifest *_rhizome_new_manifest(struct __sourceloc __whence) if (config.debug.manifests) _log_manifest_trace(__whence, __FUNCTION__); // Set global defaults for a manifest (which are not zero) - m->filesize = RHIZOME_SIZE_UNSET; - m->tail = RHIZOME_SIZE_UNSET; + rhizome_manifest_clear(m); return m; } @@ -829,17 +949,7 @@ void _rhizome_manifest_free(struct __sourceloc __whence, rhizome_manifest *m) ); /* Free variable and signature blocks. */ - unsigned i; - for(i=0;ivar_count;i++) { - free((char *) m->vars[i]); - free((char *) m->values[i]); - m->vars[i] = m->values[i] = NULL; - } - for(i=0;isig_count;i++) { - free(m->signatories[i]); - m->signatories[i] = NULL; - } - + rhizome_manifest_clear(m); if (m->dataFileName) { if (m->dataFileUnlinkOnFree && unlink(m->dataFileName) == -1) WARNF_perror("unlink(%s)", alloca_str_toprint(m->dataFileName)); @@ -856,48 +966,54 @@ void _rhizome_manifest_free(struct __sourceloc __whence, rhizome_manifest *m) return; } -/* Convert variable list to string, complaining if it ends up - too long. - Signatures etc will be added later. */ +/* Convert variable list into manifest text body and compute the hash. Do not sign. + */ int rhizome_manifest_pack_variables(rhizome_manifest *m) { + assert(m->var_count <= NELS(m->vars)); + strbuf sb = strbuf_local((char*)m->manifestdata, sizeof m->manifestdata); unsigned i; - unsigned ofs = 0; - for(i=0;ivar_count;i++) - { - if ((ofs+strlen(m->vars[i])+1+strlen(m->values[i])+1+1)>MAX_MANIFEST_BYTES) - return WHY("Manifest variables too long in total to fit in MAX_MANIFEST_BYTES"); - snprintf((char *)&m->manifestdata[ofs],MAX_MANIFEST_BYTES-ofs,"%s=%s\n", - m->vars[i],m->values[i]); - ofs+=strlen((char *)&m->manifestdata[ofs]); - } - m->manifestdata[ofs++]=0x00; - m->manifest_bytes=ofs; - if (config.debug.rhizome) DEBUG("Repacked variables in manifest."); - m->manifest_all_bytes=ofs; - - /* Recalculate hash */ - crypto_hash_sha512(m->manifesthash,m->manifestdata,m->manifest_bytes); - + for (i = 0; i < m->var_count; ++i) { + strbuf_puts(sb, m->vars[i]); + strbuf_putc(sb, '='); + strbuf_puts(sb, m->values[i]); + strbuf_putc(sb, '\n'); + } + if (strbuf_overrun(sb)) + return WHYF("Manifest overflow: body of %zu bytes exceeds limit of %zu", strbuf_count(sb) + 1, sizeof m->manifestdata); + m->manifest_body_bytes = strbuf_len(sb) + 1; + if (config.debug.rhizome) + DEBUGF("Repacked variables into manifest: %zu bytes", m->manifest_body_bytes); + m->manifest_all_bytes = m->manifest_body_bytes; + m->selfSigned = 0; return 0; } -/* Sign this manifest using our it's own BID secret key. - TODO: need a third-party signing primitive, eg, to support signing with SAS. +/* Sign this manifest using it's own BID secret key. Manifest must not already be signed. + * Manifest body hash must already be computed. */ int rhizome_manifest_selfsign(rhizome_manifest *m) { + assert(m->manifest_body_bytes > 0); + assert(m->manifest_body_bytes <= sizeof m->manifestdata); + assert(m->manifestdata[m->manifest_body_bytes - 1] == '\0'); + assert(m->manifest_body_bytes == m->manifest_all_bytes); // no signature yet if (!m->haveSecret) return WHY("Need private key to sign manifest"); + crypto_hash_sha512(m->manifesthash, m->manifestdata, m->manifest_body_bytes); rhizome_signature sig; if (rhizome_sign_hash(m, &sig) == -1) return WHY("rhizome_sign_hash() failed"); + assert(sig.signatureLength > 0); /* Append signature to end of manifest data */ - if (sig.signatureLength + m->manifest_bytes > MAX_MANIFEST_BYTES) - return WHY("Manifest plus signatures is too long"); - bcopy(&sig.signature[0], &m->manifestdata[m->manifest_bytes], sig.signatureLength); - m->manifest_bytes += sig.signatureLength; - m->manifest_all_bytes = m->manifest_bytes; + if (sig.signatureLength + m->manifest_body_bytes > sizeof m->manifestdata) + return WHYF("Manifest overflow: body %zu + signature %zu bytes exceeds limit of %zu", + m->manifest_body_bytes, + sig.signatureLength, + sizeof m->manifestdata + ); + bcopy(sig.signature, m->manifestdata + m->manifest_body_bytes, sig.signatureLength); + m->manifest_all_bytes = m->manifest_body_bytes + sig.signatureLength; return 0; } diff --git a/rhizome_crypto.c b/rhizome_crypto.c index 3055204f..d2e29102 100644 --- a/rhizome_crypto.c +++ b/rhizome_crypto.c @@ -434,7 +434,7 @@ int rhizome_sign_hash_with_key(rhizome_manifest *m,const unsigned char *sk, int mLen = crypto_hash_sha512_BYTES; int r = crypto_sign_edwards25519sha512batch(signatureBuffer, &sigLen, &hash[0], mLen, sk); if (r) - RETURN(WHY("crypto_sign_edwards25519sha512batch() failed.")); + RETURN(WHY("crypto_sign_edwards25519sha512batch() failed")); /* Here we use knowledge of the internal structure of the signature block to remove the hash, since that is implicitly transported, thus reducing the actual signature size down to 64 bytes. @@ -457,7 +457,7 @@ typedef struct manifest_signature_block_cache { #define SIG_CACHE_SIZE 1024 manifest_signature_block_cache sig_cache[SIG_CACHE_SIZE]; -int rhizome_manifest_lookup_signature_validity(unsigned char *hash,unsigned char *sig,int sig_len) +int rhizome_manifest_lookup_signature_validity(const unsigned char *hash, const unsigned char *sig, int sig_len) { IN(); unsigned int slot=0; @@ -505,73 +505,45 @@ int rhizome_manifest_lookup_signature_validity(unsigned char *hash,unsigned char int rhizome_manifest_extract_signature(rhizome_manifest *m, unsigned *ofs) { IN(); - if (!m) - RETURN(WHY("NULL pointer passed in as manifest")); - if (config.debug.rhizome) - DEBUGF("m->manifest_all_bytes=%u m->manifest_bytes=%u *ofs=%u", m->manifest_all_bytes, m->manifest_bytes, *ofs); - - if ((*ofs) >= m->manifest_all_bytes) - RETURN(0); - + if (config.debug.rhizome_manifest) + DEBUGF("*ofs=%u m->manifest_all_bytes=%zu", *ofs, m->manifest_all_bytes); + assert((*ofs) < m->manifest_all_bytes); + const unsigned char *sig = m->manifestdata + *ofs; uint8_t sigType = m->manifestdata[*ofs]; uint8_t len = (sigType << 2) + 4 + 1; - - /* Each signature type is required to have a different length to detect it. - At present only crypto_sign_edwards25519sha512batch() signatures are - supported. */ - int r; - if (m->sig_countmanifesthash,&m->manifestdata[(*ofs)+1],96); -#ifdef DEPRECATED - unsigned char sigBuf[256]; - unsigned char verifyBuf[256]; - unsigned char publicKey[256]; - bcopy(&m->manifestdata[(*ofs)+1],&sigBuf[0],64); - bcopy(&m->manifesthash[0],&sigBuf[64],crypto_hash_sha512_BYTES); - /* Get public key of signatory */ - bcopy(&m->manifestdata[(*ofs)+1+64],&publicKey[0],crypto_sign_edwards25519sha512batch_PUBLICKEYBYTES); - unsigned long long mlen=0; - int r=crypto_sign_edwards25519sha512batch_open(verifyBuf,&mlen,&sigBuf[0],128, publicKey); -#endif - if (r) { - (*ofs)+=len; - m->errors++; - RETURN(WHY("Error in signature block (verification failed).")); - } else { - /* Signature block passes, so add to list of signatures */ - m->signatureTypes[m->sig_count]=len; - m->signatories[m->sig_count] - =malloc(crypto_sign_edwards25519sha512batch_PUBLICKEYBYTES); - if(!m->signatories[m->sig_count]) { - (*ofs)+=len; - RETURN(WHY("malloc() failed when reading signature block")); - } - bcopy(&m->manifestdata[(*ofs)+1+64],m->signatories[m->sig_count], - crypto_sign_edwards25519sha512batch_PUBLICKEYBYTES); - m->sig_count++; - if (config.debug.rhizome) DEBUG("Signature passed."); - } - break; - default: - (*ofs)+=len; - m->errors++; - RETURN(WHYF("Encountered illegal or malformed signature block (unknown type=0x%02x @ offset 0x%x)",sigType,(*ofs)-len)); - } - else + if (*ofs + len > m->manifest_all_bytes) { + WARNF("Invalid signature at offset %u: type=%#02x gives len=%u that overruns manifest size", + *ofs, sigType, len); + RETURN(1); + } + *ofs += len; + assert (m->sig_count <= NELS(m->signatories)); + if (m->sig_count == NELS(m->signatories)) { + WARN("Too many signature blocks in manifest"); + RETURN(2); + } + switch (sigType) { + case 0x17: // crypto_sign_edwards25519sha512batch() { - (*ofs)+=len; - WHY("Too many signature blocks in manifest."); - m->errors++; + assert(len == 97); + /* Reconstitute signature block */ + int r = rhizome_manifest_lookup_signature_validity(m->manifesthash, sig + 1, 96); + if (r) { + WARN("Signature verification failed"); + RETURN(4); + } + m->signatureTypes[m->sig_count] = len; + if ((m->signatories[m->sig_count] = emalloc(crypto_sign_edwards25519sha512batch_PUBLICKEYBYTES)) == NULL) + RETURN(-1); + bcopy(sig + 1 + 64, m->signatories[m->sig_count], crypto_sign_edwards25519sha512batch_PUBLICKEYBYTES); + m->sig_count++; + if (config.debug.rhizome) + DEBUG("Signature verified"); + RETURN(0); } - - (*ofs)+=len; - RETURN(0); - OUT(); + } + WARNF("Unsupported signature at ofs=%u: type=%#02x", sig - m->manifestdata, sigType); + RETURN(3); } // add value to nonce, with the same result regardless of CPU endian order diff --git a/rhizome_database.c b/rhizome_database.c index 348e1e77..d7a269e1 100644 --- a/rhizome_database.c +++ b/rhizome_database.c @@ -164,31 +164,29 @@ void sqlite_log(void *ignored, int result, const char *msg) WARNF("Sqlite: %d %s", result, msg); } -void verify_bundles(){ +void verify_bundles() +{ // assume that only the manifest itself can be trusted // fetch all manifests and reinsert them. sqlite_retry_state retry = SQLITE_RETRY_STATE_DEFAULT; // This cursor must be ordered descending as re-inserting the manifests will give them a new higher manifest id. // If we didn't, we'd get stuck in an infinite loop. sqlite3_stmt *statement = sqlite_prepare(&retry, "SELECT ROWID, MANIFEST FROM MANIFESTS ORDER BY ROWID DESC;"); - while(sqlite_step_retry(&retry, statement)==SQLITE_ROW){ + while (sqlite_step_retry(&retry, statement) == SQLITE_ROW) { sqlite3_int64 rowid = sqlite3_column_int64(statement, 0); const void *manifest = sqlite3_column_blob(statement, 1); size_t manifest_length = sqlite3_column_bytes(statement, 1); rhizome_manifest *m=rhizome_new_manifest(); - int ret=0; - ret = rhizome_read_manifest_file(m, manifest, manifest_length); - if (ret==0 && m->errors) - ret=-1; - if (ret==0) - ret=rhizome_manifest_verify(m); - if (ret==0){ - m->finalised=1; - m->manifest_bytes=m->manifest_all_bytes; - // store it again, to ensure it is valid and stored correctly with matching file content. - ret=rhizome_store_bundle(m); + int ret = -1; + if ( rhizome_read_manifest_file(m, manifest, manifest_length) == 0 + && rhizome_manifest_validate(m) + && rhizome_manifest_verify(m) + ) { + assert(m->finalised); + // Store it again, to ensure that MANIFESTS columns are up to date. + ret = rhizome_store_bundle(m); } - if (ret!=0){ + if (ret) { if (config.debug.rhizome) DEBUGF("Removing invalid manifest entry @%lld", rowid); sqlite_exec_void_retry(&retry, "DELETE FROM MANIFESTS WHERE ROWID = ?;", INT64, rowid, END); @@ -1386,7 +1384,7 @@ int rhizome_store_bundle(rhizome_manifest *m) "?,?,?,?,?,?,?,?,?,?,?,?,?" ");", RHIZOME_BID_T, &m->cryptoSignPublic, - STATIC_BLOB, m->manifestdata, m->manifest_bytes, + STATIC_BLOB, m->manifestdata, m->manifest_all_bytes, INT64, m->version, INT64, (int64_t) now, STATIC_BLOB, bar, RHIZOME_BAR_BYTES, @@ -1604,7 +1602,9 @@ int rhizome_list_next(sqlite_retry_state *retry, struct rhizome_list_cursor *c) rhizome_manifest *m = c->manifest = rhizome_new_manifest(); if (m == NULL) RETURN(-1); - if (rhizome_read_manifest_file(m, manifestblob, manifestblobsize) == -1) { + if ( rhizome_read_manifest_file(m, manifestblob, manifestblobsize) == -1 + || !rhizome_manifest_validate(m) + ) { WHYF("MANIFESTS row id=%s has invalid manifest blob -- skipped", q_manifestid); continue; } @@ -1710,7 +1710,6 @@ int rhizome_find_duplicate(const rhizome_manifest *m, rhizome_manifest **found) strbuf_puts(b, " AND recipient = ?"); if (strbuf_overrun(b)) return WHYF("SQL command too long: %s", strbuf_str(b)); - int ret = 0; sqlite_retry_state retry = SQLITE_RETRY_STATE_DEFAULT; sqlite3_stmt *statement = sqlite_prepare_bind(&retry, strbuf_str(b), INT64, m->filesize, STATIC_TEXT, m->service, END); @@ -1739,11 +1738,13 @@ int rhizome_find_duplicate(const rhizome_manifest *m, rhizome_manifest **found) const unsigned char *q_manifestid = sqlite3_column_text(statement, 0); const char *manifestblob = (char *) sqlite3_column_blob(statement, 1); size_t manifestblobsize = sqlite3_column_bytes(statement, 1); // must call after sqlite3_column_blob() - if (rhizome_read_manifest_file(blob_m, manifestblob, manifestblobsize) == -1) { + if ( rhizome_read_manifest_file(blob_m, manifestblob, manifestblobsize) == -1 + || !rhizome_manifest_validate(blob_m) + ) { WARNF("MANIFESTS row id=%s has invalid manifest blob -- skipped", q_manifestid); goto next; } - if (rhizome_manifest_verify(blob_m)) { + if (!rhizome_manifest_verify(blob_m)) { WARNF("MANIFESTS row id=%s fails verification -- skipped", q_manifestid); goto next; } @@ -1781,8 +1782,8 @@ static int unpack_manifest_row(rhizome_manifest *m, sqlite3_stmt *statement) const char *q_author = (const char *) sqlite3_column_text(statement, 4); size_t q_blobsize = sqlite3_column_bytes(statement, 1); // must call after sqlite3_column_blob() uint64_t q_rowid = sqlite3_column_int64(statement, 5); - if (rhizome_read_manifest_file(m, q_blob, q_blobsize) == -1) - return WHYF("Manifest %s exists but is invalid", q_id); + if (rhizome_read_manifest_file(m, q_blob, q_blobsize) == -1 || !rhizome_manifest_validate(m)) + return WHYF("Manifest bid=%s in database but invalid", q_id); if (q_author) { sid_t author; if (str_to_sid_t(&author, q_author) == -1) diff --git a/rhizome_direct.c b/rhizome_direct.c index 02d91833..7d982195 100644 --- a/rhizome_direct.c +++ b/rhizome_direct.c @@ -442,11 +442,12 @@ rhizome_manifest *rhizome_direct_get_manifest(unsigned char *bid_prefix,int pref if (!manifestblob) goto error; rhizome_manifest *m=rhizome_new_manifest(); - if (rhizome_read_manifest_file(m,manifestblob,manifestblobsize)==-1) - { - rhizome_manifest_free(m); - goto error; - } + if ( rhizome_read_manifest_file(m,manifestblob,manifestblobsize)==-1 + || !rhizome_manifest_validate(m) + ) { + rhizome_manifest_free(m); + goto error; + } DEBUGF("Read manifest"); sqlite3_blob_close(blob); diff --git a/rhizome_direct_http.c b/rhizome_direct_http.c index 400ab40a..b84a5430 100644 --- a/rhizome_direct_http.c +++ b/rhizome_direct_http.c @@ -257,7 +257,7 @@ static int rhizome_direct_addfile_end(struct http_request *hr) if (config.debug.rhizome) DEBUGF("Import sans-manifest appeared to succeed"); /* Respond with the manifest that was added. */ - http_request_response_static(&r->http, 200, "text/plain", (const char *)m->manifestdata, m->manifest_bytes); + http_request_response_static(&r->http, 200, "text/plain", (const char *)m->manifestdata, m->manifest_all_bytes); /* clean up after ourselves */ if (mout && mout != m) rhizome_manifest_free(mout); @@ -658,7 +658,7 @@ void rhizome_direct_http_dispatch(rhizome_direct_sync_request *r) "\r\n"; /* Work out what the content length should be */ if (config.debug.rhizome_tx) - DEBUGF("manifest_all_bytes=%u, manifest_bytes=%u", m->manifest_all_bytes, m->manifest_bytes); + DEBUGF("manifest_all_bytes=%u, manifest_body_bytes=%u", m->manifest_all_bytes, m->manifest_body_bytes); assert(m->filesize != RHIZOME_SIZE_UNSET); size_t content_length = strlen(template2) - 2 /* minus 2 for the "%s" that gets replaced */ diff --git a/rhizome_fetch.c b/rhizome_fetch.c index 7f1d5d74..f01d080e 100644 --- a/rhizome_fetch.c +++ b/rhizome_fetch.c @@ -464,10 +464,9 @@ int rhizome_queue_ignore_manifest(unsigned char *bid_prefix, int prefix_len, int static int rhizome_import_received_bundle(struct rhizome_manifest *m) { m->finalised = 1; - m->manifest_bytes = m->manifest_all_bytes; // store the signatures too if (config.debug.rhizome_rx) { DEBUGF("manifest len=%u has %u signatories. Associated filesize=%"PRIu64" bytes", - m->manifest_bytes, m->sig_count, m->filesize); + m->manifest_all_bytes, m->sig_count, m->filesize); dump("manifest", m->manifestdata, m->manifest_all_bytes); } return rhizome_add_manifest(m, m->ttl - 1 /* TTL */); @@ -887,7 +886,7 @@ int rhizome_suggest_queue_manifest_import(rhizome_manifest *m, const struct sock assert(m->filesize != RHIZOME_SIZE_UNSET); if (m->filesize == 0) { - if (rhizome_manifest_verify(m) != 0) { + if (!rhizome_manifest_verify(m)) { WHY("Error verifying manifest when considering for import"); /* Don't waste time looking at this manifest again for a while */ rhizome_queue_ignore_manifest(m->cryptoSignPublic.binary, sizeof m->cryptoSignPublic.binary, 60000); @@ -923,7 +922,7 @@ int rhizome_suggest_queue_manifest_import(rhizome_manifest *m, const struct sock rhizome_manifest_free(m); RETURN(0); } - if (!m->selfSigned && rhizome_manifest_verify(m)) { + if (!m->selfSigned && !rhizome_manifest_verify(m)) { WHY("Error verifying manifest when considering queuing for import"); /* Don't waste time looking at this manifest again for a while */ rhizome_queue_ignore_manifest(m->cryptoSignPublic.binary, sizeof m->cryptoSignPublic.binary, 60000); @@ -949,7 +948,7 @@ int rhizome_suggest_queue_manifest_import(rhizome_manifest *m, const struct sock RETURN(1); } - if (!m->selfSigned && rhizome_manifest_verify(m)) { + if (!m->selfSigned && !rhizome_manifest_verify(m)) { WHY("Error verifying manifest when considering queuing for import"); /* Don't waste time looking at this manifest again for a while */ rhizome_queue_ignore_manifest(m->cryptoSignPublic.binary, sizeof m->cryptoSignPublic.binary, 60000); @@ -1306,7 +1305,9 @@ int rhizome_write_complete(struct rhizome_fetch_slot *slot) call schedule queued items. */ rhizome_manifest *m = rhizome_new_manifest(); if (m) { - if (rhizome_read_manifest_file(m, slot->manifest_buffer, (size_t)slot->manifest_bytes) == -1) { + if ( rhizome_read_manifest_file(m, slot->manifest_buffer, (size_t)slot->manifest_bytes) == -1 + || !rhizome_manifest_validate(m) + ) { DEBUGF("Couldn't read manifest"); rhizome_manifest_free(m); } else { diff --git a/rhizome_packetformats.c b/rhizome_packetformats.c index 01af7c16..461959f1 100644 --- a/rhizome_packetformats.c +++ b/rhizome_packetformats.c @@ -294,7 +294,6 @@ int overlay_rhizome_saw_advertisements(int i, struct decode_context *context, st int ad_frame_type=ob_get(f->payload); struct sockaddr_in httpaddr = context->addr; httpaddr.sin_port = htons(RHIZOME_HTTP_PORT); - size_t manifest_length; rhizome_manifest *m=NULL; int (*oldfunc)() = sqlite_set_tracefunc(is_debug_rhizome_ads); @@ -305,73 +304,66 @@ int overlay_rhizome_saw_advertisements(int i, struct decode_context *context, st if (ad_frame_type & HAS_MANIFESTS){ /* Extract whole manifests */ - while(f->payload->position < f->payload->sizeLimit) { - if (ob_getbyte(f->payload, f->payload->position)==0xff){ - f->payload->position++; + while (ob_remaining(f->payload) > 0) { + if (ob_peek(f->payload) == 0xff) { + ob_skip(f->payload, 1); break; } - - manifest_length = ob_get_ui16(f->payload); + + size_t manifest_length = ob_get_ui16(f->payload); if (manifest_length==0) continue; unsigned char *data = ob_get_bytes_ptr(f->payload, manifest_length); if (!data) { WHYF("Illegal manifest length field in rhizome advertisement frame %zu vs %d", - manifest_length, f->payload->sizeLimit - f->payload->position); + manifest_length, ob_remaining(f->payload)); break; } - /* Read manifest without verifying signatures (which would waste lots of - energy, everytime we see a manifest that we already have). - In fact, it would be better here to do a really rough and ready parser - to get the id and version fields out, and avoid the memory copies that - otherwise happen. - But we do need to make sure that at least one signature is there. - */ - m = rhizome_new_manifest(); - if (!m) { - WHY("Out of manifests"); + // Briefly inspect the manifest to see if it looks interesting. + struct rhizome_manifest_summary summ; + if (!rhizome_manifest_inspect((char *)data, manifest_length, &summ)) { + if (config.debug.rhizome_ads) + DEBUG("Ignoring manifest that looks malformed"); goto next; } - if (rhizome_read_manifest_file(m, (char *)data, manifest_length) == -1) { - WHY("Error parsing manifest body"); - goto next; - } - - /* trim manifest ID to a prefix for ease of debugging - (that is the only use of this */ - if (config.debug.rhizome_ads){ - DEBUGF("manifest id=%s version=%"PRId64, alloca_tohex_rhizome_bid_t(m->cryptoSignPublic), m->version); - } + if (config.debug.rhizome_ads) + DEBUGF("manifest id=%s version=%"PRId64, alloca_tohex_rhizome_bid_t(summ.bid), summ.version); - /* Crude signature presence test */ - if (m->manifest_bytes >= m->manifest_all_bytes){ - // no signature was found when parsing? - /* ignore the announcement, but don't ignore other people - offering the same manifest */ + // If it looks like there is no signature at all, ignore the announcement but don't brown-list + // the manifest ID, so that we will still process other offers of the same manifest with + // signatures. + if (summ.body_len == manifest_length) { if (config.debug.rhizome_ads) DEBUG("Ignoring manifest announcment with no signature"); goto next; } - if (rhizome_ignore_manifest_check(m->cryptoSignPublic.binary, sizeof m->cryptoSignPublic.binary)){ + if (rhizome_ignore_manifest_check(summ.bid.binary, sizeof summ.bid.binary)){ /* Ignoring manifest that has caused us problems recently */ if (config.debug.rhizome_ads) - DEBUGF("Ignoring manifest with errors: %s", alloca_tohex_rhizome_bid_t(m->cryptoSignPublic)); + DEBUGF("Ignoring manifest with errors bid=%s", alloca_tohex_rhizome_bid_t(summ.bid)); goto next; } - if (m->errors > 0){ - if (config.debug.rhizome_ads) - DEBUG("Unverified manifest has errors - so not processing any further."); - /* Don't waste any time on this manifest in future attempts for at least - a minute. */ + // The manifest looks potentially interesting, so now do a full parse and validation. + if ((m = rhizome_new_manifest()) == NULL) + goto next; + if ( rhizome_read_manifest_file(m, (char *)data, manifest_length) == -1 + || !rhizome_manifest_validate(m) + ) { + WARN("Malformed manifest"); + // Don't attend to this manifest for at least a minute rhizome_queue_ignore_manifest(m->cryptoSignPublic.binary, sizeof m->cryptoSignPublic.binary, 60000); goto next; } - /* Manifest is okay, so see if it is worth storing */ - + assert(m->has_id); + assert(m->version != 0); + assert(cmp_rhizome_bid_t(&m->cryptoSignPublic, &summ.bid) == 0); + assert(m->version == summ.version); + assert(m->manifest_body_bytes == summ.body_len); + // are we already fetching this bundle [or later]? rhizome_manifest *mf=rhizome_fetch_search(m->cryptoSignPublic.binary, sizeof m->cryptoSignPublic.binary); if (mf && mf->version >= m->version) diff --git a/tests/rhizomeops b/tests/rhizomeops index 615357ff..2c464bd5 100755 --- a/tests/rhizomeops +++ b/tests/rhizomeops @@ -545,10 +545,9 @@ setup_AddDeDuplicate() { assert_rhizome_list --fromhere=1 --author=$SIDB1 file1 file2 } test_AddDeDuplicate() { - # Add first file again - nothing should change in its manifests, and it - # should appear that the add command succeeded (with perhaps some grumbling - # on stderr). - execute --exit-status=2 $servald rhizome add file $SIDB1 file1 file1.manifestA + # Add first file again - should return a "duplicate" status code and nothing + # should change in its manifests. + execute --exit-status=2 --stderr $servald rhizome add file $SIDB1 file1 file1.manifestA assert [ -s file1.manifestA ] assert_stdout_add_file file1 extract_stdout_secret file1_dup_secret @@ -558,7 +557,7 @@ test_AddDeDuplicate() { assert diff file1.manifest file1.manifestA assert [ $file1_secret = $file1_dup_secret ] # Repeat for second file. - execute --exit-status=2 $servald rhizome add file $SIDB1 file2 file2.manifestA + execute --exit-status=2 --stderr $servald rhizome add file $SIDB1 file2 file2.manifestA assert [ -s file2.manifestA ] assert_stdout_add_file file2 extract_stdout_secret file2_dup_secret diff --git a/tests/rhizomeprotocol b/tests/rhizomeprotocol index ff9a58f3..497d8c8d 100755 --- a/tests/rhizomeprotocol +++ b/tests/rhizomeprotocol @@ -43,6 +43,8 @@ configure_servald_server() { set debug.rhizome on \ set debug.httpd on \ set debug.rhizome_httpd on \ + set debug.rhizome_manifest on \ + set debug.rhizome_ads on \ set debug.rhizome_tx on \ set debug.rhizome_rx on \ set server.respawn_on_crash off \