The REST code generates an etag as an md5 hash of a representation of
item properties.

That includes properties to which the user does not have access. 
Depending on the schema, that means it could be used as an oracle to
test guesses for values of such properties by generating hashes with
guessed values for those properties inserted and seeing if those match
the provided etag.

This could be addressed by using HMAC with a per-instance random secret
key, instead of a simple hash function, or by storing a random etag in
the database for each item (generated automatically like the 'activity'
property) so it's not related to item properties at all, just changes to
a new random value whenever any other change is made.

See mailing list discussion:
https://sourceforge.net/p/roundup/mailman/message/36615272/

Hi Joseph:

I just pushed a fix for this. Can you review: 5726:e199d0ae4a25
and see if it looks sufficient.

If so please close this issue.

Thanks.

-- rouilj

I don't see anything in that commit to actually use the key in the hmac 
calculation (it's not passed to hmac.new), nor to update those tests that 
assert a particular value of the etag.

Hi Joseph:

In message <alpine.DEB.2.21.1905232304440.30861@digraph.polyomino.org.uk>,
Joseph Myers writes:
>I don't see anything in that commit to actually use the key in the hmac 
>calculation (it's not passed to hmac.new), nor to update those tests that 
>assert a particular value of the etag.

Gah.... Something felt wrong/too easy when I was testing. That's why I
pinged you. Added use of key and fixing tests now.

Thanks.

Hi Joseph:

Ok, lets try rev5727:8b5171f353eb

Also I finally got the etag test finished.

Thoughts...

-- rouilj

I think that looks OK, but the test should have everything that uses the 
patched date.Date inside a

date.Date = dummyDate()
try:
    ... rest of test contents ...
finally:
    date.Date = originalDate

to ensure that the patching is undone if an exception occurs during the 
test code.  (Once we can assume Python 3, unittest.mock.patch can be used 
for this.)

Hi Joseph:

In message <alpine.DEB.2.21.1905242023010.371@digraph.polyomino.org.uk>,
Joseph Myers writes:
>I think that looks OK, but the test should have everything that uses the 
>patched date.Date inside a
>
>date.Date = dummyDate()
>try:
>    ... rest of test contents ...
>finally:
>    date.Date = originalDate
>
>to ensure that the patching is undone if an exception occurs during the 
>test code.  (Once we can assume Python 3, unittest.mock.patch can be used 
>for this.)

Just made the change. That was a try:/finally: with no except right? I
assume any exception that is raised will trigger the finally code and
then be bubbled up the stack to be handled by an except handler at a
higher level.

Just ran that specific test under py2/py3 to make sure
I got the syntax right. See checkin: 5728:bfd28644fe43

If that change works for you, feel free to just close the ticket.

Joseph should I close this issue?

-- rouilj

Yes.