From 49aec4d33149fe8202fd781220420e5c66e81c5e Mon Sep 17 00:00:00 2001 From: Andrew Bettison Date: Fri, 25 May 2012 15:38:13 +0930 Subject: [PATCH] Improve rhizome manifest debugging Move rhizome_new_manifest() out of rhizome_read_manifest_file() so that the out-of-manifest report shows the names of the functions where the manifests were really allocated. --- commandline.c | 14 +-- rhizome.c | 8 +- rhizome.h | 2 +- rhizome_bundle.c | 33 +++---- rhizome_database.c | 206 ++++++++++++++++++++++------------------ rhizome_packetformats.c | 24 +++-- 6 files changed, 160 insertions(+), 127 deletions(-) diff --git a/commandline.c b/commandline.c index 15ebad18..71ca2468 100644 --- a/commandline.c +++ b/commandline.c @@ -1146,19 +1146,19 @@ int app_rhizome_add_file(int argc, const char *const *argv, struct command_line_ return -1; /* Create a new manifest that will represent the file. If a manifest file was supplied, then read * it, otherwise create a blank manifest. */ - rhizome_manifest *m = NULL; int manifest_file_supplied = 0; + rhizome_manifest *m = rhizome_new_manifest(); + if (!m) + return WHY("Manifest struct could not be allocated -- not added to rhizome"); if (manifestpath[0] && access(manifestpath, R_OK) == 0) { if (debug & DEBUG_RHIZOME) DEBUGF("reading manifest from %s", manifestpath); - m = rhizome_read_manifest_file(manifestpath, 0, 0); // no verify - if (!m) + if (rhizome_read_manifest_file(m, manifestpath, 0, 0) == -1) { // no verify + rhizome_manifest_free(m); return WHY("Manifest file could not be loaded -- not added to rhizome"); + } manifest_file_supplied = 1; } else { - if (debug & DEBUG_RHIZOME) DEBUGF("manifest file %s does not exist", manifestpath); - m = rhizome_new_manifest(); - if (!m) - return WHY("Manifest struct could not be allocated -- not added to rhizome"); + if (debug & DEBUG_RHIZOME) DEBUGF("manifest file %s does not exist -- creating new manifest", manifestpath); } /* Fill in a few missing manifest fields, to make it easier to use when adding new files: - the default service is FILE diff --git a/rhizome.c b/rhizome.c index c5c41130..3d8d4e35 100644 --- a/rhizome.c +++ b/rhizome.c @@ -51,10 +51,14 @@ int rhizome_bundle_import(rhizome_manifest *m_in, rhizome_manifest **m_out, /* Read manifest file if no manifest was given */ rhizome_manifest *m = m_in; - if (!m_in) { - m = rhizome_read_manifest_file(manifestname, 0 /* file not buffer */, RHIZOME_VERIFY); + if (!m_in) { + m = rhizome_new_manifest(); if (!m) + return WHY("Out of manifests."); + if (rhizome_read_manifest_file(m, manifestname, 0 /* file not buffer */, RHIZOME_VERIFY) == -1) { + rhizome_manifest_free(m); return WHY("Could not read manifest file."); + } } else { if (debug&DEBUG_RHIZOMESYNC) DEBUGF("Importing direct from manifest structure fileHashedP=%d", m->fileHashedP); diff --git a/rhizome.h b/rhizome.h index dbf345d1..8573b730 100644 --- a/rhizome.h +++ b/rhizome.h @@ -199,7 +199,7 @@ int rhizome_write_manifest_file(rhizome_manifest *m, const char *filename); int rhizome_manifest_sign(rhizome_manifest *m,const char *authoring_sid); int rhizome_drop_stored_file(char *id,int maximum_priority); int rhizome_manifest_priority(char *id); -rhizome_manifest *rhizome_read_manifest_file(const char *filename, int bufferPAndSize, int flags); +int rhizome_read_manifest_file(rhizome_manifest *m, const char *filename, int bufferPAndSize, int flags); int rhizome_hash_file(const char *filename,char *hash_out); char *rhizome_manifest_get(const rhizome_manifest *m, const char *var, char *out, int maxlen); long long rhizome_manifest_get_ll(rhizome_manifest *m, const char *var); diff --git a/rhizome_bundle.c b/rhizome_bundle.c index dbc8f25d..a165b7ed 100644 --- a/rhizome_bundle.c +++ b/rhizome_bundle.c @@ -21,25 +21,26 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. #include "rhizome.h" #include -rhizome_manifest *rhizome_read_manifest_file(const char *filename, int bufferP, int flags) +int rhizome_read_manifest_file(rhizome_manifest *m, const char *filename, int bufferP, int flags) { - if (bufferP>MAX_MANIFEST_BYTES) return NULL; - - rhizome_manifest *m = rhizome_new_manifest(); - if (!m) return NULL; + if (bufferP>MAX_MANIFEST_BYTES) return WHY("Buffer too big"); + if (!m) return WHY("Null manifest"); if (bufferP) { m->manifest_bytes=bufferP; memcpy(m->manifestdata, filename, m->manifest_bytes); } else { - FILE *f=fopen(filename,"r"); - if (!f) { - WHYF("Could not open manifest file %s for reading.", filename); - rhizome_manifest_free(m); - return NULL; - } - m->manifest_bytes = fread(m->manifestdata,1,MAX_MANIFEST_BYTES,f); - fclose(f); + FILE *f = fopen(filename, "r"); + if (f == NULL) + return WHYF("Could not open manifest file %s for reading.", filename); + m->manifest_bytes = fread(m->manifestdata, 1, MAX_MANIFEST_BYTES, f); + int ret = 0; + if (m->manifest_bytes == -1) + ret = WHY_perror("fread"); + if (fclose(f) == EOF) + ret = WHY_perror("fclose"); + if (ret == -1) + return -1; } m->manifest_all_bytes=m->manifest_bytes; @@ -148,7 +149,7 @@ rhizome_manifest *rhizome_read_manifest_file(const char *filename, int bufferP, m->manifest_bytes=end_of_text; - return m; + return 0; } int rhizome_strn_is_file_hash(const char *text) @@ -309,9 +310,9 @@ rhizome_manifest *_rhizome_new_manifest(const char *filename, const char *funcna { int i; logMessage(LOG_LEVEL_ERROR, filename, line, funcname, "%s(): no free manifest records, this probably indicates a memory leak", __FUNCTION__); - WHYF(" Manifest Slot# | Last allocated by"); + WHYF(" Slot# | Last allocated by"); for(i=0;ifileHexHash)) { - WARNF("MANIFESTS row id=%s joined to FILES row id=%s has inconsistent blob: blob.filehash=%s -- skipped", - q_manifestid, m->fileHexHash, blob_filehash); - ++inconsistent; - } - if (blob_filesize != -1 && blob_filesize != m->fileLength) { - WARNF("MANIFESTS row id=%s joined to FILES row id=%s has inconsistent blob: known file size %lld, blob.filesize=%lld -- skipped", - q_manifestid, m->fileLength, blob_filesize); - ++inconsistent; - } - if (m->version != -1 && q_version != m->version) { - WARNF("SELECT query with version=%lld returned incorrect row: manifests.version=%lld -- skipped", m->version, q_version); - ++inconsistent; - } - if (blob_service == NULL) { - WARNF("MANIFESTS row id=%s has blob with no 'service' -- skipped", q_manifestid, blob_id); - ++inconsistent; - } - if (!inconsistent) { - strbuf b = strbuf_alloca(1024); - if (strcasecmp(service, RHIZOME_SERVICE_FILE) == 0) { - const char *blob_name = rhizome_manifest_get(blob_m, "name", NULL, 0); - if (blob_name && !strcmp(blob_name, name)) { - if (debug & DEBUG_RHIZOME) - strbuf_sprintf(b, " name=\"%s\"", blob_name); - ret = 1; + if (rhizome_read_manifest_file(blob_m, manifestblob, manifestblobsize, 0) == -1) { + WARNF("MANIFESTS row id=%s has invalid manifest blob -- skipped", q_manifestid); + } else { + const char *blob_service = rhizome_manifest_get(blob_m, "service", NULL, 0); + const char *blob_id = rhizome_manifest_get(blob_m, "id", NULL, 0); + long long blob_version = rhizome_manifest_get_ll(blob_m, "version"); + const char *blob_filehash = rhizome_manifest_get(blob_m, "filehash", NULL, 0); + long long blob_filesize = rhizome_manifest_get_ll(blob_m, "filesize"); + if (debug & DEBUG_RHIZOME) + DEBUGF("Consider manifest.service=%s manifest.id=%s manifest.version=%lld", blob_service, q_manifestid, blob_version); + /* Perform consistency checks, because we're paranoid. */ + int inconsistent = 0; + if (blob_id && strcasecmp(blob_id, q_manifestid)) { + WARNF("MANIFESTS row id=%s has inconsistent blob with id=%s -- skipped", q_manifestid, blob_id); + ++inconsistent; + } + if (blob_version != -1 && blob_version != q_version) { + WARNF("MANIFESTS row id=%s has inconsistent blob: manifests.version=%lld, blob.version=%lld -- skipped", + q_manifestid, q_version, blob_version); + ++inconsistent; + } + if (!blob_filehash && strcasecmp(blob_filehash, m->fileHexHash)) { + WARNF("MANIFESTS row id=%s joined to FILES row id=%s has inconsistent blob: blob.filehash=%s -- skipped", + q_manifestid, m->fileHexHash, blob_filehash); + ++inconsistent; + } + if (blob_filesize != -1 && blob_filesize != m->fileLength) { + WARNF("MANIFESTS row id=%s joined to FILES row id=%s has inconsistent blob: known file size %lld, blob.filesize=%lld -- skipped", + q_manifestid, m->fileLength, blob_filesize); + ++inconsistent; + } + if (m->version != -1 && q_version != m->version) { + WARNF("SELECT query with version=%lld returned incorrect row: manifests.version=%lld -- skipped", m->version, q_version); + ++inconsistent; + } + if (blob_service == NULL) { + WARNF("MANIFESTS row id=%s has blob with no 'service' -- skipped", q_manifestid, blob_id); + ++inconsistent; + } + if (!inconsistent) { + strbuf b = strbuf_alloca(1024); + if (strcasecmp(service, RHIZOME_SERVICE_FILE) == 0) { + const char *blob_name = rhizome_manifest_get(blob_m, "name", NULL, 0); + if (blob_name && !strcmp(blob_name, name)) { + if (debug & DEBUG_RHIZOME) + strbuf_sprintf(b, " name=\"%s\"", blob_name); + ret = 1; + } + } else if (strcasecmp(service, RHIZOME_SERVICE_FILE) == 0) { + const char *blob_sender = rhizome_manifest_get(blob_m, "sender", NULL, 0); + const char *blob_recipient = rhizome_manifest_get(blob_m, "recipient", NULL, 0); + if (blob_sender && !strcasecmp(blob_sender, sender) && blob_recipient && !strcasecmp(blob_recipient, recipient)) { + if (debug & DEBUG_RHIZOME) + strbuf_sprintf(b, " sender=%s recipient=%s", blob_sender, blob_recipient); + ret = 1; + } } - } else if (strcasecmp(service, RHIZOME_SERVICE_FILE) == 0) { - const char *blob_sender = rhizome_manifest_get(blob_m, "sender", NULL, 0); - const char *blob_recipient = rhizome_manifest_get(blob_m, "recipient", NULL, 0); - if (blob_sender && !strcasecmp(blob_sender, sender) && blob_recipient && !strcasecmp(blob_recipient, recipient)) { - if (debug & DEBUG_RHIZOME) - strbuf_sprintf(b, " sender=%s recipient=%s", blob_sender, blob_recipient); - ret = 1; + if (ret == 1) { + rhizome_hex_to_bytes(q_manifestid, blob_m->cryptoSignPublic, crypto_sign_edwards25519sha512batch_PUBLICKEYBYTES*2); + memcpy(blob_m->fileHexHash, m->fileHexHash, RHIZOME_FILEHASH_STRLEN + 1); + blob_m->fileHashedP = 1; + blob_m->fileLength = m->fileLength; + blob_m->version = q_version; + *found = blob_m; + DEBUGF("Found duplicate payload: service=%s%s version=%llu hexhash=%s", + blob_service, strbuf_str(b), blob_m->version, blob_m->fileHexHash + ); + break; } } - if (ret == 1) { - rhizome_hex_to_bytes(q_manifestid, blob_m->cryptoSignPublic, crypto_sign_edwards25519sha512batch_PUBLICKEYBYTES*2); - memcpy(blob_m->fileHexHash, m->fileHexHash, RHIZOME_FILEHASH_STRLEN + 1); - blob_m->fileHashedP = 1; - blob_m->fileLength = m->fileLength; - blob_m->version = q_version; - *found = blob_m; - DEBUGF("Found duplicate payload: service=%s%s version=%llu hexhash=%s", - blob_service, strbuf_str(b), blob_m->version, blob_m->fileHexHash - ); - break; - } } - rhizome_manifest_free(blob_m); + if (blob_m) rhizome_manifest_free(blob_m); } } sqlite3_finalize(statement); @@ -1030,11 +1047,16 @@ int rhizome_retrieve_manifest(const char *manifestid, rhizome_manifest **mp) ret = WHY("Incorrect statement column"); break; } + const char *q_manifestid = (const char *) 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 (mp) { - m = rhizome_read_manifest_file(manifestblob, manifestblobsize, 0); + m = rhizome_new_manifest(); if (m == NULL) { + WARNF("MANIFESTS row id=%s has invalid manifest blob -- skipped", q_manifestid); + ret = WHY("Out of manifests"); + } else if (rhizome_read_manifest_file(m, manifestblob, manifestblobsize, 0) == -1) { + WARNF("MANIFESTS row id=%s has invalid manifest blob -- skipped", q_manifestid); ret = WHY("Invalid manifest blob from database"); } else { ret = 1; @@ -1063,7 +1085,7 @@ int rhizome_retrieve_manifest(const char *manifestid, rhizome_manifest **mp) cli_puts("service"); cli_delim(":"); cli_puts(blob_service); cli_delim("\n"); cli_puts("manifestid"); cli_delim(":"); - cli_puts((const char *)sqlite3_column_text(statement, 0)); cli_delim("\n"); + cli_puts(q_manifestid); cli_delim("\n"); cli_puts("version"); cli_delim(":"); cli_printf("%lld", (long long) sqlite3_column_int64(statement, 2)); cli_delim("\n"); cli_puts("inserttime"); cli_delim(":"); diff --git a/rhizome_packetformats.c b/rhizome_packetformats.c index 90a0f5c5..33eda1d7 100644 --- a/rhizome_packetformats.c +++ b/rhizome_packetformats.c @@ -341,12 +341,16 @@ int overlay_rhizome_saw_advertisements(int i,overlay_frame *f, long long now) otherwise happen. But we do need to make sure that at least one signature is there. */ - m=rhizome_read_manifest_file((char *)&f->payload->bytes[ofs], - manifest_length,RHIZOME_DONTVERIFY); + m = rhizome_new_manifest(); if (!m) { WHY("Out of manifests"); return 0; } + if (rhizome_read_manifest_file(m, (char *)&f->payload->bytes[ofs], manifest_length, RHIZOME_DONTVERIFY) == -1) { + WHY("Error importing manifest body"); + rhizome_manifest_free(m); + return 0; + } /* Crude signature presence test */ for(i=m->manifest_all_bytes-1;i>0;i--) if (!m->manifestdata[i]) { @@ -357,7 +361,7 @@ int overlay_rhizome_saw_advertisements(int i,overlay_frame *f, long long now) /* ignore the announcement, but don't ignore other people offering the same manifest */ WARN("Ignoring manifest announcment with no signature"); - rhizome_manifest_free(m); + rhizome_manifest_free(m); return 0; } int importManifest=0; @@ -404,23 +408,25 @@ int overlay_rhizome_saw_advertisements(int i,overlay_frame *f, long long now) /* Okay, so the manifest looks like it is potentially interesting to us, i.e., we don't already have it or a later version of it. Now reread the manifest, this time verifying signatures */ - m=rhizome_read_manifest_file((char *)&f->payload->bytes[ofs], - manifest_length,RHIZOME_VERIFY); - if (m->errors) { + if ((m = rhizome_new_manifest()) == NULL) + WHY("Out of manifests"); + else if (rhizome_read_manifest_file(m, (char *)&f->payload->bytes[ofs], manifest_length, RHIZOME_VERIFY) == -1) { + WHY("Error importing manifest body"); + rhizome_manifest_free(m); + m = NULL; + } else if (m->errors) { if (debug&DEBUG_RHIZOME) DEBUGF("Verifying manifest %s revealed errors -- not storing.", manifest_id); rhizome_queue_ignore_manifest(m,(struct sockaddr_in*)f->recvaddr,60000); rhizome_manifest_free(m); + m = NULL; } else { if (debug&DEBUG_RHIZOME) DEBUGF("Verifying manifest %s revealed no errors -- will try to store.", manifest_id); - /* Add manifest to import queue. We need to know originating IPv4 address so that we can transfer by HTTP. */ if (0) DEBUG("Suggesting fetching of a bundle"); rhizome_suggest_queue_manifest_import(m,(struct sockaddr_in *)f->recvaddr); } } - else - rhizome_manifest_free(m); if (!manifest_length) { WHY("Infinite loop in packet decoding");