Relax the dir name check, username can contain ..

[ddelong]

Problem

Between Hadoop 3.3.1 and 3.3.6, new checks were introduced for security purposes. However, those checks were not nearly precise enough, searching for the existence of .. in any part of a path. Usernames can include .., which is acknowledged in some paths (user specific ones), but not in ones that include user as part of a greater path.

Solution

Rather than use the simplistic strstr check, we're using strtok_r to tokenize each part of a path to check if any of those elements exactly matches ... This better captures the intent of the check rather than search for it anywhere in the path string.

These changes are all targeted exclusively at checks introduced between HubSpot 3.3.1 (working) and 3.3.6 (currently incompatible with some jobs).

Alternate Solution Considered

We also considered changing the pattern for kerberos ID generation to remap any instances of .. to another character(s). However, there are very specific conditions and requirements at play here that make this solution non-viable.

1. . was chosen as an alternative to - it wasn't used by URL safe base64 encoding, but wasn't dangerous to our use cases. Any character that can be used by that encoding is invalid as an option (this includes _).
2. We need to be able to map back to the original encoding for hs-secret-agent use, meaning condensing the characters down is also not viable.
3. Other punctuation is either not URL safe or not shell safe, leading to possible side-effects as we experienced when using -.

Ultimately, changing this source code to be more specific is the safest option for currently running jobs and risks fewer future incompatibilities. It's also advantageous as it doesn't require changing multiple other pieces.

Testing

Sadly, the unit test framework for this code is not great and instead relies upon the username of the build entity to do validation. I wrote an impromptu test program to validate the function (see gist below). This will be further tested in QA.

[ddelong]

One alternative that might provide stricter protection: use strtok_r to tokenize these strings with a delimiter of /. Checking the tokens for .. literally then should be very effective as well.

[cathturner]

Makes sense, thanks for doing this! ✅

[johnnysohn]

🚢

[ddelong]

Tested and verified in QA.