[Esapi-user] Planned fix for Encryptor.seal() / Encryptor.unseal()

Sun May 9 21:36:10 EDT 2010

I vote nuke and pave - blow it away and do it right. :)

Jim Manico

On May 9, 2010, at 4:46 PM, "Kevin W. Wall" <kevin.w.wall at gmail.com>  
wrote:

> Back in August 2009, cyounkins reported a bug in the reference  
> implementation
> of Encryptor.seal() / Encryptor.unseal() methods. (See Google issue  
> # 28.)
>
> I recently independently discovered the same bug while inspecting  
> these methods
> in the JavaEncryptor reference implementation class.
>
> At issue is that the seal / unseal uses a ":" for a delimiter  
> character but
> never does any special encoding of the original data to be sealed.  
> Thus if
> the original data has any colons anywhere in it, the attempt to  
> unseal will
> throw an EncryptionException stating that it was an "Invalid seal".
>
> This is a really easy problem to fix *IF* backward compatibility is  
> not
> something that we need to be concerned about. And unless someone is  
> *storing*
> these cryptographic seals based on any version (thus far--up through  
> 2.0-rc6)
> of ESAPI, there is not reason to apply this simple fix.
>
> So my question, mostly directed to the ESAPI-Users group is this:
>
>    Is anyone calling the JavaEncryptor.seal() method and *storing*
>    the results?
>
> If the answer is "yes", is it important to you that fixed versions be
> backward compatible with the current buggy versions? If that's  
> something
> that is needed, I think I can come up with a to make it work, but  
> the solution
> will be rather ugly as well as resulting in any new seals being  
> longer than
> the otherwise would need to be.
>
> CAVEAT: It should be noted that these seals are dependent on the  
> deprecated
>    Encryptor.encrypt() / Encryptor.decrypt() modes. And since even  
> that
>    has changed significantly since ESAPI 1.4.x (that used a completely
>    different encryption algorithm), if you plan on moving from ESAPI  
> 1.4.x
>    to ESAPI 2.0, then the seals will not be backward compatible  
> anyhow.
>    (It should use a fixed crypto algorithm rather than one dependent  
> on
>    settings from ESAPI.properties, but that's another issue entirely.)
>
> But for now, I am just exploring if anyone has an objection to me  
> intentionally
> breaking backward compatibility (with a defect) to favor the simple  
> solution
> over a much more complex one.
>
> Please let me know your vote by next Sunday, 5/16: (*)
>
>    A) It *must* remain backward compatible with the current buggy
>       approach as I have stored numerous cryptographic seals that
>       I cannot recalculate.
>
>    B) Blow it away and do it right. Simple solutions are preferred
>       over complex and if I have any stored cryptographic seals I
>       am willing to recalculate them.
> -----------------
> *  I am allowing one full week for those of you who on ESAPI-Users  
> who have
>   signed up for weekly digests of the mailing lists.
>
> If I fail to hear from a significant number of you, I will likely  
> choose 'B'.
> If I end up changing it so it is not backward compatible, I will  
> also be
> getting rid of the deprecated encrypt() / decrypt() calls that these  
> seal() /
> unseal() methods use.
>
> -kevin
> -- 
> Kevin W. Wall
> "The most likely way for the world to be destroyed, most experts  
> agree,
> is by accident. That's where we come in; we're computer professionals.
> We cause accidents."        -- Nathaniel Borenstein, co-creator of  
> MIME
> _______________________________________________
> Esapi-user mailing list
> Esapi-user at lists.owasp.org
> https://lists.owasp.org/mailman/listinfo/esapi-user