Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#936 closed defect (fixed)

Blowfish Base64 Interoperability

Reported by: peterwawood@… Owned by: Tom Trenka
Priority: high Milestone:
Component: General Version: 0.3
Keywords: Cc: peterwawood@…
Blocked By: Blocking:

Description

I was experiencing interoperability problems with dojo.crypto.blowfish using base64 encoding. The issue was the way dojo.crypto.blowfish was padding the Base64 string. (It was encoding one or two Undefineds and then adding padding characters).

I have patched the base64 encode and decode functions and achieved interoperability with Java (based on Marcus Hahn's BlowfishJ with iHarder.net's Base64) and Rebol. My patches are not pretty or optimised as I made the least changes possible to achieve inter-operability. They are included below.

Regards

Peter Wood

The code patches:

function toBase64(ba){

var p="="; var tab="ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; var s=[];

var t=0; PW for (var i =0; i<(ba.length - 2);){ PW

t=ba[i++]<<16|ba[i++]<<8|ba[i++]; s.push(tab.charAt((t>>>18)&0x3f)); s.push(tab.charAt((t>>>12)&0x3f)); s.push(tab.charAt((t>>>6)&0x3f)); s.push(tab.charAt(t&0x3f));

}

var left = ba.length - i; PW

switch(left) PW

{ PW

case 2: 2 bytes left PW

t=ba[i++]<<16|ba[i++]<<8 PW s.push(tab.charAt((t>>>18)&0x3f)); PW s.push(tab.charAt((t>>>12)&0x3f)); PW s.push(tab.charAt((t>>>6)&0x3f)); PW s.push(p); PW break; PW

case 1: 1 byte left PW

t=ba[i++]<<16 PW s.push(tab.charAt((t>>>18)&0x3f)); PW s.push(tab.charAt((t>>>12)&0x3f)); PW s.push(p); PW s.push(p); PW break; PW

default: PW

break; PW

} PW

return s.join("");

} function fromBase64(str){

var s=str.split(""); var p="="; var tab="ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; var out=[]; var t = 0; PW

var l=s.length; var plen = 0; PW

while(s[--l]==p) Replace == with zero padding PW

{

s[l] = 'A'; PW plen++; PW

};

for (var i=0; i<l;){

t=tab.indexOf(s[i++])<<18|tab.indexOf(s[i++])<<12|tab.indexOf(s[i++])<<6|tab.indexOf(s[i++]); out.push((t>>>16)&0xff); out.push((t>>>8)&0xff); out.push(t&0xff);

} return out;

}

Change History (17)

comment:1 Changed 13 years ago by anonymous

Version: 0.20.3

comment:2 Changed 13 years ago by peterwawood@…

The two statements:

var plen = 0;

plen++;

are extraneous though they do no harm other than waste memory and cycles.

comment:3 Changed 13 years ago by bill

Owner: changed from anonymous to Tom Trenka

comment:4 Changed 13 years ago by bill

Cc: peterwawood@… added

Hi Peter, before Tom can take a look at this you need to file an ICLA (www.dojotoolkit.org/icla.txt). Thanks.

comment:5 Changed 13 years ago by Tom Trenka

Also, when Peter does file the CLA please let me know (here is ok).

comment:6 Changed 13 years ago by anonymous

Tom

I faxed a signed ICLA on Tuesday, 13th June (probably Monday - WST). Please let me know by email if it hasn't been received and I will re-send.

comment:7 Changed 13 years ago by anonymous

These are the test cases that I generated via BlowfishJ and Base64 and checked with REBOL/command. They amy be of help.

plainText: "Short Message" password: "Longish Password" EncryptedB64: "uHCkg3z0hiiMU0ggcnJfAw=="

plainText: 'A typical one paragraph post that somebody would write and send ' +

'to another user. It includes $234,000 and an account number ' + '303-5678112-909. It was sent yesterday.'

password: "Peter" EncryptedB64: 'vhcO3rPIi7DxQbC6kU/1i1BvbmoXbzvA0MAjVoWBpuG1ayTpjq' +

'DV6nh83e57Dz21Yj50CNxrEQIzuKTid0hz1J6OpcGJc0GT7y0xlb' + 'Y3ZQEAUQ3U3ocjlXXxrX9nw6aZ3DR8pwgYYbgzP9mbctR8uPdi' '6QdrkhzWc2XodkQHkkPS9YI8GLktsbKpFampFuvmDs8b5OiD7q' + '5+yo8wx8fT98W+ZLQp4xyY'

plainText: 'More than 16 char' password: 'Password' EncryptedB64: 'wFmEC0Mtly1fK5yavNAFIEt+bytbBFuz'

plainText: 'short' password: 'short' EncryptedB64: 'A2ljtAhII54='

plainText: 'short123' password: 'short123' EncryptedB64: '3aRWwwmkSbsDq4DCS+BbYg?=='

plainText: 'short123' password: 'short123' EncryptedB64: '3aRWwwmkSbsDq4DCS+BbYg?=='

plainText: 'short!' password: 'short' EncryptedB64: 'WWNTx/xN7d0='

plainText: 'My other message.' password: 'Peter' EncryptedB64: 'hG3TClEzVFK8VPdkCQ7ZUctxAphLzdT/'

plainText: 'short' password: 'password' EncryptedB64: 'CNZg4eX/KcA='

comment:8 Changed 13 years ago by Tom Trenka

Peter,

I just want to be sure of something...can you also do an MD5 hash comparison and see if you get the same problem? I'm fairly sure what we're dealing with here isn't my Blowfish implementation but the base64 functionality, which is (essentially) duplicated from the base64 conversion functions found in the original Paul Johnstone MD5 implementation. Basically try to do a digest using the Java stuff (a lot of the code I'm working on with crypto is informed by Bouncy Castle, http://www.bouncycastle.org) and then compare it to the output generated by the dojo.crypto.MD5.compute method...and see if you're running into the same issue.

When I wrote this, I tested it against the .NET version of the Bouncy Castle crypto, another C# implementation, a PHP implementation and a Python one (thanks again, Alex), and in each case the base64 conversion was correct...

comment:9 Changed 13 years ago by anonymous

Tom

I will try to do a MD5 comparison (though it will probably be tomorrow at the earliest).

I think the basic conversion is sound, the issue I came across was padding the Base64 string. So the results match the Java/Rebol? results when the length of the char array is divisible by 3.

I let you know as soon as I've been able to look into this further.

Peter

comment:10 Changed 13 years ago by guest

Tom

Just to confirm that I see the problem only with the Base64 encoding, the basic algorithm works well. Take this example:

Password: "Longish Password" Message: "Short Message" BlowfishJ/Base64: "uHCkg3z0hiiMU0ggcnJfAw==" dojo: "uHCkg3z0hiiMU0ggcnJfAwAA=="

The difference is the AA inserted by dojo before the two padding characters. The Java Base64 string is 24 bytes, the dojo string 26 bytes. As I understand, a Base64 string should always be a multiple of 4 bytes.

The current Base64 function doesn't seem to recognise the end of the byte array properly and unintentionally encodes two, one or no Undefined values depending on the length of the byte array.

Peter

comment:11 Changed 13 years ago by guest

I tried a couple of SHA1 hashes and see a different problem. I generated the comparison data with a Java command line program using the Bouncy Castle classes, Rebol and my own simple port of Paul Johnston's code.

Message: "A slightly longer piece of text to hash" dojo: WvMQA+XXDBDjmdwDJusMWM5twoA= Bouncy: tjMQxz2Oqv0oDBpxQAuNuCGP5jY= REBOL: tjMQxz2Oqv0oDBpxQAuNuCGP5jY= My PAJ port: tjMQxz2Oqv0oDBpxQAuNuCGP5jY=

Message: "abc" My PAJ port: qZk+NkcGgWq6PiVxeFDCbJzQ2J0= REBOL qZk+NkcGgWq6PiVxeFDCbJzQ2J0= Bouncy: qZk+NkcGgWq6PiVxeFDCbJzQ2J0=

Peter

comment:12 Changed 13 years ago by guest

I forgot the dojo result from the 'abc' test:

dojo: WvMQA+XXDBDjmdwDJusMWM5twoA=

others: qZk+NkcGgWq6PiVxeFDCbJzQ2J0=

comment:13 Changed 13 years ago by guest

I did a couple of tests using MD5 against REBOL.

Test: "abc" dojo: kAFQmDzST7DWlj99KOF/cg== Rebol: kAFQmDzST7DWlj99KOF/cg==

Test: "A longer message, but not too long"

dojo: 4K2c1/QirGEBn2oA8mPZZA==

Rebol: 4K2c1/QirGEBn2oA8mPZZA==

So apparently no probelms there.

comment:14 Changed 13 years ago by guest

I checked the toBase64 functions in MD5 and Blowfish - they are different. The function in the MD5 class handles the padding inside the main for loop. The function in the Blowfish class handles it afterwards.

comment:15 Changed 13 years ago by Tom Trenka

OK, I'll take a look and fix soonest then. Thanks for doing the comparisons...

btw SHA1 is not official yet at all; I was starting work on that and other things intervened...

comment:16 Changed 13 years ago by Tom Trenka

Resolution: fixed
Status: newclosed

This should be fixed. I see the issue; the toBase64 method was based on a version that operated on words, and I failed to take into account the idea that the byte array was not necessarily a multiple of 3.

Thanks for the catch.

comment:17 Changed 13 years ago by guest

Glad to be of some small assistance.

Note: See TracTickets for help on using tickets.