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 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
}
86 changes: 86 additions & 0 deletions pkg/fs/special_linux.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
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) {
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/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")
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 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,
gwossum marked this conversation as resolved.
Show resolved Hide resolved
// 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"}
gwossum marked this conversation as resolved.
Show resolved Hide resolved
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 {
// See above for why we're returning an error here.
return math.MaxUint64, nil
gwossum marked this conversation as resolved.
Show resolved Hide resolved
}
return fDevId, nil
}
for _, fn := range tmpfsMounts {
// We ignore errors if getFileDevId fails, which could
// mean that the file (e.g. /run) doesn't exist. The error
gwossum marked this conversation as resolved.
Show resolved Hide resolved
if fnDevId, err := getFileDevId(fn); err == nil {
if fnDevId == devId {
return false, nil
}
}
}

// We didn't find any a reason to give st a special file system exception.
return true, nil
}
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 size64 > limitReadFileMaxSize {
gwossum marked this conversation as resolved.
Show resolved Hide resolved
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