Skip to content
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 security header #2341

Merged
merged 3 commits into from
Mar 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions api/conf/conf.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@ conf:
# such as absolute path on Windows: winfile:///C:\access.log
# log example: 2020-12-09T16:38:09.039+0800 INFO filter/logging.go:46 /apisix/admin/routes/r1 {"status": 401, "host": "127.0.0.1:9000", "query": "asdfsafd=adf&a=a", "requestId": "3d50ecb8-758c-46d1-af5b-cd9d1c820156", "latency": 0, "remoteIP": "127.0.0.1", "method": "PUT", "errs": []}
max_cpu: 0 # supports tweaking with the number of OS threads are going to be used for parallelism. Default value: 0 [will use max number of available cpu cores considering hyperthreading (if any)]. If the value is negative, is will not touch the existing parallelism profile.
# security:
# access_control_allow_origin: "http://httpbin.org"
# access_control_allow_credentials: true # support using custom cors configration
# access_control_allow_headers: "Authorization"
# access_control-allow_methods: "*"
# x_frame_options: "deny"
# content_security_policy: ""default-src 'self'; script-src 'self' 'unsafe-eval'; style-src 'self' 'unsafe-inline'""


authentication:
secret:
Expand Down
35 changes: 35 additions & 0 deletions api/internal/conf/conf.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ var (
ImportSizeLimit = 10 * 1024 * 1024
AllowList []string
Plugins = map[string]bool{}
SecurityConf Security
)

type MTLS struct {
Expand Down Expand Up @@ -110,6 +111,7 @@ type Conf struct {
Log Log
AllowList []string `mapstructure:"allow_list"`
MaxCpu int `mapstructure:"max_cpu"`
Security Security
}

type User struct {
Expand All @@ -129,6 +131,15 @@ type Config struct {
Plugins []string
}

type Security struct {
AllowCredentials string `mapstructure:"access_control_allow_credentials"`
AllowOrigin string `mapstructure:"access_control_allow_origin"`
AllowMethods string `mapstructure:"access_control-allow_methods"`
AllowHeaders string `mapstructure:"access_control_allow_headers"`
XFrameOptions string `mapstructure:"x_frame_options"`
ContentSecurityPolicy string `mapstructure:"content_security_policy"`
}

// TODO: we should no longer use init() function after remove all handler's integration tests
// ENV=test is for integration tests only, other ENV should call "InitConf" explicitly
func init() {
Expand Down Expand Up @@ -246,6 +257,9 @@ func setupConfig() {

// set plugin
initPlugins(config.Plugins)

// security configuration
initSecurity(config.Conf.Security)
}

func setupEnv() {
Expand Down Expand Up @@ -316,3 +330,24 @@ func initParallelism(choiceCores int) {
}
runtime.GOMAXPROCS(choiceCores)
}

// initialize security settings
func initSecurity(conf Security) {
var se Security
// if conf == se, then conf is empty, we should use default value
if conf != se {
SecurityConf = conf
if conf.ContentSecurityPolicy == "" {
SecurityConf.ContentSecurityPolicy = "default-src 'self'; script-src 'self' 'unsafe-eval'; style-src 'self' 'unsafe-inline'"
}
if conf.XFrameOptions == "" {
SecurityConf.XFrameOptions = "deny"
}
return
}

SecurityConf = Security{
XFrameOptions: "deny",
ContentSecurityPolicy: "default-src 'self'; script-src 'self' 'unsafe-eval'; style-src 'self' 'unsafe-inline'",
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we don't have the default value of AllowCredentials AllowOrigin AllowMethods AllowHeaders

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need default value for AllowCredentialsAlloOriginAllowMethodsAllowHeaders. These configuration are left to the user to customize.

}
33 changes: 28 additions & 5 deletions api/internal/filter/cors.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,37 @@
*/
package filter

import "github.com/gin-gonic/gin"
import (
"github.com/gin-gonic/gin"

"github.com/apisix/manager-api/internal/conf"
)

func CORS() gin.HandlerFunc {
return func(c *gin.Context) {
c.Writer.Header().Set("Access-Control-Allow-Origin", "*")
c.Writer.Header().Set("Access-Control-Allow-Credentials", "true")
c.Writer.Header().Set("Access-Control-Allow-Headers", "Authorization")
c.Writer.Header().Set("Access-Control-Allow-Methods", "*")
if conf.SecurityConf.AllowOrigin != "" {
c.Writer.Header().Set("Access-Control-Allow-Origin", conf.SecurityConf.AllowOrigin)
}

if conf.SecurityConf.AllowHeaders != "" {
c.Writer.Header().Set("Access-Control-Allow-Headers", conf.SecurityConf.AllowHeaders)
}

if conf.SecurityConf.AllowMethods != "" {
c.Writer.Header().Set("Access-Control-Allow-Methods", conf.SecurityConf.AllowMethods)
}

if conf.SecurityConf.AllowCredentials != "" {
c.Writer.Header().Set("Access-Control-Allow-Credentials", conf.SecurityConf.AllowCredentials)
}

if conf.SecurityConf.XFrameOptions != "" {
c.Writer.Header().Set("X-Frame-Options", conf.SecurityConf.XFrameOptions)
}

if conf.SecurityConf.ContentSecurityPolicy != "" {
c.Writer.Header().Set("Content-Security-Policy", conf.SecurityConf.ContentSecurityPolicy)
}
if c.Request.Method == "OPTIONS" {
c.AbortWithStatus(204)
return
Expand Down
26 changes: 26 additions & 0 deletions api/test/shell/cli_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,32 @@ stop_dashboard() {
recover_service_file
}

#15
@test "Check Security configuration" {
recover_conf

start_dashboard 3

# check response header without custom header
run curl -i http://127.0.0.1:9000

[ $(echo "$output" | grep -c "X-Frame-Options: deny") -eq '1' ]

stop_dashboard 6

sed -i 's@# security:@security:@' ${CONF_FILE}
sed -i 's@# x_frame_options: "deny"@ x_frame_options: "test"@' ${CONF_FILE}

start_dashboard 3

# check response header with custom header
run curl -i http://127.0.0.1:9000

[ $(echo "$output" | grep -c "X-Frame-Options: test") -eq '1' ]

stop_dashboard 6
}

#post
@test "Clean test environment" {
# kill etcd
Expand Down