Skip to content

Commit 3973c95

Browse files
authored
feat: optional security enhancement (#7451)
1 parent cd5777a commit 3973c95

File tree

3 files changed

+134
-6
lines changed

3 files changed

+134
-6
lines changed

backend/helpers/pluginhelper/api/api_client.go

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,12 @@ import (
2525
"encoding/xml"
2626
"fmt"
2727
"io"
28+
"net"
2829
"net/http"
2930
"net/url"
3031
"reflect"
3132
"regexp"
33+
"strings"
3234
"sync"
3335
"time"
3436
"unicode/utf8"
@@ -43,9 +45,13 @@ import (
4345

4446
// ErrIgnoreAndContinue is a error which should be ignored
4547
var (
46-
ErrIgnoreAndContinue = errors.Default.New("ignore and continue")
47-
ErrEmptyResponse = errors.Default.New("empty response")
48-
_ plugin.ApiClient = (*ApiClient)(nil)
48+
ErrIgnoreAndContinue = errors.Default.New("ignore and continue")
49+
ErrEmptyResponse = errors.Default.New("empty response")
50+
ErrRedirectionNotAllowed = errors.BadInput.New("redirection is not allowed")
51+
ErrInvalidURL = errors.Default.New("Invalid URL")
52+
ErrInvalidCIDR = errors.Default.New("Invalid CIDR")
53+
ErrHostNotAllowed = errors.Default.New("Host is not allowed")
54+
_ plugin.ApiClient = (*ApiClient)(nil)
4955
)
5056

5157
// ApiClient is designed for simple api requests
@@ -103,6 +109,18 @@ func NewApiClient(
103109
proxy string,
104110
br context.BasicRes,
105111
) (*ApiClient, errors.Error) {
112+
cfg := br.GetConfigReader()
113+
log := br.GetLogger()
114+
115+
// endpoint blacklist
116+
endpointCidrBlacklist := cfg.GetString("ENDPOINT_CIDR_BLACKLIST")
117+
if endpointCidrBlacklist != "" {
118+
err := checkCidrBlacklist(endpointCidrBlacklist, endpoint, log)
119+
if err != nil {
120+
return nil, err
121+
}
122+
}
123+
106124
apiClient := &ApiClient{}
107125
apiClient.Setup(
108126
endpoint,
@@ -113,7 +131,7 @@ func NewApiClient(
113131
apiClient.client.Transport = &http.Transport{}
114132

115133
// set insecureSkipVerify
116-
insecureSkipVerify := br.GetConfigReader().GetBool("IN_SECURE_SKIP_VERIFY")
134+
insecureSkipVerify := cfg.GetBool("IN_SECURE_SKIP_VERIFY")
117135
if insecureSkipVerify {
118136
apiClient.client.Transport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
119137
}
@@ -155,6 +173,14 @@ func NewApiClient(
155173
}
156174
apiClient.SetContext(ctx)
157175

176+
// apply global security settings
177+
forbidRedirection := cfg.GetBool("FORBID_REDIRECTION")
178+
if forbidRedirection {
179+
apiClient.client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
180+
return ErrRedirectionNotAllowed
181+
}
182+
}
183+
158184
return apiClient, nil
159185
}
160186

@@ -454,3 +480,35 @@ func RemoveStartingSlashFromPath(relativePath string) string {
454480
}
455481
return relativePath
456482
}
483+
484+
func checkCidrBlacklist(blacklist, endpoint string, log log.Logger) errors.Error {
485+
// only if blacklist is given and the host of the endpoint is an IP address
486+
parsedEp, err := url.Parse(endpoint)
487+
if err != nil {
488+
return ErrInvalidURL
489+
}
490+
endpointHost := parsedEp.Hostname()
491+
if endpointHost == "" {
492+
return ErrInvalidURL
493+
}
494+
endpointIp := net.ParseIP(endpointHost)
495+
if endpointIp != nil {
496+
// check if the IP is in the blacklist
497+
cidrs := strings.Split(blacklist, ",")
498+
for _, cidr := range cidrs {
499+
// CIDR format : 10.0.0.1/24
500+
// check the net.ParseCIDR for details
501+
cidr = strings.TrimSpace(cidr)
502+
_, ipnet, err := net.ParseCIDR(cidr)
503+
if err != nil {
504+
// the CIDR is invalid
505+
log.Error(err, "Invalid CIDR", "cidr", cidr)
506+
return ErrInvalidCIDR
507+
}
508+
if ipnet.Contains(endpointIp) {
509+
return ErrHostNotAllowed
510+
}
511+
}
512+
}
513+
return nil
514+
}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/*
2+
Licensed to the Apache Software Foundation (ASF) under one or more
3+
contributor license agreements. See the NOTICE file distributed with
4+
this work for additional information regarding copyright ownership.
5+
The ASF licenses this file to You under the Apache License, Version 2.0
6+
(the "License"); you may not use this file except in compliance with
7+
the License. You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
*/
17+
18+
package api
19+
20+
import (
21+
"testing"
22+
23+
"github.com/apache/incubator-devlake/core/errors"
24+
"github.com/apache/incubator-devlake/impls/logruslog"
25+
"github.com/stretchr/testify/assert"
26+
)
27+
28+
func TestApiClientBlackList(t *testing.T) {
29+
for _, tc := range []struct {
30+
Name string
31+
Pattern string
32+
Endpoints []string
33+
Err errors.Error
34+
}{
35+
{
36+
Name: "Internal IP Addresses",
37+
Pattern: "10.0.0.1/16",
38+
Endpoints: []string{
39+
"https://10.0.0.1",
40+
"http://10.0.0.254",
41+
"http://10.0.254.1",
42+
"https://10.0.254.254",
43+
},
44+
Err: ErrHostNotAllowed,
45+
},
46+
{
47+
Name: "Internal IP Addresses",
48+
Pattern: "10.0.0.1/16",
49+
Endpoints: []string{
50+
"http://10.1.0.1",
51+
"http://10.1.0.254",
52+
"http://10.1.254.1",
53+
"http://10.1.254.254",
54+
},
55+
Err: nil,
56+
},
57+
} {
58+
t.Run(tc.Name, func(t *testing.T) {
59+
for _, endpoint := range tc.Endpoints {
60+
err := checkCidrBlacklist(tc.Pattern, endpoint, logruslog.Global)
61+
assert.Equal(t, tc.Err, err, "pattern %s and endpoint %s should return %v, but got %v", tc.Pattern, endpoint, tc.Err, err)
62+
}
63+
})
64+
}
65+
}

env.example

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,14 @@ DISABLED_REMOTE_PLUGINS=
5858
ENCRYPTION_SECRET=
5959

6060
##########################
61-
# Set if skip verify and connect with out trusted certificate when use https
61+
# Security settings
6262
##########################
63-
IN_SECURE_SKIP_VERIFY=
63+
# Set if skip verify and connect with out trusted certificate when use https
64+
IN_SECURE_SKIP_VERIFY=false
65+
# Forbid accessing sensity networks, CIDR form separated by comma: 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16
66+
ENDPOINT_CIDR_BLACKLIST=
67+
# Do not follow redirection when requesting data source APIs
68+
FORBID_REDIRECTION=false
6469

6570
##########################
6671
# In plugin gitextractor, use go-git to collector repo's data

0 commit comments

Comments
 (0)