[OWASP-ESAPI] Is there too much use os String in ESAPI interfaces?

Kevin W. Wall kevin.w.wall at gmail.com
Fri Sep 18 15:55:02 EDT 2009


Paul Grove wrote...
>> Could some of the ESAPI interfaces benifit from the points in this
>> article
>>
http://codemonkeyism.com/never-never-never-use-string-in-java-or-at-least-less-often/
>> ?

and in reply, Jim Manico responded:
> I agree with that design pattern for the application level, but for a 
> low-level API like ESAPI, we need the strings. IMO.

As "code monkey" Stephan Schmidt points out in the referenced article,
this really is a domain issue. Keeping that in mind, what is the application
domain for OWASP ESAPI?

Well, "enterprise security" obviously, but also there is a strong focus on
"web applications" (the 'WA' in OWASP). But beyond that, we really can't say
too much about the context of how it might be used.

My $.02 is that for certain important constructs that will be passed from
one class to another and where type safety is paramount because distinguishing
between two different interpretations of a particular String is nebulous
and that confusion in semantics can possibly cause security issues, then
for those cases, we *definitely* should use a (security) domain-related
class.

A candidate of where I think this should have been done, but wasn't
is in the AccessController interface. There we see methods like:

	boolean isAuthorized(Object key, Object runtimeParameter)
	boolean isAuthorizedForData(String action, Object data)
	boolean isAuthorizedForFile(String filepath)
	boolean isAuthorizedForFunction(String functionName)
	boolean isAuthorizedForService(String serviceName)
	boolean isAuthorizedForURL(String url)

The first is probably overly general and seems like a miscellaneous catch all,
and because it is overly general, it causes awkward interfaces for the others.

Access control is definitely one key aware where we want to avoid the possible
semantic confusion that Stephan Schmidt is referring to in the referenced
article.

Personally, I would have preferred something more along the lines of
this, even though it's a bit more work for the ESAPI developers:

	boolean isAuthorized(AccessControlKey key, AccessControlParam runtimeParameter
param)
	boolean isAuthorized(Permission perm, ProtectedResource data)
	boolean isAuthorized(File filepath)
	boolean isAuthorized(Function functionName)
	boolean isAuthorized(Service serviceName)
	boolean isAuthorized(URL url)       // Or perhaps       URI uri  ?

That is part is what OO is all about after all. Whenever you find yourself
doing something like defining a method name to indicate what type of parameter
to accept, you should really revisit your low-level design. Sometimes you
*have to* for compatibility with legacy code; more often we do it because
we are short on time or lazy. (I confess!) But in general, this is a BAD IDEA.

Now, if instead, had AccessController been a concrete class instead of an
interface, we might have gotten away with just 3 isAuthorized() methods...

	boolean isAuthorized(AccessControlKey key, AccessControlParam runtimeParameter
param)
	boolean isAuthorized(Permission perm, ProtectedResource data)
	boolean isAuthorized(ProtectedData data)

but because this is an interface, you give someone who chooses to use
their own implementation a chance to be more selective in what they override
and they could just use the reference implementation for those they don't
override (via delegation).

However, to say that we should go hog-wild crazy and take Stephan Schmidt's
advice literally to "Never, never, never use String in Java", that would,
as Chris Schmidt (probably no relation...but this isn't just sibling
rivalry is it Chris? ;-) "add a lot of code bloat".

Besides, we are somewhat constrained by some of the important J2EE interfaces
themselves (think javax.servlet.http classes) which mostly just deal with String
rather than more specific classes like HttpRequestHeader, HttpResponseHeader,
HttpRequestParameter, or whatever. So if we *did* take this advice to extreme,
not only would ESAPI developers be dealing with code bloat internally, but
we'd receive the scorn and wrath of very web application developer in the world
who was forced to use ESAPI Java, cursing us under their breath as they
convert strings representing HTTP headers or HTTP POST or query parameters
from the Strings that J2EE containers deal with to what ESAPI might require
just so they could validate that their data was safe. (Or course, that's
assuming that ESAPI would have any takers at all. If something like security
isn't fairly straightforward to use, it won't get used. End of story.)

I think that what Stephan Schmidt's article failed to capture is that OO
is not _just_ about data encapsulation, it is also very fundamentally about
_behavior_. So if you have a "FirstName" class and a "LastName" class that
have identical behavior, you need to seriously question why you just don't have
the more general "PersonName" class. Sure there are times where you may want
to distinguish things only so you can have strong typing (those of you who have
used and understand how C++'s Standard Template Library works will know
what I'm talking about), but in general, you want to consider both _data
encapsulation_ AND _behavior_, not just one or the other.


OK, so maybe that was more than $.02 worth. More like $2.00. I'll shut up now.

-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 OWASP-ESAPI mailing list