Skip to content

Commit

Permalink
feat: disable file:// urls when hardening enabled (#25069)
Browse files Browse the repository at this point in the history
Stacks and templates allow specifying file:// URLs. Add command line
option `--template-file-urls-disabled` to disable their use for people who don't require them.

Closes: #25068
(cherry picked from commit 9fd91a5)
  • Loading branch information
gwossum authored Jun 17, 2024
1 parent c6c00b8 commit 852a03d
Show file tree
Hide file tree
Showing 8 changed files with 876 additions and 33 deletions.
26 changes: 20 additions & 6 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 @@ -643,9 +647,10 @@ func (o *InfluxdOpts) BindCliOpts() []cli.Opt {
},

// hardening options
// --hardening-enabled is meant to enable all hardending
// --hardening-enabled is meant to enable all hardening
// options in one go. Today it enables the IP validator for
// flux and pkger templates HTTP requests. In the future,
// flux and pkger templates HTTP requests, and disables file://
// protocol for pkger templates. 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
Expand All @@ -657,7 +662,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; disable file URLs in templates)",
},

// --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
}
91 changes: 91 additions & 0 deletions pkg/fs/special_linux.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package fs

import (
"errors"
"io/fs"
"math"
"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) {
// 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/admin-guide/devices.txt file.

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")
}
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 might be used at a variety of mount points.
// Since the minor IDs are assigned dynamically, we'll find the device ID
// for each common tmpfs mount point. If the mount point's device ID matches this st's,
// then it is reasonable to assume the file is in tmpfs. If the device ID
// does not match, then st is not located in that special file system so we
// can't give an exception based on that file system root. This check is still
// valid even if the directory we check against isn't mounted as tmpfs, because
// the device ID won't match so we won't grant a tmpfs exception based on it.
// On Linux, every tmpfs mount has a different device ID, so we need to check
// against all common ones that might be in use.
tmpfsMounts := []string{"/tmp", "/run", "/dev/shm"}
if tmpdir := os.TempDir(); tmpdir != "/tmp" {
tmpfsMounts = append(tmpfsMounts, tmpdir)
}
if xdgRuntimeDir := os.Getenv("XDG_RUNTIME_DIR"); xdgRuntimeDir != "" {
tmpfsMounts = append(tmpfsMounts, xdgRuntimeDir)
}
getFileDevId := func(n string) (uint64, error) {
fSt, err := os.Stat(n)
if err != nil {
return math.MaxUint64, err
}
fDevId, err := getDevId(fSt)
if err != nil {
return math.MaxUint64, err
}
return fDevId, nil
}
var errs []error
for _, fn := range tmpfsMounts {
// Don't stop if getFileDevId returns an error. It could
// be because the tmpfsMount we're checking doesn't exist,
// which shouldn't prevent us from checking the other
// potential mount points.
if fnDevId, err := getFileDevId(fn); err == nil {
if fnDevId == devId {
return false, nil
}
} else if !errors.Is(err, os.ErrNotExist) {
// Ignore errors for missing mount points.
errs = append(errs, err)
}
}

// We didn't find any a reason to give st a special file system exception.
return true, errors.Join(errs...)
}
77 changes: 71 additions & 6 deletions pkger/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (
fluxurl "github.com/influxdata/flux/dependencies/url"
"github.com/influxdata/flux/parser"
errors2 "github.com/influxdata/influxdb/v2/kit/platform/errors"
caperr "github.com/influxdata/influxdb/v2/pkg/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 +104,65 @@ 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.
func limitReadFile(name string) (buf []byte, rErr error) {
// use os.Open() to avoid TOCTOU
f, err := os.Open(name)
if err != nil {
return nil, err
}
defer caperr.Capture(&rErr, f.Close)()

// 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, fmt.Errorf("%w: %q", err, name)
} else if special {
return nil, fmt.Errorf("file in special file system: %q", name)
}

// only support reading regular files
if st.Mode()&os.ModeType != 0 {
return nil, fmt.Errorf("not a regular file: %q", name)
}

// limit how much we read into RAM
var size int
size64 := st.Size()
if limitReadFileMaxSize > 0 && size64 > limitReadFileMaxSize {
return nil, fmt.Errorf("file too big: %q", name)
} else if size64 == 0 {
return nil, fmt.Errorf("file empty: %q", name)
}
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, fmt.Errorf("short read: %q", name)
}

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 +177,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 +325,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
Loading

0 comments on commit 852a03d

Please sign in to comment.