search

sunliyong / openrtb

Automatically exported from code.google.com/p/openrtb
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Lowercase JSON before calculating MD5 Hash (One line change in Class Signable) #10

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Background/Problem Description:
On line no. 70 and 98 in Class Signable , we are Hex encoding shared secret 
like:
Hex.encodeHex(sharedSecret)

I want to propose a change that would change this call to :
Hex.encodeHex(sharedSecret, true)

Please refer Java docs for this method :
http://commons.apache.org/codec/apidocs/org/apache/commons/codec/binary/Hex.html
#encodeHex(byte[])

The change is suggested to use all lower case letters (or may be upper case) 
while encoding in hex to make encoding consistent.We are generating token using 
the same hex encoded shared secret. As the algorithm used to generate token is 
MD5, case sensitivity of source string matters. 

This would be a breaking change for those who use opposite kind of letter case.
Please let me know your suggestions about this change.

Thanks,
Vaibhav Kamble

Original issue reported on code.google.com by rtb-...@pubmatic.com on 17 May 2011 at 12:20

GoogleCodeExporter commented 9 years ago 
Vaibhav,

I'm going to mark this as WontFix, in favor of the proposal to remove the MD5 
hashing entirely, esp. given issues with different JSON implementations 
ordering elements differently, which seems insurmountable.

From Isreal on openrtb-dev:

Hi,

My understanding was that there is a consensus to drop the MD5 signature 
altogether. I will remove the Signable class, unless there are any objections.

Israel

Original comment by js...@pulsepoint.com on 18 May 2011 at 11:27