Influxdata
/
influxdb
mirror of https://github.com/influxdata/influxdb.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

feat: add --hardening-enabled option to limit flux/pkger HTTP requests (#23207) 

Flux HTTP and template fetching requests do not perform IP address
checks for local addresses. This behavior on the one hand allows SSRF
(Server Side Request Forgery) attacks via authenticated requests but on
the other hand is useful for scenarios that have legitimate requirements
to fetch from private addresses (eg, hosting templates internally or
performing flux queries to local resources during development).

To not break existing installations, the default behavior will remain
the same but a new --hardening-enabled option is added to influxd to
turn on IP address verification and limit both flux and template
fetching HTTP requests to non-private addresses. We plan to enable new
security features that aren't suitable for the default install with this
option.  Put another way, this new option is intended to be used to make
it easy to turn on all security options when running in production
environments. The 'Manage security and authorization' section of the
docs will also be updated for this option.

Specifically for flux, when --hardening-enabled is specified, we now
pass in PrivateIPValidator{} to the flux dependency configuration. The
flux url validator will then tap into the http.Client 'Control'
mechanism to validate the IP address since it is called after DNS lookup
but before the connection starts.

For pkger (template fetching), when --hardening-enabled is specified,
the template parser's HTTP client will be configured to also use
PrivateIPValidator{}. Note that /api/v2/stacks POST ('init', aka create)
and PATCH ('update') only store the new url to be applied later with
/api/v2/templates/apply. While it is possible to have InitStack() and
UpdateStack() mimic net.DialContext() to setup a go routine to perform a
DNS lookup and then loop through the returned addresses to verify none
are for a private IP before storing the url, this would add considerable
complexity to the stacks implementation. Since the stack's urls are
fetched when it is applied and the IP address is verified as part of
apply (see above), for now we'll keep this simple and not validate the
IPs of the stack's urls during init or update.

Lastly, update pkger/http_server_template_test.go's Templates() test for
disabled jsonnet to also check the contents of the 422 error (since the
flux validator also returns a 422 with different message). Also, fix the
URL in one of these tests to use a valid path.
pull/23214/head
Jamie Strandboge 2022-03-18 09:25:31 -05:00 committed by GitHub
parent a40e12b615
commit 2c930fd127
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 185 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
cmd/influxd/launcher/cmd.go
View File

@ -187,6 +187,8 @@ type InfluxdOpts struct {
StorageConfig storage.Config
Viper *viper.Viper
HardeningEnabled bool
}
// NewOpts constructs options with default values.
@ -237,6 +239,8 @@ func NewOpts(viper *viper.Viper) *InfluxdOpts {
Testing: false,
TestingAlwaysAllowSetup: false,
HardeningEnabled: false,
}
}
@ -627,6 +631,24 @@ func (o *InfluxdOpts) BindCliOpts() []cli.Opt {
Default: o.UIDisabled,
Desc: "Disable the InfluxDB UI",
},
// hardening options
// --hardening-enabled is meant to enable all hardending
// options in one go. Today it enables the IP validator for
// flux and pkger templates HTTP requests. In the future,
// --hardening-enabled might be used to enable other security
// features, at which point we can add per-feature flags so
// that users can either opt into all features
// (--hardening-enabled) or to precisely the features they
// require. Since today there is but one feature, there is no
// need to introduce --hardening-ip-validation-enabled (or
// similar).
{
DestP: &o.HardeningEnabled,
Flag: "hardening-enabled",
Default: o.HardeningEnabled,
Desc: "enable hardening options (disallow private IPs within flux and templates HTTP requests)",
},
}
}

14
cmd/influxd/launcher/launcher.go
View File

@ -15,6 +15,7 @@ import (
"github.com/influxdata/flux"
"github.com/influxdata/flux/dependencies/testing"
"github.com/influxdata/flux/dependencies/url"
platform "github.com/influxdata/influxdb/v2"
"github.com/influxdata/influxdb/v2/annotations"
annotationTransport "github.com/influxdata/influxdb/v2/annotations/transport"
@ -386,6 +387,15 @@ func (m *Launcher) run(ctx context.Context, opts *InfluxdOpts) (err error) {
pointsWriter = replicationSvc
// When --hardening-enabled, use an HTTP IP validator that restricts
// flux and pkger HTTP requests to private addressess.
var urlValidator url.Validator
if opts.HardeningEnabled {
urlValidator = url.PrivateIPValidator{}
} else {
urlValidator = url.PassValidator{}
}
deps, err := influxdb.NewDependencies(
storageflux.NewReader(storage2.NewStore(m.engine.TSDBStore(), m.engine.MetaClient())),
pointsWriter,
@ -393,6 +403,7 @@ func (m *Launcher) run(ctx context.Context, opts *InfluxdOpts) (err error) {
authorizer.NewOrgService(ts.OrganizationService),
authorizer.NewSecretService(secretSvc),
nil,
influxdb.WithURLValidator(urlValidator),
)
if err != nil {
m.log.Error("Failed to get query controller dependencies", zap.Error(err))
@ -735,6 +746,7 @@ func (m *Launcher) run(ctx context.Context, opts *InfluxdOpts) (err error) {
authedUrmSVC := authorizer.NewURMService(b.OrgLookupService, b.UserResourceMappingService)
pkgerLogger := m.log.With(zap.String("service", "pkger"))
pkgSVC = pkger.NewService(
pkger.WithHTTPClient(pkger.NewDefaultHTTPClient(urlValidator)),
pkger.WithLogger(pkgerLogger),
pkger.WithStore(pkger.NewStoreKV(m.kvStore)),
pkger.WithBucketSVC(authorizer.NewBucketService(b.BucketService)),
@ -764,7 +776,7 @@ func (m *Launcher) run(ctx context.Context, opts *InfluxdOpts) (err error) {
var templatesHTTPServer *pkger.HTTPServerTemplates
{
tLogger := m.log.With(zap.String("handler", "templates"))
templatesHTTPServer = pkger.NewHTTPServerTemplates(tLogger, pkgSVC)
templatesHTTPServer = pkger.NewHTTPServerTemplates(tLogger, pkgSVC, pkger.NewDefaultHTTPClient(urlValidator))
}
userHTTPServer := ts.NewUserHTTPHandler(m.log)

10
pkger/http_server_template.go
View File

@ -30,14 +30,16 @@ type HTTPServerTemplates struct {
api *kithttp.API
logger *zap.Logger
svc SVC
client *http.Client
}
// NewHTTPServerTemplates constructs a new http server.
func NewHTTPServerTemplates(log *zap.Logger, svc SVC) *HTTPServerTemplates {
func NewHTTPServerTemplates(log *zap.Logger, svc SVC, client *http.Client) *HTTPServerTemplates {
svr := &HTTPServerTemplates{
api: kithttp.NewAPI(kithttp.WithLog(log)),
logger: log,
svc: svc,
client: client,
}
exportAllowContentTypes := middleware.AllowContentType("text/yml", "application/x-yaml", "application/json")
@ -212,13 +214,13 @@ type ReqApply struct {
}
// Templates returns all templates associated with the request.
func (r ReqApply) Templates(encoding Encoding) (*Template, error) {
func (r ReqApply) Templates(encoding Encoding, client *http.Client) (*Template, error) {
var rawTemplates []*Template
for _, rem := range r.Remotes {
if rem.URL == "" {
continue
}
template, err := Parse(rem.Encoding(), FromHTTPRequest(rem.URL), ValidSkipParseError())
template, err := Parse(rem.Encoding(), FromHTTPRequest(rem.URL, client), ValidSkipParseError())
if err != nil {
msg := fmt.Sprintf("template from url[%s] had an issue: %s", rem.URL, err.Error())
return nil, influxErr(errors.EUnprocessableEntity, msg)
@ -386,7 +388,7 @@ func (s *HTTPServerTemplates) apply(w http.ResponseWriter, r *http.Request) {
}
}
parsedTemplate, err := reqBody.Templates(encoding)
parsedTemplate, err := reqBody.Templates(encoding, s.client)
if err != nil {
s.api.Err(w, r, &errors.Error{
Code: errors.EUnprocessableEntity,

100
pkger/http_server_template_test.go
View File

@ -15,6 +15,7 @@ import (
"testing"
"github.com/go-chi/chi"
fluxurl "github.com/influxdata/flux/dependencies/url"
"github.com/influxdata/influxdb/v2"
pcontext "github.com/influxdata/influxdb/v2/context"
"github.com/influxdata/influxdb/v2/kit/platform"
@ -43,6 +44,8 @@ func TestPkgerHTTPServerTemplate(t *testing.T) {
filesvr := httptest.NewServer(mux)
defer filesvr.Close()
defaultClient := pkger.NewDefaultHTTPClient(fluxurl.PassValidator{})
newPkgURL := func(t *testing.T, svrURL string, pkgPath string) string {
t.Helper()
@ -65,7 +68,7 @@ func TestPkgerHTTPServerTemplate(t *testing.T) {
}, nil
}
svc := pkger.NewService(pkger.WithLabelSVC(fakeLabelSVC))
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc)
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc, defaultClient)
svr := newMountedHandler(pkgHandler, 1)
testttp.
@ -95,7 +98,7 @@ func TestPkgerHTTPServerTemplate(t *testing.T) {
})
t.Run("should be invalid if not org ids or resources provided", func(t *testing.T) {
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), nil)
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), nil, defaultClient)
svr := newMountedHandler(pkgHandler, 1)
testttp.
@ -177,7 +180,7 @@ func TestPkgerHTTPServerTemplate(t *testing.T) {
}
core, sink := observer.New(zap.InfoLevel)
pkgHandler := pkger.NewHTTPServerTemplates(zap.New(core), svc)
pkgHandler := pkger.NewHTTPServerTemplates(zap.New(core), svc, defaultClient)
svr := newMountedHandler(pkgHandler, 1)
ctx := context.Background()
@ -308,7 +311,7 @@ func TestPkgerHTTPServerTemplate(t *testing.T) {
}
core, _ := observer.New(zap.InfoLevel)
pkgHandler := pkger.NewHTTPServerTemplates(zap.New(core), svc)
pkgHandler := pkger.NewHTTPServerTemplates(zap.New(core), svc, defaultClient)
svr := newMountedHandler(pkgHandler, 1)
testttp.
@ -375,7 +378,7 @@ func TestPkgerHTTPServerTemplate(t *testing.T) {
},
}
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc)
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc, defaultClient)
svr := newMountedHandler(pkgHandler, 1)
body := newReqApplyYMLBody(t, platform.ID(9000), true)
@ -406,7 +409,7 @@ func TestPkgerHTTPServerTemplate(t *testing.T) {
},
}
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc)
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc, defaultClient)
svr := newMountedHandler(pkgHandler, 1)
testttp.
@ -523,7 +526,7 @@ func TestPkgerHTTPServerTemplate(t *testing.T) {
},
}
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc)
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc, defaultClient)
svr := newMountedHandler(pkgHandler, 1)
testttp.
@ -593,7 +596,7 @@ func TestPkgerHTTPServerTemplate(t *testing.T) {
},
}
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc)
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc, defaultClient)
svr := newMountedHandler(pkgHandler, 1)
testttp.
@ -609,12 +612,12 @@ func TestPkgerHTTPServerTemplate(t *testing.T) {
t.Run("resp apply err response", func(t *testing.T) {
tests := []struct {
name string
name string
contentType string
reqBody pkger.ReqApply
reqBody pkger.ReqApply
}{
{
name: "invalid json",
name: "invalid json",
contentType: "application/json",
reqBody: pkger.ReqApply{
DryRun: true,
@ -655,7 +658,7 @@ func TestPkgerHTTPServerTemplate(t *testing.T) {
},
}
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc)
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc, defaultClient)
svr := newMountedHandler(pkgHandler, 1)
testttp.
@ -713,7 +716,7 @@ func TestPkgerHTTPServerTemplate(t *testing.T) {
},
}
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc)
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc, defaultClient)
svr := newMountedHandler(pkgHandler, 1)
testttp.
@ -743,7 +746,7 @@ func TestPkgerHTTPServerTemplate(t *testing.T) {
},
}
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc)
pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc, defaultClient)
svr := newMountedHandler(pkgHandler, 1)
testttp.
@ -781,7 +784,7 @@ func TestPkgerHTTPServerTemplate(t *testing.T) {
reqBody: pkger.ReqApply{
OrgID: platform.ID(9000).String(),
Remotes: []pkger.ReqTemplateRemote{{
URL: newPkgURL(t, filesvr.URL, "testdata/bucket_associates_labels_one.jsonnet"),
URL: newPkgURL(t, filesvr.URL, "testdata/bucket_associates_labels.jsonnet"),
}},
},
encoding: pkger.EncodingJsonnet,
@ -797,10 +800,75 @@ func TestPkgerHTTPServerTemplate(t *testing.T) {
}
for _, tt := range tests {
tmpl, err := tt.reqBody.Templates(tt.encoding)
tmpl, err := tt.reqBody.Templates(tt.encoding, defaultClient)
assert.Nil(t, tmpl)
require.Error(t, err)
assert.Equal(t, "unprocessable entity", influxerror.ErrorCode(err))
assert.Contains(t, influxerror.ErrorMessage(err), "invalid encoding provided: jsonnet")
}
})
t.Run("Templates() remotes with IP validation", func(t *testing.T) {
tests := []struct {
name string
client *http.Client
expCode int
expErr string
}{
{
name: "no filter ip",
client: pkger.NewDefaultHTTPClient(fluxurl.PassValidator{}),
expCode: http.StatusOK,
expErr: "",
},
{
name: "filter ip",
client: pkger.NewDefaultHTTPClient(fluxurl.PrivateIPValidator{}),
expCode: http.StatusUnprocessableEntity,
expErr: "no such host",
},
}
svc := &fakeSVC{
dryRunFn: func(ctx context.Context, orgID, userID platform.ID, opts ...pkger.ApplyOptFn) (pkger.ImpactSummary, error) {
return pkger.ImpactSummary{}, nil
},
}
for _, tt := range tests {
core, sink := observer.New(zap.InfoLevel)
pkgHandler := pkger.NewHTTPServerTemplates(zap.New(core), svc, tt.client)
svr := newMountedHandler(pkgHandler, 1)
reqBody := pkger.ReqApply{
DryRun: true,
OrgID: platform.ID(9000).String(),
Remotes: []pkger.ReqTemplateRemote{{
URL: newPkgURL(t, filesvr.URL, "testdata/remote_bucket.json"),
}},
}
ctx := context.Background()
testttp.
PostJSON(t, "/api/v2/templates/apply", reqBody).
Headers("Content-Type", "application/json").
WithCtx(ctx).
Do(svr).
ExpectStatus(tt.expCode).
ExpectBody(func(buf *bytes.Buffer) {
var resp pkger.RespApply
decodeBody(t, buf, &resp)
assert.Len(t, resp.Summary.Buckets, 0)
assert.Len(t, resp.Diff.Buckets, 0)
})
if tt.expErr != "" {
// Verify logging output has the expected generic flux message
entries := sink.TakeAll() // resets to 0
fmt.Printf("%+v\n", entries)
require.Equal(t, 1, len(entries))
assert.Contains(t, fmt.Sprintf("%s", entries[0].Context[0].Interface), tt.expErr)
}
}
})
}

35
pkger/parser.go
View File

@ -7,6 +7,7 @@ import (
"fmt"
"io"
"io/ioutil"
"net"
"net/http"
"net/url"
"path"
@ -14,10 +15,12 @@ import (
"sort"
"strconv"
"strings"
"syscall"
"time"
"github.com/influxdata/flux/ast"
"github.com/influxdata/flux/ast/edit"
fluxurl "github.com/influxdata/flux/dependencies/url"
"github.com/influxdata/flux/parser"
errors2 "github.com/influxdata/influxdb/v2/kit/platform/errors"
"github.com/influxdata/influxdb/v2/pkg/jsonnet"
@ -145,15 +148,39 @@ func FromString(s string) ReaderFn {
}
}
var defaultHTTPClient = &http.Client{
Timeout: time.Minute,
// NewDefaultHTTPClient creates a client with the specified flux IP validator.
// This is copied from flux/dependencies/http/http.go
func NewDefaultHTTPClient(urlValidator fluxurl.Validator) *http.Client {
// Control is called after DNS lookup, but before the network
// connection is initiated.
control := func(network, address string, c syscall.RawConn) error {
host, _, err := net.SplitHostPort(address)
if err != nil {
return err
}
ip := net.ParseIP(host)
return urlValidator.ValidateIP(ip)
}
dialer := &net.Dialer{
Timeout: time.Minute,
Control: control,
// DualStack is deprecated
}
return &http.Client{
Transport: &http.Transport{
DialContext: dialer.DialContext,
},
}
}
// FromHTTPRequest parses a pkg from the request body of a HTTP request. This is
// very useful when using packages that are hosted..
func FromHTTPRequest(addr string) ReaderFn {
func FromHTTPRequest(addr string, client *http.Client) ReaderFn {
return func() (io.Reader, string, error) {
resp, err := defaultHTTPClient.Get(normalizeGithubURLToContent(addr))
resp, err := client.Get(normalizeGithubURLToContent(addr))
if err != nil {
return nil, addr, err
}

13
pkger/service.go
View File

@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"net/http"
"net/url"
"path"
"regexp"
@ -149,6 +150,7 @@ type serviceOpt struct {
logger *zap.Logger
applyReqLimit int
client *http.Client
idGen platform.IDGenerator
nameGen NameGenerator
timeGen influxdb.TimeGenerator
@ -170,6 +172,13 @@ type serviceOpt struct {
// ServiceSetterFn is a means of setting dependencies on the Service type.
type ServiceSetterFn func(opt *serviceOpt)
// WithHTTPClient sets the http client for the service.
func WithHTTPClient(c *http.Client) ServiceSetterFn {
return func(o *serviceOpt) {
o.client = c
}
}
// WithLogger sets the logger for the service.
func WithLogger(log *zap.Logger) ServiceSetterFn {
return func(o *serviceOpt) {
@ -297,6 +306,7 @@ type Service struct {
// internal dependencies
applyReqLimit int
client *http.Client
idGen platform.IDGenerator
nameGen NameGenerator
store Store
@ -335,6 +345,7 @@ func NewService(opts ...ServiceSetterFn) *Service {
log: opt.logger,
applyReqLimit: opt.applyReqLimit,
client: opt.client,
idGen: opt.idGen,
nameGen: opt.nameGen,
store: opt.store,
@ -3088,7 +3099,7 @@ func (s *Service) getStackRemoteTemplates(ctx context.Context, stackID platform.
encoding = EncodingYAML
}
readerFn := FromHTTPRequest(u.String())
readerFn := FromHTTPRequest(u.String(), s.client)
if u.Scheme == "file" {
readerFn = FromFile(u.Path)
}

15
query/stdlib/influxdata/influxdb/dependencies.go
View File

@ -79,6 +79,15 @@ func (d Dependencies) PrometheusCollectors() []prometheus.Collector {
return collectors
}
type FluxDepOption func(*flux.Deps)
func WithURLValidator(v url.Validator) FluxDepOption {
return func(d *flux.Deps) {
d.Deps.URLValidator = v
d.Deps.HTTPClient = http.NewDefaultClient(d.Deps.URLValidator)
}
}
func NewDependencies(
reader query.StorageReader,
writer storage.PointsWriter,
@ -86,10 +95,16 @@ func NewDependencies(
orgSvc influxdb.OrganizationService,
ss influxdb.SecretService,
metricLabelKeys []string,
fluxopts ...FluxDepOption,
) (Dependencies, error) {
fdeps := flux.NewDefaultDependencies()
fdeps.Deps.HTTPClient = http.NewDefaultClient(url.PassValidator{})
fdeps.Deps.SecretService = query.FromSecretService(ss)
// apply fluxopts before assigning fdeps to deps (ie, before casting)
for _, opt := range fluxopts {
opt(&fdeps)
}
deps := Dependencies{FluxDeps: fdeps}
bucketLookupSvc := query.FromBucketService(bucketSvc)
orgLookupSvc := query.FromOrganizationService(orgSvc)

3
task/backend/analytical_storage_test.go
View File

@ -9,6 +9,7 @@ import (
"github.com/golang/mock/gomock"
"github.com/influxdata/flux"
"github.com/influxdata/flux/dependencies/url"
"github.com/influxdata/influxdb/v2"
"github.com/influxdata/influxdb/v2/authorization"
icontext "github.com/influxdata/influxdb/v2/context"
@ -208,7 +209,7 @@ func newAnalyticalBackend(t *testing.T, orgSvc influxdb.OrganizationService, buc
storageStore := storage2.NewStore(engine.TSDBStore(), engine.MetaClient())
readsReader := storageflux.NewReader(storageStore)
deps, err := stdlib.NewDependencies(readsReader, engine, bucketSvc, orgSvc, nil, nil)
deps, err := stdlib.NewDependencies(readsReader, engine, bucketSvc, orgSvc, nil, nil, stdlib.WithURLValidator(url.PassValidator{}))
if err != nil {
t.Fatal(err)
}