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

Kevin W. Wall kevin.w.wall at gmail.com
Sun May 9 19:46:06 EDT 2010


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


More information about the Esapi-user mailing list