Closed Bug 625183 Opened 13 years ago Closed 10 years ago

config option to enable strict XFF requirement

Categories

(Cloud Services :: Server: Core, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED WONTFIX
Future

People

(Reporter: Atoll, Assigned: rfkelly)

References

Details

(Whiteboard: [qa?])

it would be very useful in production if we could tell *all* of the Python applications to abort and exit if they cannot extract an IP address from the X-Forwarded-For http header.  this isn't a blocker but it would be nice to have.
good idea
Target Milestone: --- → Future
Assignee: nobody → rmiller
Blocks: 676423
That's something we can add into Cornice, as Toby suggested
Shouldn't this go in moz-svc's as its rather Mozilla specific?
it sounds like a good candidate for a 400, so I would do this as a validator, and I can imagine having it in cornice.validators among other 'generic' validators. 

That also makes me think, it would be nice to be able to set default validators at the configuration level somehow
I'm not using cornice for queuey at the moment, which was part of the reason I was hoping to avoid having to implement it again in queuey. It could be a middleware in mozsvc's since it needs to do the check in front of the *entire* app vs. individual views. One of the few really good use-cases for WSGI middleware.
fair enough. Also now I am wondering if this could be simply solved as an nginx rule...
Whiteboard: [qa?]
(In reply to Tarek Ziadé (:tarek) from comment #6)
> fair enough. Also now I am wondering if this could be simply solved as an
> nginx rule...

While we *could* implement this check as an Nginx rule, it would not be sufficient in case of an Nginx misconfiguration. It only takes one human error to break all infrasec CEF logging, without having any way to detect that we've made a mistake.

So, in the end, the application itself is our last chance to detect a missing XFF header - and that's why I filed this bug.
That shouldn't be implemented in Cornice, but rather in the service that uses cornice. This could be done with the 'filter' concept provided by Cornice:

you could define a filter like this one:

    def verify_xff_header(request):
        if 'X-Forwarded-For' not in request.headers:
            request.errors.add('header', 'X-Forwarded-For', 'The "X-Forwarded-For" header is mendatory')

I don't see any benefit of having this directly in Cornice. Having it in mozsvc could make sense, though.
Handing this over to Ryan in case it's still relevant. Ryan, feel free to close if it's not something you want/need to act on.
Assignee: rmiller → rfkelly
This seems useful but I think the config switch should be a more generic "enable moz infra security sanity-checking" or something, because there are other things (e.g. CEF logging) that might profitably be toggled off in other deployments.
After much deliberation, I've decided I don't care enough about this to make time for it - particularly now that we're letting heka take responsibility for CEF logging, and sending the full XFF chain in the heka messages.  Closing out but anyone can feel free to reopen if they feel a strong need for it.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
OK.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.