Subject: Re: [P2PSIP] StoredData and its Signature

From: Cullen Jennings <[email-masked]>
Date: Sun, 8 Nov 2009 13:48:55 +0900

Yep, good catch. There is something wrong here and we need to fix it.  
I think this happened as we cut and pasted too much text as the data  
models got extended.

It seems to me that when we went from having just StoredData to having  
a StoredDataValue inside the storedData, we forgot to update the  
signature.

But before we figure out how to fix this, we need to talk about the  
lifetime and storage_time fields work.

Say you have 2 nodes that can both write to the same location in the  
DHT. The way we deal with merges of partitioned networks is to take  
whatever value was written last. This is done using the storage_time  
field. Unfortunately, this means these two nodes need to have  
synchronized time for this to work. This does not seem all bad as the  
nodes that fail to have synchronized time only hurt their own data. So  
in many usage only one node can write to given location so no  
synchronized time is required at all. The result so this is when a  
node goes to sign something that it wants to store, it needs to  
include it's idea of what the absolute time is in the storage_time.   
Now consider things from the point of view of the nodes actually  
storing the data. They don't want to all have synchronized time. So  
they use the relative times in the lifetime field. Say the usage says  
the data will be stored for 1 hour. Initially the lifetime is set to  
3600 seconds. But after half an hour, a new node joins the network and  
becomes responsible for the data. At this point the currently storing  
node needs to transfer the data to new node, but the 30 minutes have  
already gone by so it needs to transfer the data with lifetime of 1800  
seconds.  This allows all the nodes storing data to use only relative  
time so no global time synchronization is needed. However, the storing  
node needs to update the lifetime when it transfers the data to a new  
node so the lifetime can't be in the signature. Now you might say that  
not having it in the signatures allows the storing node to delete it  
early but this is always true - the storing node can always "loose"  
the data. It's not one of the security invariants of the DHT that your  
data won't be "lost" - we only guarantee it won't be modified if it is  
not lost.

So I'm proposing the fix to this is

Where we have the signature computed over
> resource_id + kind + StoredData (with lifetime set to 0) +  
> SignerIdentity

This is clearly silly as the StoredData includes the Signature.

It should be closer

> resource_id + kind + StoredDataValue + SignerIdentity

However, this changes looses the storage_time so I think it needs to  
become

>   resource_id + kind + storage_time  + StoredDataValue +  
> SignerIdentity

I think this is exactly what you suggested. The other possibility is  
to include the lifetime but set it to zero (that's what the spec says  
now but that seems wacky)

More inline .....

On Oct 31, 2009, at 7:03 , Michael Chen wrote:

> Cullen,
>
> The definition of StoredData the opening of Section 6 and its  
> Signature in Section 6.1 are confusing:
>
>       struct {
>         uint32                  length;
>         uint64                  storage_time;
>         uint32                  lifetime;
>         StoredDataValue         value;
>         Signature               signature;
>       } StoredData;
>
>  The contents of this structure are as follows:
>
>  length
>     The length of the rest of the structure in octets.
>  ...
>
>  value
>     The data value itself, as described in Section 6.2.
>
>  signature
>     A signature over the data value.  Section 6.1 describes the
>     signature computation.  The element is formatted as described in
>     Section 5.3.4
>
> 6.1. Data Signature Computation
>
>  Each StoredData ....  The input to the signature algorithm is:
>
>     resource_id + kind + StoredData + SignerIdentity
>
>  Where these values are:
>  ...
>
>  StoredData
>     The contents of the stored data value, as described in the
>     previous sections, with the lifetime set to 0.
>
> 1) "length" seems to suggest it equals (sizeof(StoredData) - sizeof 
> (length)), which means the size of StoredDataValue and Signature  
> must be obtained before signing. No big deal, but the text should  
> more explicit and precise. Suggestion:
>
>  length
>     The size of the StoredData structure in octets excluding the size
>     of length itself.

sure - I read it to sort of say that but if this change makes it  
clearer, well I like it being clearer so I put you text in.

>
> 2) The big problem is the signature input. In Section 6 the term  
> "data value" appears in both the definition of "value" and  
> "signature", seems to suggest that the 3rd input block for the  
> signature only refer to the "value" field.  In Section 6.1, the  
> instruction of setting "lifetime" to 0 before signing suggest the  
> signature includes the rest of the fields PLUS the signature itself  
> (the 3rd input block is "+ StoredData +").
>
> If I am not mistaken, the correct definition for the signature input  
> should be:
>
>   resource_id + kind + storage_time + lifetime + value +  
> SignerIdentity
>
> The fact that "length" includes the size of the signature is  
> somewhat awkward to be in the input. Excluding it does not degrade  
> the security of the signature.  If it is up to me, instead of  
> setting "lifetime" to 0 before signing, why not exclude it all  
> together?

totally agree  (BTW, when I looked at our code, we were doing the  
above with lifetime set to zero but I agree removing lifetime makes  
sense).

>
> 3) "SignerIdentity" in this section should refer to its definition  
> in Section 5.3.4 to prevent people from reading it as simply a 20- 
> byte SHA1 hash value.
>
>  SignerIdentity
>     The signer identity as defined in Section 5.3.4, which MUST  
> include
>     a hash of the signer's certificate.

yep makes sense - thanks for text

> Thanks
> --Michael