From 738b70b5138eeb3feaaf7047b7c83b9f0c087ca5 Mon Sep 17 00:00:00 2001 From: Andrew Bettison Date: Tue, 13 Mar 2012 18:31:29 +1030 Subject: [PATCH] Test and fix ACTION_CREATEHLR idempotency code: - refactor hlrSid() to not return pointer to static buffer, take 3rd arg instead - introduce SID_STRLEN macro constant, use it everywhere - reformat some code for readability --- client.c | 6 +++--- dataformats.c | 6 ++++-- export.c | 4 ++-- hlrdata.c | 17 +++++++++-------- overlay_abbreviations.c | 2 +- overlay_route.c | 8 +++----- rhizome.c | 15 ++++++++++----- serval.h | 8 +++++--- server.c | 24 ++++++++++++++++-------- 9 files changed, 53 insertions(+), 37 deletions(-) diff --git a/client.c b/client.c index ab8d7956..a39f71c5 100644 --- a/client.c +++ b/client.c @@ -246,7 +246,7 @@ int requestNewHLR(char *did,char *pin,char *sid, break; case ACTION_OKAY: { - char sid[128]; + char sid[SID_STRLEN+1]; int ofs=0; extractSid(&responses.responses->sid[0],&ofs,&sid[0]); printf("OK:%s\n",sid); @@ -453,7 +453,7 @@ int writeItem(char *sid,int var_id,int instance,unsigned char *value, while(r) { int slen=0; - char sid[SID_SIZE*2+1]; + char sid[SID_STRLEN+1]; extractSid(r->sid,&slen,sid); switch(r->code) { @@ -626,7 +626,7 @@ int requestItem(char *did,char *sid,char *item,int instance, r=responses.responses; while(r) { - char sid[SID_SIZE*2+1]; + char sid[SID_STRLEN+1]; int slen=0; extractSid(r->sid,&slen,sid); switch(r->code) diff --git a/dataformats.c b/dataformats.c index 976e9758..68bdb43c 100644 --- a/dataformats.c +++ b/dataformats.c @@ -99,7 +99,7 @@ int extractSid(unsigned char *packet,int *ofs,char *sid) sid[d++]=hexdigit[packet[*ofs]&0xf]; (*ofs)++; } - sid[64]=0; + sid[d]=0; return 0; } @@ -107,7 +107,9 @@ int stowSid(unsigned char *packet,int ofs,char *sid) { int i; if (debug&DEBUG_PACKETFORMATS) printf("Stowing SID \"%s\"\n",sid); - if (strlen(sid)!=64) return setReason("Asked to stow invalid SID (should be 64 hex digits)"); + size_t n = strlen(sid); + if (n != SID_STRLEN) + return setReason("Asked to stow invalid SID (strlen is %u but should be %u hex digits)", n, SID_STRLEN); for(i=0;i1) index=(index<<8)|index_bytes[1]; diff --git a/overlay_route.c b/overlay_route.c index d6a5678a..eed135ff 100644 --- a/overlay_route.c +++ b/overlay_route.c @@ -911,13 +911,12 @@ int overlay_route_recalc_neighbour_metrics(overlay_neighbour *n,long long now) } -char ors_out[SID_SIZE*2+1]; +char ors_out[SID_STRLEN+1]; char *overlay_render_sid(unsigned char *sid) { int zero=0; - extractSid(sid,&zero,ors_out); - ors_out[SID_SIZE*2]=0; + ors_out[SID_STRLEN] = '\0'; return ors_out; } @@ -926,8 +925,7 @@ char *overlay_render_sid_prefix(unsigned char *sid,int l) int zero=0; if (l<0) l=0; - if (l>(SID_SIZE*2)) l=SID_SIZE*2; - + if (l>SID_STRLEN) l=SID_STRLEN; extractSid(sid,&zero,ors_out); ors_out[l]=0; return ors_out; diff --git a/rhizome.c b/rhizome.c index df78f5aa..c58e3865 100644 --- a/rhizome.c +++ b/rhizome.c @@ -71,9 +71,11 @@ int rhizome_bundle_import(rhizome_manifest *m_in,char *bundle,char *groups[], in } if (checkFileP||signP) { - if (rhizome_hash_file(filename,hexhash)) - { rhizome_manifest_free(m); return WHY("Could not hash file."); } - bcopy(&hexhash[0],&m->fileHexHash[0],SHA512_DIGEST_STRING_LENGTH); + if (rhizome_hash_file(filename,hexhash)) { + rhizome_manifest_free(m); + return WHY("Could not hash file."); + } + memcpy(&m->fileHexHash[0],&hexhash[0],SHA512_DIGEST_STRING_LENGTH); m->fileHashedP=1; } @@ -85,8 +87,11 @@ int rhizome_bundle_import(rhizome_manifest *m_in,char *bundle,char *groups[], in char *mhexhash; if (checkFileP) { if ((mhexhash=rhizome_manifest_get(m,"filehash",NULL,0))!=NULL) - if (strcmp(hexhash,mhexhash)) verifyErrors++; } - if (m->errors) verifyErrors+=m->errors; + if (strcmp(hexhash,mhexhash)) + verifyErrors++; + } + if (m->errors) + verifyErrors+=m->errors; if (verifyErrors) { rhizome_manifest_free(m); unlink(manifestname); diff --git a/serval.h b/serval.h index 65e6eaaa..c18ab6a9 100644 --- a/serval.h +++ b/serval.h @@ -208,9 +208,11 @@ extern char *batman_peerfile; #define OFS_PINFIELD (OFS_SIDDIDFIELD+SIDDIDFIELD_LEN) #define OFS_PAYLOAD (OFS_PINFIELD+16+16) +#define SID_STRLEN (SID_SIZE*2) + struct response { int code; - unsigned char sid[32]; + unsigned char sid[SID_SIZE]; struct in_addr sender; int recvttl; unsigned char *response; @@ -359,7 +361,7 @@ int extractSid(unsigned char *packet,int *ofs,char *sid); int hlrSetVariable(unsigned char *hlr,int hofs,int varid,int varinstance, unsigned char *value,int len); int extractDid(unsigned char *packet,int *ofs,char *did); -char *hlrSid(unsigned char *hlr,int ofs); +char *hlrSid(unsigned char *hlr, int ofs, char *sid); int parseAssignment(unsigned char *text,int *var_id,unsigned char *value,int *value_len); int writeItem(char *sid,int var_id,int instance,unsigned char *value, int value_start,int value_length,int flags, @@ -929,7 +931,7 @@ int rhizome_fetching_get_fds(struct pollfd *fds,int *fdcount,int fdmax); int rhizome_fetch_poll(); typedef struct dna_identity_status { - char sid[SID_SIZE*2+1]; + char sid[SID_STRLEN]; char did[64+1]; char name[255+1]; diff --git a/server.c b/server.c index 686ffff9..3c7fa827 100644 --- a/server.c +++ b/server.c @@ -208,16 +208,21 @@ int processRequest(unsigned char *packet,int len, if (debug&DEBUG_HLR) fprintf(stderr,"Verified that create request supplies DID but not SID\n"); { - char sid[128]; /* Look for an existing HLR with the requested DID. If there is one, respond with its SID. This handles duplicates of the same message. If there is none, then make a new HLR with random SID and initial DID. */ int ofs = 0; - if (findHlr(hlr, &ofs, sid, did) || !createHlr(did, sid)) { - return respondSimple(sid, ACTION_OKAY, NULL, 0, transaction_id, recvttl, sender, CRYPT_CIPHERED|CRYPT_SIGNED); - } else { - return respondSimple(NULL,ACTION_DECLINED,NULL,0,transaction_id, recvttl,sender,CRYPT_CIPHERED|CRYPT_SIGNED); + int response = ACTION_DECLINED; + if (findHlr(hlr, &ofs, sid, did)) { + hlrSid(hlr, ofs, sid); + if (debug&DEBUG_HLR) fprintf(stderr,"HLR found with did='%s' at ofs=%x: sid='%s'\n", did, ofs, sid); + response = ACTION_OKAY; } + else if (createHlr(did, sid) == 0) { + if (debug&DEBUG_HLR) fprintf(stderr,"HLR created with did='%s': sid='%s'\n", did, sid); + response = ACTION_OKAY; + } + return respondSimple(sid, response, NULL, 0, transaction_id, recvttl, sender, CRYPT_CIPHERED|CRYPT_SIGNED); } pofs+=1; pofs+=1+SID_SIZE; @@ -287,7 +292,8 @@ int processRequest(unsigned char *packet,int len, // Record in cache to prevent re-delivering the same message if a duplicate is received. insertTransactionInCache(transaction_id); } - respondSimple(hlrSid(hlr, ofs), ACTION_OKAY, NULL, 0, transaction_id, recvttl, sender, CRYPT_CIPHERED|CRYPT_SIGNED); + char sid[SID_STRLEN+1]; + respondSimple(hlrSid(hlr, ofs, sid), ACTION_OKAY, NULL, 0, transaction_id, recvttl, sender, CRYPT_CIPHERED|CRYPT_SIGNED); } } break; @@ -397,6 +403,7 @@ int processRequest(unsigned char *packet,int len, if (var_id&0x80) instance=packet[++pofs]; pofs++; int offset=(packet[pofs]<<8)+packet[pofs+1]; pofs+=2; + char sid[SID_STRLEN+1]; char *hlr_sid=NULL; pofs+=2; @@ -448,7 +455,7 @@ int processRequest(unsigned char *packet,int len, if (packageVariableSegment(data,&dlen,h,offset,MAX_DATA_BYTES+16)) return setReason("packageVariableSegment() failed."); - hlr_sid=hlrSid(hlr,ofs); + hlr_sid = hlrSid(hlr, ofs, sid); sendDone++; } @@ -495,7 +502,8 @@ int processRequest(unsigned char *packet,int len, if (packageVariableSegment(data,&dlen,&fake,offset,MAX_DATA_BYTES+16)) return setReason("packageVariableSegment() of gateway URI failed."); - respondSimple(hlrSid(hlr,0),ACTION_DATA,data,dlen, + char sid[SID_STRLEN+1]; + respondSimple(hlrSid(hlr, 0, sid),ACTION_DATA,data,dlen, transaction_id,recvttl,sender, CRYPT_CIPHERED|CRYPT_SIGNED); }