Thread: [Jamwiki-devel] Question about code fragment

Brought to you by: wrh2

jamwiki-devel

[Jamwiki-devel] Question about code fragment

From: <jam...@li...> - 2013-03-10 18:56:55

Hi,

I stumbled about JAMWiki not using any salt for password hashing and wanted to start a request about it.
Than I figured JIRA exists and found JAMWIKI-36 ... So now I'm trying to take care about it.

While crawling through the code, to get an image about who's using what, when and why I saw 

org.jamwiki.utils.Encryption.bytes2String(byte[])

and am wondering, what's the concrete intention behind it?
I see what it does and from it's usage I see what's it currently expected to do.
But to achieve the very same much simpler code would be sufficient. [1]
So I fear there's something else "implied", which I don't see ...

Anybody out there able to answer my question?

Best regards,
Peter

[1]: 'new String(byteData, "US-ASCII")' or 'new String(byteData)'
because simply casting byte to char is only valid for the very optimistic assumption every provided byte value represents a valid US-ASCII (or system default charset) character.

The concrete question that came up to me is: what, if this method just returns a hex string representation of 'byte[]'? OTOH than Encryption.decrypt64(String) might break ...

Re: [Jamwiki-devel] Question about code fragment

From: <jam...@li...> - 2013-03-10 19:14:34

I think the Encryption.bytes2String method can be killed - if I had to 
guess that's probably just very old code and no one noticed the possible 
cleanup.

One caveat if you're looking into the password salts is to ensure that 
any change will work for existing installations (or with an upgrade 
process).  I haven't looked at that issue in a long time, but as I 
recall there was no easy way to upgrade existing installations to 
support password salts using Java 5, but the new encryption methods in 
Java 6 made it possible.  With JAMWiki 2.0 dropping support for Java 5 
it should now be feasible to support password salts and upgrades, 
although it has been so long since I looked at the issue that I don't 
remember what the specific issues requiring a new Java version were.

Let me know if you run into any problems as this was one of the items I 
had on the TODO list once Java 5 support was dropped.

Ryan

On 3/10/2013 11:56 AM, jam...@li... wrote:
> Hi,
>
> I stumbled about JAMWiki not using any salt for password hashing and
> wanted to start a request about it. Than I figured JIRA exists and
> found JAMWIKI-36 ... So now I'm trying to take care about it.
>
> While crawling through the code, to get an image about who's using
> what, when and why I saw
>
> org.jamwiki.utils.Encryption.bytes2String(byte[])
>
> and am wondering, what's the concrete intention behind it? I see what
> it does and from it's usage I see what's it currently expected to
> do. But to achieve the very same much simpler code would be
> sufficient. [1] So I fear there's something else "implied", which I
> don't see ...
>
> Anybody out there able to answer my question?
>
> Best regards, Peter
>
> [1]: 'new String(byteData, "US-ASCII")' or 'new String(byteData)'
> because simply casting byte to char is only valid for the very
> optimistic assumption every provided byte value represents a valid
> US-ASCII (or system default charset) character.
>
> The concrete question that came up to me is: what, if this method
> just returns a hex string representation of 'byte[]'? OTOH than
> Encryption.decrypt64(String) might break ...

[Jamwiki-devel] Passwords with salts [was: Question about code fragment]

From: Peter P. <pit...@us...> - 2013-03-16 23:46:20

Attachments: jamwiki_password_salt.zip

Hi,

Am 10.03.2013 um 20:14 schrieb jam...@li...:
> I think the Encryption.bytes2String method can be killed - if I had to 
> guess that's probably just very old code and no one noticed the possible 
> cleanup.

Did a SVN history search and the code came already with import from VQwiki.
Cleaned it up, because I think it's useless and even more: dangerous.
"new String()" is dangerous the very same way, but does not obfuscate this; 
So I preferred it for now, until I found a way to rearrange this stuff as a whole thing.

> One caveat if you're looking into the password salts is to ensure that 
> any change will work for existing installations (or with an upgrade 
> process).

I gave my very best. There's a small upgrade necessity, but I'll explain below.

> I haven't looked at that issue in a long time, but as I 
> recall there was no easy way to upgrade existing installations to 
> support password salts using Java 5,

I actually had and have no idea, why salts for passwords should be a matter of Java version, whether 1.4, 5, 6 or 7.
I implemented salts without any known binding to used Java version.

I created a comprehensive patch, which hopefully covers all relevant locations.
I'll attach it to this mail. If this list blocks it, I'll upload it to some web space and share the link.
The reason behind this is, I'd like to avoid committing an unchecked patch to my branch, that has to be reverted.
"Unchecked" in the sense I'd like to have at least a second, even better third, opinion on the proposal for change.

So it would be nice if someone could apply this patch to a local export of the sources and test if every thing works well; for me it does.
Maybe someone can even test if an already existing installation still works ... After the necessary small upgrade.

The details and my thoughts are in "README.md", contained in the mentioned attachment.

So ... Feedback time :-)
-- 
Best regards,
Peter