[dev.icinga.com #10548] Implement CSRF protection for the API

[icinga-migration]

This issue has been migrated from Redmine: https://dev.icinga.com/issues/10548

Created by tgelf on 2015-11-05 13:59:55 +00:00

Assignee: mfriedrich
Status: Resolved (closed on 2015-11-06 17:04:21 +00:00)
Target Version: 2.4.0
Last Update: 2015-11-12 15:34:51 +00:00 (in Redmine)

Backport?: No
Include in Changelog: 1

---

The issue

Our REST-API is usable through a generic browser. Who does so is usually a potential target for CSRF attacks, even if he "just played around". We do not have to care about JavaScript, there we are forced to trust in SOP. We also cannot protect XSS victims. But we absolutely need to protect users from opening themselves to CSRF attacks.

Proposed solution

CSRF tokens make no sense for API clients (and require additional roundtrips). The easiest solution to mitigate this problem is to make an HTTP header not used by web browsers for legacy forms as a hard requirement, at least for all non-GET requests. I'd opt for:

Accept: application/json

Special cases

• We will allow GET requests without an Accept header, otherwise we would hinder people from "playing around with a web browser"
• This means that GET requests would still be subject to JSON hijacking. Fortunately, those attacks are only possible when the root node of your JSON response is an Array. To me it looks like all our responses are Objects right now, please double-check whether this is always given.
• config/files/path/to/file is afaik the only request that does NOT return a JSON object. API clients firing such requests with Accept: application/json should either be rejected (quickfix) or given a result based on the Accept header (current behaviour for text/plain, { results: [ {filename: '...', content: '...' } ], ...} for application/json

Cheers,
Thomas

Changesets

2015-11-05 14:18:53 +00:00 by mfriedrich 7e5f554

Require 'Accept' header for API requests (except for GET)

fixes #10548

[icinga-migration]

Updated by tgelf on 2015-11-05 14:00:53 +00:00

• Description updated

[icinga-migration]

Updated by mfriedrich on 2015-11-05 14:20:04 +00:00

• Status changed from Assigned to Resolved
• Done % changed from 0 to 100

Applied in changeset 7e5f554.

[icinga-migration]

Updated by mfriedrich on 2015-11-05 14:20:29 +00:00

• Assigned to changed from gbeutner to mfriedrich

Please test the fix.

[icinga-migration]

Updated by gbeutner on 2015-11-06 16:22:57 +00:00

• Subject changed from REST-API needs CSRF protection to Implement CSRF protection for the API
• Status changed from Resolved to Feedback

[icinga-migration]

Updated by gbeutner on 2015-11-06 17:04:21 +00:00

• Status changed from Feedback to Resolved

[icinga-migration]

Updated by mfriedrich on 2015-11-11 14:33:43 +00:00

• Tracker changed from Bug to Feature

[icinga-migration]

Updated by tgelf on 2015-11-11 14:56:35 +00:00

Just for the records, lack of CSRF protection is a bug nowadays, it's implementation is IMHO not a feature ;)

[icinga-migration]

Updated by mfriedrich on 2015-11-12 15:34:51 +00:00

• Backport? changed from TBD to No