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: disable file:// urls when hardening enabled #24858

Merged
merged 17 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 17 additions & 4 deletions cmd/influxd/launcher/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,11 @@ type InfluxdOpts struct {

Viper *viper.Viper

// HardeningEnabled toggles multiple best-practice hardening options on.
HardeningEnabled bool
StrongPasswords bool
// TemplateFileUrlsDisabled disables file protocol URIs in templates.
TemplateFileUrlsDisabled bool
StrongPasswords bool
}

// NewOpts constructs options with default values.
Expand Down Expand Up @@ -243,8 +246,9 @@ func NewOpts(viper *viper.Viper) *InfluxdOpts {
Testing: false,
TestingAlwaysAllowSetup: false,

HardeningEnabled: false,
StrongPasswords: false,
HardeningEnabled: false,
TemplateFileUrlsDisabled: false,
StrongPasswords: false,
}
}

Expand Down Expand Up @@ -657,7 +661,16 @@ func (o *InfluxdOpts) BindCliOpts() []cli.Opt {
DestP: &o.HardeningEnabled,
Flag: "hardening-enabled",
Default: o.HardeningEnabled,
Desc: "enable hardening options (disallow private IPs within flux and templates HTTP requests)",
Desc: "enable hardening options (disallow private IPs within flux and templates HTTP requests, template file URLs)",
jdstrand marked this conversation as resolved.
Show resolved Hide resolved
},

// --template-file-urls-disabled prevents file protocol URIs
// from being used for templates.
{
DestP: &o.TemplateFileUrlsDisabled,
Flag: "template-file-urls-disabled",
Default: o.TemplateFileUrlsDisabled,
Desc: "disable template file URLs",
},
{
DestP: &o.StrongPasswords,
Expand Down
2 changes: 2 additions & 0 deletions cmd/influxd/launcher/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -752,8 +752,10 @@ func (m *Launcher) run(ctx context.Context, opts *InfluxdOpts) (err error) {
authedOrgSVC := authorizer.NewOrgService(b.OrganizationService)
authedUrmSVC := authorizer.NewURMService(b.OrgLookupService, b.UserResourceMappingService)
pkgerLogger := m.log.With(zap.String("service", "pkger"))
disableFileUrls := opts.HardeningEnabled || opts.TemplateFileUrlsDisabled
pkgSVC = pkger.NewService(
pkger.WithHTTPClient(pkger.NewDefaultHTTPClient(urlValidator)),
pkger.WithFileUrlsDisabled(disableFileUrls),
pkger.WithLogger(pkgerLogger),
pkger.WithStore(pkger.NewStoreKV(m.kvStore)),
pkger.WithBucketSVC(authorizer.NewBucketService(b.BucketService)),
Expand Down
13 changes: 13 additions & 0 deletions pkg/fs/special.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//go:build !linux
// +build !linux

package fs

import "io/fs"

// IsSpecialFSFromFileInfo determines if a file resides on a special file
// system (e.g. /proc, /dev/, /sys) based on its fs.FileInfo.
// The bool return value should be ignored if err is not nil.
func IsSpecialFSFromFileInfo(st fs.FileInfo) (bool, error) {
return false, nil
}
63 changes: 63 additions & 0 deletions pkg/fs/special_linux.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package fs

import (
"errors"
"io/fs"
"os"
"syscall"

"golang.org/x/sys/unix"
)

// IsSpecialFSFromFileInfo determines if a file resides on a special file
// system (e.g. /proc, /dev/, /sys) based on its fs.FileInfo.
// The bool return value should be ignored if err is not nil.
func IsSpecialFSFromFileInfo(st fs.FileInfo) (bool, error) {
jdstrand marked this conversation as resolved.
Show resolved Hide resolved
// On Linux, special file systems like /proc, /dev/, and /sys are
// considered unnamed devices (non-device mounts). These devices
// will always have a major device number of 0 per the kernels
// Documentation/devices.txt file.
jdstrand marked this conversation as resolved.
Show resolved Hide resolved

getDevId := func(st fs.FileInfo) (uint64, error) {
st_sys_any := st.Sys()
if st_sys_any == nil {
return 0, errors.New("nil returned by fs.FileInfo.Sys")
}

st_sys, ok := st_sys_any.(*syscall.Stat_t)
if !ok {
return 0, errors.New("could not convert st.sys() to a *syscall.Stat_t")
davidby-influx marked this conversation as resolved.
Show resolved Hide resolved
}
return st_sys.Dev, nil
}

devId, err := getDevId(st)
if err != nil {
return false, err
}
if unix.Major(devId) != 0 {
// This file is definitely not on a special file system.
return false, nil
}

// We know the file is in a special file system, but we'll make an
// exception for tmpfs, which some distros use for /tmp.
// Since the minor IDs are assigned dynamically, we'll find the device ID
// for /tmp. If /tmp's device ID matches this st's, then it is safe to assume
// the file is in tmpfs. Otherwise, it is a special device that is not tmpfs.
// Note that if /tmp is not tmpfs, then the device IDs won't match since
// tmp would be a standard file system.
jdstrand marked this conversation as resolved.
Show resolved Hide resolved
tmpSt, err := os.Stat(os.TempDir())
if err != nil {
// We got an error getting stats on /tmp, but that just means we can't
// say the file is in tmpfs. We'll still go ahead and call it a special file.
return true, nil
}
tmpDevId, err := getDevId(tmpSt)
if err != nil {
// See above for why we're returning an error here.
return true, nil
}
// It's a special file unless the device ID matches /tmp's ID.
return devId != tmpDevId, nil
gwossum marked this conversation as resolved.
Show resolved Hide resolved
}
82 changes: 76 additions & 6 deletions pkger/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
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/fs"
"github.com/influxdata/influxdb/v2/pkg/jsonnet"
"github.com/influxdata/influxdb/v2/task/options"
"gopkg.in/yaml.v3"
Expand Down Expand Up @@ -102,8 +103,71 @@ func Parse(encoding Encoding, readerFn ReaderFn, opts ...ValidateOptFn) (*Templa
return pkg, nil
}

// FromFile reads a file from disk and provides a reader from it.
func FromFile(filePath string) ReaderFn {
// limitReadFileMaxSize is the maximum file size that limitReadFile will read.
const limitReadFileMaxSize int64 = 2 * 1024 * 1024

// limitReadFile operates like ioutil.ReadFile() in that it reads the contents
// of a file into RAM, but will only read regular files up to the specified
// max. limitReadFile reads the file named by filename and returns the
// contents. A successful call returns err == nil, not err == EOF. Because
// limitReadFile reads the whole file, it does not treat an EOF from Read as an
// error to be reported.
//
// Ultimately, only remocal will have file:// URLs enabled for its 'data
// catalogs' bootstrap functionality. At present, the largest catalog is 40k,
// so set a small max here with a bit of headroom.
jdstrand marked this conversation as resolved.
Show resolved Hide resolved
func limitReadFile(name string) ([]byte, error) {
// use os.Open() to avoid TOCTOU
f, err := os.Open(name)
if err != nil {
return nil, err
}
defer f.Close()
gwossum marked this conversation as resolved.
Show resolved Hide resolved

// Check that properties of file are OK.
st, err := f.Stat()
if err != nil {
return nil, err
}

// Disallow reading from special file systems (e.g. /proc, /sys/, /dev).
if special, err := fs.IsSpecialFSFromFileInfo(st); err != nil {
return nil, err
} else if special {
return nil, errors.New("file in special file system")
gwossum marked this conversation as resolved.
Show resolved Hide resolved
}

// only support reading regular files
if st.Mode()&os.ModeType != 0 {
return nil, errors.New("not a regular file")
gwossum marked this conversation as resolved.
Show resolved Hide resolved
}

// limit how much we read into RAM
var size int
size64 := st.Size()
if size64 > limitReadFileMaxSize {
gwossum marked this conversation as resolved.
Show resolved Hide resolved
return nil, errors.New("file too big")
gwossum marked this conversation as resolved.
Show resolved Hide resolved
} else if size64 == 0 {
// A lot of /proc files report their length as 0, so this will also
// catch a large proportion of /proc files.
jdstrand marked this conversation as resolved.
Show resolved Hide resolved
return nil, errors.New("file empty")
}
size = int(size64)

// Read file
data := make([]byte, size)
b, err := f.Read(data)
if err != nil {
return nil, err
} else if b != size {
return nil, errors.New("short read")
gwossum marked this conversation as resolved.
Show resolved Hide resolved
}

return data, nil
}

// fromFile reads a file from disk and provides a reader from it.
func FromFile(filePath string, extraFileChecks bool) ReaderFn {
return func() (io.Reader, string, error) {
u, err := url.Parse(filePath)
if err != nil {
Expand All @@ -118,9 +182,15 @@ func FromFile(filePath string) ReaderFn {
}

// not using os.Open to avoid having to deal with closing the file in here
b, err := os.ReadFile(u.Path)
if err != nil {
return nil, filePath, err
var b []byte
var rerr error
if extraFileChecks {
b, rerr = limitReadFile(u.Path)
} else {
b, rerr = os.ReadFile(u.Path)
}
if rerr != nil {
return nil, filePath, rerr
}

return bytes.NewBuffer(b), u.String(), nil
Expand Down Expand Up @@ -260,7 +330,7 @@ func parseSource(r io.Reader, opts ...ValidateOptFn) (*Template, error) {
b = bb
}

contentType := http.DetectContentType(b[:512])
contentType := http.DetectContentType(b[:min(len(b), 512)])
switch {
case strings.Contains(contentType, "jsonnet"):
// highly unlikely to fall in here with supported content type detection as is
Expand Down