From 931376bbfeb3f77516fb80d360e79b9fb6276674 Mon Sep 17 00:00:00 2001 From: Klesh Wong Date: Fri, 10 May 2024 15:49:19 +0800 Subject: [PATCH] feat: optional security enhancement --- .../helpers/pluginhelper/api/api_client.go | 66 +++++++++++++++++-- .../pluginhelper/api/api_client_test.go | 65 ++++++++++++++++++ env.example | 9 ++- 3 files changed, 134 insertions(+), 6 deletions(-) create mode 100644 backend/helpers/pluginhelper/api/api_client_test.go diff --git a/backend/helpers/pluginhelper/api/api_client.go b/backend/helpers/pluginhelper/api/api_client.go index e69a8868034..5e1535de27d 100644 --- a/backend/helpers/pluginhelper/api/api_client.go +++ b/backend/helpers/pluginhelper/api/api_client.go @@ -25,10 +25,12 @@ import ( "encoding/xml" "fmt" "io" + "net" "net/http" "net/url" "reflect" "regexp" + "strings" "sync" "time" "unicode/utf8" @@ -43,9 +45,13 @@ import ( // ErrIgnoreAndContinue is a error which should be ignored var ( - ErrIgnoreAndContinue = errors.Default.New("ignore and continue") - ErrEmptyResponse = errors.Default.New("empty response") - _ plugin.ApiClient = (*ApiClient)(nil) + ErrIgnoreAndContinue = errors.Default.New("ignore and continue") + ErrEmptyResponse = errors.Default.New("empty response") + ErrRedirectionNotAllowed = errors.BadInput.New("redirection is not allowed") + ErrInvalidURL = errors.Default.New("Invalid URL") + ErrInvalidCIDR = errors.Default.New("Invalid CIDR") + ErrHostNotAllowed = errors.Default.New("Host is not allowed") + _ plugin.ApiClient = (*ApiClient)(nil) ) // ApiClient is designed for simple api requests @@ -103,6 +109,18 @@ func NewApiClient( proxy string, br context.BasicRes, ) (*ApiClient, errors.Error) { + cfg := br.GetConfigReader() + log := br.GetLogger() + + // endpoint blacklist + endpointCidrBlacklist := cfg.GetString("ENDPOINT_CIDR_BLACKLIST") + if endpointCidrBlacklist != "" { + err := checkCidrBlacklist(endpointCidrBlacklist, endpoint, log) + if err != nil { + return nil, err + } + } + apiClient := &ApiClient{} apiClient.Setup( endpoint, @@ -113,7 +131,7 @@ func NewApiClient( apiClient.client.Transport = &http.Transport{} // set insecureSkipVerify - insecureSkipVerify := br.GetConfigReader().GetBool("IN_SECURE_SKIP_VERIFY") + insecureSkipVerify := cfg.GetBool("IN_SECURE_SKIP_VERIFY") if insecureSkipVerify { apiClient.client.Transport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true} } @@ -155,6 +173,14 @@ func NewApiClient( } apiClient.SetContext(ctx) + // apply global security settings + forbidRedirection := cfg.GetBool("FORBID_REDIRECTION") + if forbidRedirection { + apiClient.client.CheckRedirect = func(req *http.Request, via []*http.Request) error { + return ErrRedirectionNotAllowed + } + } + return apiClient, nil } @@ -454,3 +480,35 @@ func RemoveStartingSlashFromPath(relativePath string) string { } return relativePath } + +func checkCidrBlacklist(blacklist, endpoint string, log log.Logger) errors.Error { + // only if blacklist is given and the host of the endpoint is an IP address + parsedEp, err := url.Parse(endpoint) + if err != nil { + return ErrInvalidURL + } + endpointHost := parsedEp.Hostname() + if endpointHost == "" { + return ErrInvalidURL + } + endpointIp := net.ParseIP(endpointHost) + if endpointIp != nil { + // check if the IP is in the blacklist + cidrs := strings.Split(blacklist, ",") + for _, cidr := range cidrs { + // CIDR format : 10.0.0.1/24 + // check the net.ParseCIDR for details + cidr = strings.TrimSpace(cidr) + _, ipnet, err := net.ParseCIDR(cidr) + if err != nil { + // the CIDR is invalid + log.Error(err, "Invalid CIDR", "cidr", cidr) + return ErrInvalidCIDR + } + if ipnet.Contains(endpointIp) { + return ErrHostNotAllowed + } + } + } + return nil +} diff --git a/backend/helpers/pluginhelper/api/api_client_test.go b/backend/helpers/pluginhelper/api/api_client_test.go new file mode 100644 index 00000000000..361dc619d49 --- /dev/null +++ b/backend/helpers/pluginhelper/api/api_client_test.go @@ -0,0 +1,65 @@ +/* +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. +*/ + +package api + +import ( + "testing" + + "github.com/apache/incubator-devlake/core/errors" + "github.com/apache/incubator-devlake/impls/logruslog" + "github.com/stretchr/testify/assert" +) + +func TestApiClientBlackList(t *testing.T) { + for _, tc := range []struct { + Name string + Pattern string + Endpoints []string + Err errors.Error + }{ + { + Name: "Internal IP Addresses", + Pattern: "10.0.0.1/16", + Endpoints: []string{ + "https://10.0.0.1", + "http://10.0.0.254", + "http://10.0.254.1", + "https://10.0.254.254", + }, + Err: ErrHostNotAllowed, + }, + { + Name: "Internal IP Addresses", + Pattern: "10.0.0.1/16", + Endpoints: []string{ + "http://10.1.0.1", + "http://10.1.0.254", + "http://10.1.254.1", + "http://10.1.254.254", + }, + Err: nil, + }, + } { + t.Run(tc.Name, func(t *testing.T) { + for _, endpoint := range tc.Endpoints { + err := checkCidrBlacklist(tc.Pattern, endpoint, logruslog.Global) + assert.Equal(t, tc.Err, err, "pattern %s and endpoint %s should return %v, but got %v", tc.Pattern, endpoint, tc.Err, err) + } + }) + } +} diff --git a/env.example b/env.example index bb8bafc099a..b45799356ac 100755 --- a/env.example +++ b/env.example @@ -58,9 +58,14 @@ DISABLED_REMOTE_PLUGINS= ENCRYPTION_SECRET= ########################## -# Set if skip verify and connect with out trusted certificate when use https +# Security settings ########################## -IN_SECURE_SKIP_VERIFY= +# Set if skip verify and connect with out trusted certificate when use https +IN_SECURE_SKIP_VERIFY=false +# Forbid accessing sensity networks, CIDR form separated by comma: 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16 +ENDPOINT_CIDR_BLACKLIST= +# Do not follow redirection when requesting data source APIs +FORBID_REDIRECTION=false ########################## # In plugin gitextractor, use go-git to collector repo's data