-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add CSRF plugin #5727
feat: add CSRF plugin #5727
Changes from 26 commits
ad8c54d
843a6ff
e40347b
af25b19
b1032e7
0ea8477
2a52163
55d656f
7a61185
a03da0a
7d14375
6413427
15c9802
b52d042
9c42231
45065bb
a4b284e
6c999e6
165b8ad
2e3dd65
d05242e
a219794
7eb2e9f
7eb2524
2454909
d439243
d0f1f71
cfff42a
9305e2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,161 @@ | ||
-- | ||
-- Licensed to the Apache Software Foundation (ASF) under one or more | ||
-- contributor license agreements. See the NOTICE file distributed with | ||
-- this work for additional information regarding copyright ownership. | ||
-- The ASF licenses this file to You under the Apache License, Version 2.0 | ||
-- (the "License"); you may not use this file except in compliance with | ||
-- the License. You may obtain a copy of the License at | ||
-- | ||
-- http://www.apache.org/licenses/LICENSE-2.0 | ||
-- | ||
-- Unless required by applicable law or agreed to in writing, software | ||
-- distributed under the License is distributed on an "AS IS" BASIS, | ||
-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
-- See the License for the specific language governing permissions and | ||
-- limitations under the License. | ||
-- | ||
local core = require("apisix.core") | ||
local resty_sha256 = require("resty.sha256") | ||
local str = require("resty.string") | ||
local ngx = ngx | ||
local ngx_encode_base64 = ngx.encode_base64 | ||
local ngx_decode_base64 = ngx.decode_base64 | ||
local ngx_time = ngx.time | ||
local cookie_time = ngx.cookie_time | ||
local math = math | ||
|
||
|
||
local schema = { | ||
Baoyuantop marked this conversation as resolved.
Show resolved
Hide resolved
|
||
type = "object", | ||
properties = { | ||
key = { | ||
description = "use to generate csrf token", | ||
type = "string", | ||
}, | ||
expires = { | ||
description = "expires time(s) for csrf token", | ||
type = "integer", | ||
default = 7200 | ||
}, | ||
name = { | ||
description = "the csrf token name", | ||
type = "string", | ||
default = "apisix-csrf-token" | ||
} | ||
}, | ||
required = {"key"} | ||
} | ||
|
||
|
||
local _M = { | ||
version = 0.1, | ||
priority = 2980, | ||
name = "csrf", | ||
schema = schema, | ||
} | ||
|
||
|
||
function _M.check_schema(conf) | ||
return core.schema.check(schema, conf) | ||
end | ||
|
||
|
||
local function gen_sign(random, expires, key) | ||
local sha256 = resty_sha256:new() | ||
|
||
local sign = "{expires:" .. expires .. ",random:" .. random .. ",key:" .. key .. "}" | ||
|
||
sha256:update(sign) | ||
local digest = sha256:final() | ||
|
||
return str.to_hex(digest) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please localize the method as from resty.string module we are using this single method. |
||
end | ||
|
||
|
||
local function gen_csrf_token(conf) | ||
local random = math.random() | ||
local sign = gen_sign(random, conf.expires, conf.key) | ||
|
||
local token = { | ||
random = random, | ||
expires = conf.expires, | ||
sign = sign, | ||
} | ||
|
||
local cookie = ngx_encode_base64(core.json.encode(token)) | ||
return cookie | ||
end | ||
|
||
|
||
local function check_csrf_token(conf, ctx, token) | ||
local token_str = ngx_decode_base64(token) | ||
if token_str == nil then | ||
Baoyuantop marked this conversation as resolved.
Show resolved
Hide resolved
|
||
core.log.error("csrf token base64 decode error") | ||
return false | ||
end | ||
|
||
local token_table, err = core.json.decode(token_str) | ||
if err then | ||
core.log.error("decode token error: ", err) | ||
return false | ||
end | ||
|
||
local random = token_table["random"] | ||
if not random then | ||
core.log.error("no random in token") | ||
return false | ||
end | ||
|
||
local expires = token_table["expires"] | ||
if not expires then | ||
core.log.error("no expires in token") | ||
return false | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a proposal here, in the token object instead of total expiration seconds, can we use the expiration timestamp? So during the checking phase inside the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Thanks for @bisakhmondal 's suggestion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey @Baoyuantop, thanks for making the changes. Are you going to address this in this PR or do you think it would be better to have it on a separate PR? Thank you. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @bisakhmondal, I would prefer a separate PR to solve this problem, this PR is already a bit big. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. Sounds good to me. Thanks There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please open an issue for keeping track of the progress. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done @bisakhmondal. #6141 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you. Merging. |
||
|
||
local sign = gen_sign(random, expires, conf.key) | ||
if token_table["sign"] ~= sign then | ||
core.log.error("Invalid signatures") | ||
return false | ||
end | ||
|
||
return true | ||
end | ||
|
||
|
||
function _M.access(conf, ctx) | ||
local safe_methods = {"GET", "HEAD", "OPTIONS"} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we extract this out from the access phase and treat it as a constant array? |
||
local method = core.request.get_method(ctx) | ||
if core.table.array_find(safe_methods, method) then | ||
return | ||
end | ||
|
||
local header_token = core.request.header(ctx, conf.name) | ||
if not header_token or header_token == "" then | ||
return 401, {error_msg = "no csrf token in headers"} | ||
end | ||
|
||
local cookie_token = ctx.var["cookie_" .. conf.name] | ||
if not cookie_token then | ||
return 401, {error_msg = "no csrf cookie"} | ||
end | ||
|
||
if header_token ~= cookie_token then | ||
return 401, {error_msg = "csrf token mismatch"} | ||
end | ||
|
||
local result = check_csrf_token(conf, ctx, cookie_token) | ||
if not result then | ||
return 401, {error_msg = "Failed to verify the csrf token signature"} | ||
end | ||
end | ||
|
||
|
||
function _M.header_filter(conf, ctx) | ||
local csrf_token = gen_csrf_token(conf) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better to emphasize in the doc that a new cookie is generated for each request There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This problem is not addressed? |
||
local cookie = conf.name .. "=" .. csrf_token .. ";path=/;SameSite=Lax;Expires=" | ||
.. cookie_time(ngx_time() + conf.expires) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of Since, the operation is performed for each request, at the long run it will save some computation. |
||
core.response.add_header("Set-Cookie", cookie) | ||
end | ||
|
||
Baoyuantop marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return _M |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,136 @@ | ||||||
--- | ||||||
title: csrf | ||||||
--- | ||||||
|
||||||
<!-- | ||||||
# | ||||||
# Licensed to the Apache Software Foundation (ASF) under one or more | ||||||
# contributor license agreements. See the NOTICE file distributed with | ||||||
# this work for additional information regarding copyright ownership. | ||||||
# The ASF licenses this file to You under the Apache License, Version 2.0 | ||||||
# (the "License"); you may not use this file except in compliance with | ||||||
# the License. You may obtain a copy of the License at | ||||||
# | ||||||
# http://www.apache.org/licenses/LICENSE-2.0 | ||||||
# | ||||||
# Unless required by applicable law or agreed to in writing, software | ||||||
# distributed under the License is distributed on an "AS IS" BASIS, | ||||||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||
# See the License for the specific language governing permissions and | ||||||
# limitations under the License. | ||||||
# | ||||||
--> | ||||||
|
||||||
## Summary | ||||||
|
||||||
- [**Description**](#description) | ||||||
- [**Attributes**](#attributes) | ||||||
- [**How To Enable**](#how-to-enable) | ||||||
- [**Test Plugin**](#test-plugin) | ||||||
- [**Disable Plugin**](#disable-plugin) | ||||||
|
||||||
## Description | ||||||
|
||||||
The `CSRF` plugin based on the `Double Submit Cookie` way, protect your API from CSRF attacks. This plugin considers the `GET`, `HEAD` and `OPTIONS` methods to be safe operations. Therefore calls to the `GET`, `HEAD` and `OPTIONS` methods are not checked for interception. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess it would be better to add a hyperlink at Double Submit Cookie. https://en.wikipedia.org/wiki/Cross-site_request_forgery#Double_Submit_Cookie |
||||||
|
||||||
In the following we define `GET`, `HEAD` and `OPTIONS` as the `safe-methods` and those other than these as `unsafe-methods`. | ||||||
|
||||||
## Attributes | ||||||
|
||||||
| Name | Type | Requirement | Default | Valid | Description | | ||||||
| ---------------- | ------- | ----------- | ------- | ----- | ------------------------------------------------------------ | | ||||||
| name | string | optional | `apisix-csrf-token` | | The name of the token in the generated cookie. | | ||||||
| expires | number | optional | `7200` | | Expiration time(s) of csrf cookie. | | ||||||
| key | string | required | | | The secret key used to encrypt the cookie. | | ||||||
|
||||||
## How To Enable | ||||||
|
||||||
1. Create the route and enable the plugin. | ||||||
|
||||||
```shell | ||||||
curl -i http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT-d ' | ||||||
{ | ||||||
"uri": "/hello", | ||||||
"plugins": { | ||||||
"csrf": { | ||||||
"key": "edd1c9f034335f136f87ad84b625c8f1" | ||||||
} | ||||||
}, | ||||||
"upstream": { | ||||||
"type": "roundrobin", | ||||||
"nodes": { | ||||||
"127.0.0.1:9001": 1 | ||||||
} | ||||||
} | ||||||
}' | ||||||
``` | ||||||
|
||||||
The route is then protected, and if you access it using methods other than `GET`, you will see that the request was blocked and receive a 401 status code back. | ||||||
|
||||||
2. Using `GET` requests `/hello`, a cookie with an encrypted token is received in the response. Token name is the `name` field set in the plugin configuration, if not set, the default value is `apisix-csrf-token`. | ||||||
|
||||||
Please note: We return a new cookie for each request. | ||||||
|
||||||
3. In subsequent unsafe-methods requests to this route, you need to read the encrypted token from the cookie and append the token to the `request header`, setting the field name to the `name` in the plugin configuration. | ||||||
|
||||||
## Test Plugin | ||||||
|
||||||
Direct access to the '/hello' route using a `POST` method will return an error: | ||||||
|
||||||
```shell | ||||||
curl -i http://127.0.0.1:9080/hello -X POST | ||||||
|
||||||
HTTP/1.1 401 Unauthorized | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can only show the status code? |
||||||
... | ||||||
{"error_msg":"no csrf token in headers"} | ||||||
``` | ||||||
|
||||||
When accessed with a GET request, the correct return and a cookie with an encrypted token are obtained: | ||||||
|
||||||
```shell | ||||||
curl -i http://127.0.0.1:9080/hello | ||||||
|
||||||
HTTP/1.1 200 OK | ||||||
Set-Cookie: apisix-csrf-token=eyJyYW5kb20iOjAuNjg4OTcyMzA4ODM1NDMsImV4cGlyZXMiOjcyMDAsInNpZ24iOiJcL09uZEF4WUZDZGYwSnBiNDlKREtnbzVoYkJjbzhkS0JRZXVDQm44MG9ldz0ifQ==;path=/;Expires=Mon, 13-Dec-21 09:33:55 GMT | ||||||
``` | ||||||
|
||||||
The token needs to be read from the cookie and carried in the request header in subsequent unsafe-methods requests. | ||||||
|
||||||
For example, use [js-cookie](https://github.com/js-cookie/js-cookie) read cookie and [axios](https://github.com/axios/axios) send request in client: | ||||||
|
||||||
```js | ||||||
const token = Cookie.get('apisix-csrf-token'); | ||||||
|
||||||
const instance = axios.create({ | ||||||
headers: {'apisix-csrf-token': token} | ||||||
}); | ||||||
``` | ||||||
|
||||||
You also need to make sure that you carry the cookie. | ||||||
|
||||||
Use curl send request: | ||||||
|
||||||
```shell | ||||||
curl -i http://127.0.0.1:9080/hello -X POST -H 'apisix-csrf-token: eyJyYW5kb20iOjAuNjg4OTcyMzA4ODM1NDMsImV4cGlyZXMiOjcyMDAsInNpZ24iOiJcL09uZEF4WUZDZGYwSnBiNDlKREtnbzVoYkJjbzhkS0JRZXVDQm44MG9ldz0ifQ==' -b 'apisix-csrf-token=eyJyYW5kb20iOjAuNjg4OTcyMzA4ODM1NDMsImV4cGlyZXMiOjcyMDAsInNpZ24iOiJcL09uZEF4WUZDZGYwSnBiNDlKREtnbzVoYkJjbzhkS0JRZXVDQm44MG9ldz0ifQ==' | ||||||
|
||||||
HTTP/1.1 200 OK | ||||||
``` | ||||||
|
||||||
## Disable Plugin | ||||||
|
||||||
Send a request to update the route to disable the plugin: | ||||||
|
||||||
```shell | ||||||
curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d ' | ||||||
{ | ||||||
"uri": "/hello", | ||||||
"upstream": { | ||||||
"type": "roundrobin", | ||||||
"nodes": { | ||||||
"127.0.0.1:1980": 1 | ||||||
} | ||||||
} | ||||||
}' | ||||||
``` | ||||||
|
||||||
CSRF plugin have been disabled. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For code style consistency can we use
ngx_cookie_time
instead ofcookie_time
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree