Wandering Thoughts archives

Exim's change to 'taint' some Exim variables is going to cause us pain

Exim is a very flexible mail system (aka a MTA, Mail Transfer agent), to the extent that in its raw state Exim is a mailer construction kit more than a mailer (if you want a simple mailer, consider Postfix). You can use this power for a lot of things, like building simple mailing lists, where a mailing list is created by putting a file of addresses in a specific directory (the name of the file being the name of the mailing list).

This flexibility and power can create security issues, for example when you directly use information from the incoming mail message (information that's under the control of the sender) to open a file in a directory. If not carefully controlled, an attacker who knows enough about your Exim configuration could possibly make you open files you don't intend to, like '../../../etc/passwd'.

(This is a standard risk when using information that's ultimately provided by an attacker.)

For a long time, Exim left it up to whoever wrote your Exim configuration file to worry about this. It was on them to do input validation to make sure that /cs/lists/$local_part would never have anything dangerous in it. Recently the Exim developers decided that this was not sufficient and introduced the idea of 'tainted data', which isn't allowed to be used in various places (especially, as part of a filename that will be opened or accessed). Things that are under the control of a potential attacker, such as the local part or the domain of an address, are tainted.

Unfortunately, there are a lot of places where it's traditionally been natural to use the Exim $local_part string variable as part of file access, which is now tainted and forbidden. Specifically, we have various places in the Exim configurations for several mail machines that use it. These uses are safe in our environment because we only make use of $local_part after it's been verified to exist in our generated list of valid local addresses, but Exim isn't smart enough to know that they're safe. Instead there are a collection of ways to de-taint strings (eg, also, also, also), which from one perspective are a set of artificial hoops you now have to jump through to pacify Exim. Some of these options for de-tainting are backward compatible to versions of Exim before tainting was introduced, but generally the compatible ways are more awkward than the modern best ways.

People, us included, who upgrade to a version of Exim that includes tainting will have to go through their Exim configuration files and revise them to de-taint various things the configuration needs to use. For us, this has to happen for any upgrade of our mail machines to Ubuntu 20.04; 20.04 has a version of Exim with tainting, while the Exim versions in 18.04 and 16.04 are pre-tainting ones. This means that upgrading any of our mail machines to Ubuntu 20.04 needs configuration changes, and some of these configuration changes may not be backward compatible. I think I can find all of the places where our Exim configurations might use tainted data, but I'm not completely confident of that; if I miss one, we're going to experience Exim errors and failures to properly process some email in production.

This is going to be a little bit painful. I'm not looking forward to it, especially as it is yet another case of 'do more work to wind up in exactly the same place'.

(There's an obvious better way for the Exim people to have done this transition to tainted data, but it would have been slower and meant that Exim remained insecure by default for longer.)

PS: We're at least better off than the people on CentOS using EPEL, who apparently got a 'tainted data' version of Exim just dropped on them as a regular package update (cf).

The issue of how to propagate some errors in our Django web app

Much of what our Django application to handle (Unix) account requests does is only available to special people such as professors, who can actually approve account requests instead of just making them. Following our usual, we protect the management section of the web app with Apache HTTP Basic Authentication, where only people in designated Unix groups (such as the 'sponsors' group) have access. However, the Django application also has a 'users' table, and in order to have access to the application you have to be in it (as well as be in a Unix group that's allowed access). Normally we maintain these two things in sync; when we add someone to the relevant Unix group, we also add them as a user (and set the type of user they are, and set up other necessary data for people who can sponsor accounts). But sometimes we overlook this extra step so people wind up permitted by Apache but not in the 'users' table. If they actually try to use our web application, this causes it to stop with an error.

(This 'users' table is part of the application's own set of tables and models, not the normal Django authentication system's User table. Possibly I should have handled this differently so that it's more integrated with normal Django stuff, but when I started this web application I was new to Django and keeping things completely separate was much easier.)

Right now, a bunch of our views look like this at the start:

def approve(request):
  urec = get_user(request)
  if urec.is_staff():
    ...
  ...

You'll note that there's no error handling. This is because get_user() does the brute force simple thing (with some error checks removed):

def get_user(request):
  user = request.META['REMOTE_USER']
  try:
    return models.User.objects.get(login=user)
  except models.User.DoesNotExist:
    ... log a message ...
    raise django.http.Http404

This is simple and reliable, but it has a downside, which is that people who run into this mistake of ours get the same HTTP 404 error page that they'd get if they were trying to go to a URL that genuinely didn't exist in our application. This is at best uninformative and at worst confusing, and I'd like to do better. Unfortunately I'm not sure what the best way to do it is.

My first attempt was to raise Django's Http404 error with a specific message string and then try to make our template for the application's 404 error page check for that string and generate a different set of messages. That failed, because as far as I can see either Django drops the message string at some point in its processing or doesn't pass it to your custom template as a template variable.

I can see three alternate approaches, none of which I'm persuaded by. The simple but unappealing option is to change get_user() to return an error in this situation. This would require a boilerplate change at every place it's called to check the error and handle it by generating a standard 'we screwed up' response page, which makes the code feel like Go instead of Python. But at least how things worked would be obvious (and if I returned None, I could make failures to handle this case relatively obvious).

The more complicated but less code approach is to raise a custom error and wrap every function that calls get_user() with a decorator that catches the error to generate and return the standard explanation page. I would have to decorate every view function that directly or indirectly calls get_user() (and remember to add this if I added new functions), and decorators are sort of advanced Python magic that aren't necessarily either clear or straightforward for people to follow.

I suspect that the way I'm supposed to do this in Django is with some form of middleware. If I kept to much of the current approach, I could do this with a middleware that just used process_exception(), but that doesn't seem the most idiomatic way. Since this is a common processing step for anything protected behind HTTP Basic Authentication, it feels like the middleware should do the user lookup itself and attach it to the request somehow, with the actual views not even calling get_user(). But I don't know how to attach arbitrary data to Django's request objects, and anyway that involves even more Django magic than middleware that catches a custom exception (and the magic is less clear, since the view functions would just access data without any obvious reason for it to be there).

(I care about the amount of magic involved in any solution because my co-workers aren't particularly familiar with Django and even I only touch the code every once in a while. Possibly this means I should just use the explicit error checking version, even if it makes me twitch.)