ESAPI GitHub issue #582 - Separating codecs and canonicalization functionality into a separate maven artifact

[Kevin W. Wall]

All,

I would like to move this discussion of ESAPI GitHub issue #582 to here, since it is exactly the sort of thing that was intended to be discussed here. Also, I think that it is a bit easier to respond point by point than it is via GitHub comments of GitHub issues. Please see the aforementioned issues link for the complete history of these comments. I intend to specifically reply to the points raised by @forgedhallpass from the comments here, which I am reproducing here for your convenience. (Blame any formatting issues or botches in copy/paste on me.)

As for the rest of those on this list, PLEASE participate and let us know your thoughts about this. (Ideally, I'd ask you to trim the content and reply inline as I have below, but I'm not going to scream [at least not out loud at you] even if you top-post! :)

Comment by @forgedhallpass interspersed with my comments inline below, in red. -kevin

I am still not sure what do you see as a big of a problem, maybe I am missing something. There could be a maven multi-module project, with the parent with pom packaging, two modules for now, the esapi-java-legacy and the esapi-c14,

esapi-c14n. c14n as in i18n. But whatever.

or some other name as you mentioned above. The esapi-java-legacy module with jar packaging type could have the esapi-c14 module as a compile dependency. Both modules could inherit the version from the parent, so the releasing should not be too complicated and
users wouldn't need to add the new dependency manually either.

Fair enough. That would at least solve the complexity from the user because if they included the esapi jar, they would get the esapi-c14n jar pulled down from Maven Central as a dependency. Which means devs using ESAPI wouldn't have to revise their build configurations (at least not for build tools that automatically fetch dependencies).

That still leaves some problems to be addressed, like revising the instructions for the ESAPI-release-steps.odt document. That also doesn't address the separation of the JUnit tests and the Javadoc. Neither of those are insurmountable problems, but they are headaches that the ESAPI core team doesn't particularly want to address either. However, if the proposed PR addressed those potential pitfalls, then I think the 3 of us would be much more inclined to answer.

As for my motivation to get rid of ESAPI:

• question like Should I use ESAPI? I'd prefer keeping the operational risk of my projects low and legacy for me means that at some point it will be deprecated.

The deprecretation policy is pretty liberal. From ESAPI's README.md file (with typos corrected):

Unless we unintentionally screw-up, our intent is to keep classes, methods, and/or fields which have been annotated as "@deprecated" for a minimum of two (2) years or until the next major release number (e.g., 3.x as of now), whichever comes first, before we remove them.

Note that this policy does not apply to classes under the org.owasp.esapi.reference package. You are not expected to be using such classes directly in your code.

If your concern is that we will immediately disbandon it once we release ESAPI 3, that is certainly not the intent, but at that point, we certainly wouldn't be entertaining new features or restructuring. The ESAPI 2.x release would go into a maintenance mode only and we'd support it [well, that is the intent; I suppose we could all get abducted by aliens and no longer have access to the Internet], with a focus on fixing showstopper bugs and any identified security vulnerabilities. Furthermore, the intent for ESAPI 3 is to give it a new package namespace. (E.g., org.owasp.esapi3 or perhaps org.owasp.esapiNG, etc. Exact name is TBD.) That at least in theory, should allow developers to start using ESAPI 3 code while still keeping their ESAPI 3 code. That means they wouldn't be required to make a flash cut. (An alternative might be to build an ESAPI 2 to ESAPI 3 bridge, but to me that seems like a bigger kludge.)

• "The second question you should ask, if I’m using it, why am I not contributing to it in some manner?"

  You can't say I haven't tried ;-)

Well, there's a couple things here. There's been others who have requested to me (in private email conversations or at security conferences) that we "split out component X" from ESAPI 2. (Safe logging has been a common one, but I've also had one request for splitting out encryption for use with encrypted properties files as the main use case.) Saying yes to "canonicalization" may seem like we are playing favorites here, especially when (IMO), this is a very marginalized / nuche use case compared to Safe Logging. The only reason why I'm remotely even giving it consideration is your offer to do a PR. But I doubt if we can count on you to maintain it, or can we? That may make a difference.

• it also seems that the 3.x version is DOA. Last commits around 7 years old. Maybe the investment should rather go there...

Have you ever heard of the "Tyranny of the Urgent"? It's a thing. Given that we have so many open issues (roughly a hundred last I counted) and only 3 people to work consistently on ESAPI 2, we focus on that because we have existing clients. But trust me, I think the 3 of us would much prefer working on ESAPI 3.x.

(And as for those commits, don't be surprised if we burn all that to the ground and start over. Those were created long ago in a galaxy far away under different leadership.)

• I need to get rid of LGPL 2.1 licenses (com.io7m.xom:xom)

Okay. Xom is only used in 2 ESAPI classes:

• org.owasp.esapi.waf.ConfigurationException
• org.owasp.esapi.waf.ConfigurationParser

If that is a goal, then maybe your PR should be to rewrite the ESAPI WAF's Configuration parser. That might get rid of Xom and some of the transitive dependencies as well, such as xalan.

• my OSS scanning tool reported that there were vulnerable transitive dependencies (I believe that has been solved with v2.2.2.0)

There is still an unresolved vulnerable transitive dependency that is not part of NVD that is:

1. Not exploitable in the manner that ESAPI uses is.
2. A workaround is possible.

See Security Bulletin #3 for details. And if you are using log4j 1.x for ESAPI logging, there is a vulnerability there that might show up in a scan but also isn't exploitable. (See Security Bulletin #2 for details on that.) If you are having problems convincing your management not to be alarmed, then read those 2 security bulletins. They ought to aleve your management's concerns. (I've been doing that sort of analysis for better than 10 years as part of my day job, so unless you have an Insecure Reflection issue elsewhere in your application code, I don't think either of them are concerns in the manner that ESAPI uses the vulnerable dependencies.)

In addition to regularly running OWASP Dependency Check when I make changes to ESAPI, I am also subscribed to Snyk and GitHub Dependabot so new vulnerabilities show up pretty early and if not, someone generally emails me on a private email.

• I appreciate your effort of wanting to be backwards compatible, but since this is blocking you moving further it will only drive people away. E.g. the EOL date for Java 7 was ~5 years ago. The market dictates if you decide not to upgrade, you'll have to pay for the support...in case of FOS, that could mean that you'll have to backport the changes to a legacy version that you are using.

Well, in all honesty, the same can be said for the OWASP Java Encoder Project. I think they still only require Java 5 and that's been EOL for much longer and you seem to have no concerns there qw your GitHub issue for that implies you are using it.

All that really says about ESAPI is that we write it in a manner that you can use it if you only are using Java 7. Hopefully now one is, but you'd be surprised, especially with smaller businesses.

There are people using ESAPI 2.x with Java 8 and Java 11 as well. It's more of a pain to the ESAPI maintainers than it is to the ESAPI user community.

With that being said, I do not intend to spend time on a PR, if you are not convinced,

I am skeptical, but I believe myself and my 2 ESAPI developer colleagues are fair and I think we would give you a fair shake. But one reason why I moved the discussion here is that I hope that others will pipe in with their thoughts on this. Separating things out the way you propose will be a somewhat bigger maintenance effort for us so I'd like to see if others would actually think this is a useful split. I'm not convinced that it is, based on my observation of how I've seen ESAPI used by F100 businesses, but I am also open minded.

but it's funny that:

1. you have talked about backward compatibility, because I've tried upgrading from v2.2.0.0 to v2.2.2.0 and my build failed. Of course this could be because I might have initialized the solution in an unintended way (a few years back), with the intention to get it up and running with as less initialization code as possible:

private static void initESAPI() {
        final Properties esapiProperties = new Properties();
        esapiProperties.setProperty(DefaultSecurityConfiguration.ENCODER_IMPLEMENTATION, CustomEncoder.class.getName());
        esapiProperties.setProperty(DefaultSecurityConfiguration.ALLOW_MULTIPLE_ENCODING, String.valueOf(false));
        esapiProperties.setProperty(DefaultSecurityConfiguration.ALLOW_MIXED_ENCODING, String.valueOf(false));
        esapiProperties.setProperty(DefaultSecurityConfiguration.CIPHER_TRANSFORMATION_IMPLEMENTATION, "AES/CBC/PKCS5Padding");

        ESAPI.override(new DefaultSecurityConfiguration(esapiProperties));
    }

Danger Will Robinson! From the ESAPI.override() Javadoc:

 Overrides the current security configuration with a new implementation. This is meant
 to be used as a temporary means to alter the behavior of the ESAPI and should *NEVER*
 be used in a production environment as it will affect the behavior and configuration of
 the ESAPI *GLOBALLY*.

I seriously thought that we had deprecated that. Maybe Matt or Jeremiah remember why we didn't. But it was
put in as a test, mostly for running JUnit tests because some stuff was difficult, if not impossible to test because
of the abuse of the Singleton pattern in ESAPI.

 To clear an overridden Configuration, simple call this method with null for the config
 parameter.
    
public static class CustomEncoder extends DefaultEncoder {

	private static volatile Encoder singletonInstance;

	public CustomEncoder(final List<String> codecNames) {
		super(codecNames);
	}

	public static Encoder getInstance() {
		if (singletonInstance == null) {
			synchronized (DefaultEncoder.class) {
				if (singletonInstance == null) {
					singletonInstance = new DefaultEncoder(Arrays.asList(HTMLEntityCodec.class.getSimpleName(),
											     PercentCodec.class.getSimpleName()));
				}
			}
		}
		return singletonInstance;
	}
}

I'm not sure what your compilation error was, but if you want to email me with it (or better post it to either the
ESAPI-Pro...@owasp.org Google group or post on Stack Overflow we can try to help. Or if it is a bug (very possible), then

create a new GitHub issue. Certainly it wasn't our intent to break backwards compatibility.

2. After I've extracted only the canonicalization part for my projects, I had to modify the documentation because my build failed again because of the javadocs from ESAPI (-Xdoclint:-html).

That too sounds like a bug in ESAPI Javadoc. There are lots of warnings we get when building with Java 8, but maybe you are using Java 11 and it has stricter checks. Could you please open a new GitHub issue if you can narrow the problem down a bit? Thanks.

-kevin

--

Blog: http://off-the-wall-security.blogspot.com/    | Twitter: @KevinWWall
NSA: All your crypto bit are belong to us.