On the importance of thorough testing

Much of modern software development revolves around the concept of “quality”. As with all abstract concepts, “quality” is somewhat difficult to pin down, but for this article we can define it as “how well software conforms to its requirements”. Less formally, we can say that “high-quality software does what it’s supposed to”.

One way to ensure that you consistently deliver high-quality software is to have automated tests. I’m not a Test-Driven Development fundamentalist insisting on 100% test coverage; but if some part of your software is important to your users, there should be at least one automated test demonstrating that it works. This is obviously true for your software’s functional requirements, but it applies equally to its non-functional requirements such as auditing and performance.

So what non-functional requirements are important to the users of Basefarm’s internally-developed applications? One thing that immediately springs to (at least my) mind is security. So we need some tests for that, right?

Think like a hacker

When developing secure applications, it is always useful to try to think like an attacker. If I were to try to get unauthorized access to this application, how would I go about it?

Obviously, you’d first need to log in. For Basefarm’s internal applications we use username/password logins authenticating against our internal LDAP server; this means that an attacker must obtain a set of working credentials somehow.

This suggests three tests:

An existing user can log in with the correct password (this is what we call the happy-path test, which demonstrates that in fair weather conditions the software works as intended);

A non-existent user is not allowed access;

An existing user logging in with an incorrect password is not allowed access

As part of these tests, it is also useful to think about things like auditing and information leakage; login attempts — successful or not — should be logged, and the error message(s) presented on failure should not give an attacker information she does not already have. In particular, authentication failures should not say “no such user” or “incorrect password”; “Invalid credentials” is a safe, neutral way to put it.

Don’t reinvent the wheel

The developers at Basefarm, as in most other places, don’t have the time (or, to be honest, the expertise) to write everything from scratch. It is hard enough to implement our specific functionality; implementing and maintaining code to talk to databases and LDAP, handle authentication and transactions and all the other things a large-scale (dare I say Enterprise?) application needs would be impossible. So like many other teams on the JVM platform we lean heavily on the Spring framework and its attendant ecosystem of libraries; in particular, we use Spring Security for authentication and authorization.

We originally implemented this in 2011; and the configuration worked well, all the tests passed, and we were happy.

Let’s dance!

Fast-forward six years, to 2017. As part of our regular maintenance cycle, I went through all of our application’s dependencies and updated them to the latest version, to get the benefit of whatever new features and fixes are available.

This is usually a cushy job: Update some version numbers; recompile everything; drink coffee while the tests pass; and then ship it.

Finding the problem

I’ll spare you the details of how we debugged the issue; it’s not really interesting, involving as it did copious amounts of coffee, creative swearing, scratching of heads, wailing and gnashing of teeth, and poring over code, logs and packet captures.

It turned out that the problem was not in Spring Security per se, but in Spring LDAP; a change in Spring Security’s use of that library exposed a weakness in DefaultTlsDirContextAuthenticationStrategy, which never actually performs an LDAP “bind” operation using the provided credentials.

The desired sequence of events when authenticating in our setup is something like this:

Open a new LDAP connection

Secure it using STARTTLS

Bind with the search credentials (username/password from the context-source)

Perform an LDAP search for the DN to bind as for the real authentication step

Open another new LDAP connection

Secure it using STARTTLS

Bind using the DN from step 4 and the provided password

The logs from the LDAP server show what really happens (I’ve removed some entries from the log; the full log can be seen here):

However, a slight modification of the example project showed that the erroneous behaviour persists even when anonymous bind is not used; it is clear from the log that there is no bind attempt with the user credentials at all.

After a little back and forth, the Spring LDAP team released a fixed version of Spring LDAP on October 6, 2017, nearly a year after the initial bug report.