diff --git a/.circleci/config.yml b/.circleci/config.yml index 5461fa954..ca8a7a38f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -6,7 +6,7 @@ jobs: working_directory: ~/go/src/github.com/fnproject/fn environment: # apparently expansion doesn't work here yet: https://discuss.circleci.com/t/environment-variable-expansion-in-working-directory/11322 - GOPATH=/home/circleci/go - - GOVERSION=1.11 + - GOVERSION=1.11.2 - OS=linux - ARCH=amd64 - FN_LOG_LEVEL=debug @@ -32,6 +32,11 @@ jobs: - run: nproc # fixes git-diff warning: "warning: inexact rename detection was skipped due to too many files."" - run: git config diff.renamelimit 65535 + # NOTE: if GOFLAGS and GOMODULE are set, gosec will be noisy. unset them (run this before any 'make' command) + - run: | + wget -O - -q https://raw.githubusercontent.com/securego/gosec/master/install.sh | sh -s 1.2.0 + ./bin/gosec -quiet -severity medium ./... + - run: make clear-images # Work out what needs rebuilding and what has changed diff --git a/Makefile b/Makefile index 6e18d086e..6269009da 100644 --- a/Makefile +++ b/Makefile @@ -1,9 +1,7 @@ -export GO111MODULE=on -export GOFLAGS=-mod=vendor # Just builds .PHONY: mod dep: - go mod vendor -v + GO111MODULE=on GOFLAGS=-mod=vendor go mod vendor -v .PHONY: mod-up dep-up: diff --git a/api/agent/agent.go b/api/agent/agent.go index 0670d9ae9..1a7a236c8 100644 --- a/api/agent/agent.go +++ b/api/agent/agent.go @@ -119,6 +119,7 @@ type agent struct { type Option func(*agent) error // RegistryToken is a reserved call extensions key to pass registry token +/* #nosec */ const RegistryToken = "FN_REGISTRY_TOKEN" // New creates an Agent that executes functions locally as Docker containers. diff --git a/api/agent/drivers/docker/docker_client.go b/api/agent/drivers/docker/docker_client.go index b00faf01a..1351b5dff 100644 --- a/api/agent/drivers/docker/docker_client.go +++ b/api/agent/drivers/docker/docker_client.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "os" + "path/filepath" "strings" "time" @@ -297,7 +298,7 @@ func (d *dockerWrap) LoadImages(ctx context.Context, filePath string) error { ctx, closer := makeTracker(ctx, "docker_load_images") defer closer() - file, err := os.Open(filePath) + file, err := os.Open(filepath.Clean(filePath)) if err != nil { return err } diff --git a/api/agent/resource.go b/api/agent/resource.go index 003be2b37..1595be546 100644 --- a/api/agent/resource.go +++ b/api/agent/resource.go @@ -8,6 +8,7 @@ import ( "io" "io/ioutil" "os" + "path/filepath" "runtime" "strconv" "strings" @@ -493,7 +494,7 @@ func getMemoryHeadRoom(usableMemory uint64, cfg *Config) (uint64, error) { } func readString(fileName string) (string, error) { - b, err := ioutil.ReadFile(fileName) + b, err := ioutil.ReadFile(filepath.Clean(fileName)) if err != nil { return "", err } diff --git a/api/common/logging.go b/api/common/logging.go index 2e9128fc9..aa11a9319 100644 --- a/api/common/logging.go +++ b/api/common/logging.go @@ -83,7 +83,7 @@ func SetLogDest(to, prefix string) { return } case "file": - f, err := os.OpenFile(parsed.Path, os.O_RDWR|os.O_APPEND|os.O_CREATE, 0666) + f, err := os.OpenFile(parsed.Path, os.O_RDWR|os.O_APPEND|os.O_CREATE, 0600) if err != nil { logrus.WithError(err).WithFields(logrus.Fields{"to": to, "path": parsed.Path}).Error("cannot open file, defaulting to stderr") return diff --git a/api/common/tls_utils.go b/api/common/tls_utils.go index 6c1e6a48b..87e3bccc9 100644 --- a/api/common/tls_utils.go +++ b/api/common/tls_utils.go @@ -10,7 +10,7 @@ import ( "path/filepath" ) -// A simple TLS Config generator using cert/key +// NewTLSSimple creates a new tls config with the given cert and key file paths func NewTLSSimple(certPath, keyPath string) (*tls.Config, error) { err := checkFile(certPath) @@ -34,7 +34,7 @@ func NewTLSSimple(certPath, keyPath string) (*tls.Config, error) { }, nil } -// Add a Client CA +// AddClientCA adds a client cert to the given tls config func AddClientCA(tlsConf *tls.Config, clientCAPath string) error { err := checkFile(clientCAPath) @@ -42,7 +42,7 @@ func AddClientCA(tlsConf *tls.Config, clientCAPath string) error { return err } // Create a certificate pool from the certificate authority - authority, err := ioutil.ReadFile(clientCAPath) + authority, err := ioutil.ReadFile(filepath.Clean(clientCAPath)) if err != nil { return fmt.Errorf("Could not read client CA (%s) certificate: %s", clientCAPath, err) } @@ -58,7 +58,7 @@ func AddClientCA(tlsConf *tls.Config, clientCAPath string) error { return nil } -// Add CA +// AddCA adds a ca cert to the given tls config func AddCA(tlsConf *tls.Config, caPath string) error { err := checkFile(caPath) @@ -66,7 +66,7 @@ func AddCA(tlsConf *tls.Config, caPath string) error { return err } - ca, err := ioutil.ReadFile(caPath) + ca, err := ioutil.ReadFile(filepath.Clean(caPath)) if err != nil { return fmt.Errorf("could not read ca (%s) certificate: %s", caPath, err) } diff --git a/api/datastore/sql/migratex/migrate.go b/api/datastore/sql/migratex/migrate.go index 702b7f841..17da4fabe 100644 --- a/api/datastore/sql/migratex/migrate.go +++ b/api/datastore/sql/migratex/migrate.go @@ -283,6 +283,7 @@ func SetVersion(ctx context.Context, tx *sqlx.Tx, version int64, dirty bool) err // TODO need to handle down migration better // ideally, we have a record of each up/down migration with a timestamp for auditing, // this just nukes the whole table which is kinda lame. + /* #nosec */ query := tx.Rebind("DELETE FROM " + MigrationsTable) if _, err := tx.Exec(query); err != nil { logrus.WithError(err).Error("error deleting version table") @@ -290,6 +291,7 @@ func SetVersion(ctx context.Context, tx *sqlx.Tx, version int64, dirty bool) err } if version >= 0 { + /* #nosec */ query = tx.Rebind(`INSERT INTO ` + MigrationsTable + ` (version, dirty) VALUES (?, ?)`) if _, err := tx.ExecContext(ctx, query, version, dirty); err != nil { logrus.WithError(err).Error("error updating version table") @@ -316,6 +318,7 @@ func Version(ctx context.Context, tx *sqlx.Tx) (version int64, dirty bool, err e return NilVersion, false, nil } + /* #nosec */ query := tx.Rebind(`SELECT version, dirty FROM ` + MigrationsTable + ` LIMIT 1`) err = tx.QueryRowContext(ctx, query).Scan(&version, &dirty) diff --git a/api/datastore/sql/migrations/10_add_app_id.go b/api/datastore/sql/migrations/10_add_app_id.go index 5c0917c3a..f1dc9ac32 100644 --- a/api/datastore/sql/migrations/10_add_app_id.go +++ b/api/datastore/sql/migrations/10_add_app_id.go @@ -113,6 +113,7 @@ func up10(ctx context.Context, tx *sqlx.Tx) error { } for _, t := range []string{"routes", "calls", "logs"} { + /* #nosec */ q := fmt.Sprintf(`UPDATE %s SET app_id=:id WHERE app_name=:name;`, t) _, err = tx.NamedExecContext(ctx, tx.Rebind(q), app) if err != nil { @@ -138,6 +139,7 @@ func up10(ctx context.Context, tx *sqlx.Tx) error { statement := t["statement"] tableName := t["table"] columns := t["columns"] + /* #nosec */ _, err = tx.ExecContext(ctx, tx.Rebind(fmt.Sprintf("ALTER TABLE %s RENAME TO old_%s;", tableName, tableName))) if err != nil { return err @@ -146,11 +148,13 @@ func up10(ctx context.Context, tx *sqlx.Tx) error { if err != nil { return err } + /* #nosec */ _, err = tx.ExecContext(ctx, tx.Rebind(fmt.Sprintf("INSERT INTO %s (%s) SELECT %s FROM old_%s;", tableName, columns, columns, tableName))) if err != nil { return err } + /* #nosec */ _, err = tx.ExecContext(ctx, tx.Rebind(fmt.Sprintf("DROP TABLE old_%s;", tableName))) if err != nil { return err @@ -204,7 +208,8 @@ func down10(ctx context.Context, tx *sqlx.Tx) error { // it is required for some reason, can't do this within the rows iteration. for _, app := range res { for _, t := range []string{"routes", "calls", "logs"} { - q := "UPDATE " + t + " SET app_name=:name WHERE app_id=:id;" + /* #nosec */ + q := fmt.Sprintf("UPDATE %s SET app_name=:name WHERE app_id=:id;", t) _, err = tx.NamedExecContext(ctx, tx.Rebind(q), app) if err != nil { return err diff --git a/api/datastore/sql/mysql/mysql.go b/api/datastore/sql/mysql/mysql.go index cc590a464..ccab4daa9 100644 --- a/api/datastore/sql/mysql/mysql.go +++ b/api/datastore/sql/mysql/mysql.go @@ -1,13 +1,13 @@ package mysql import ( - "fmt" + "net/url" + "strings" + "github.com/fnproject/fn/api/datastore/sql/dbhelper" "github.com/go-sql-driver/mysql" _ "github.com/go-sql-driver/mysql" "github.com/jmoiron/sqlx" - "net/url" - "strings" ) type mysqlHelper int @@ -25,12 +25,11 @@ func (mysqlHelper) PostCreate(db *sqlx.DB) (*sqlx.DB, error) { } func (mysqlHelper) CheckTableExists(tx *sqlx.Tx, table string) (bool, error) { - query := tx.Rebind(fmt.Sprintf(`SELECT count(*) + query := tx.Rebind(`SELECT count(*) FROM information_schema.TABLES - WHERE TABLE_NAME = '%s' -`, table)) + WHERE TABLE_NAME = ?`) - row := tx.QueryRow(query) + row := tx.QueryRow(query, table) var count int err := row.Scan(&count) diff --git a/api/datastore/sql/sql.go b/api/datastore/sql/sql.go index 9baa57e51..eabb9101e 100644 --- a/api/datastore/sql/sql.go +++ b/api/datastore/sql/sql.go @@ -14,15 +14,14 @@ import ( "time" "github.com/fnproject/fn/api/common" - "github.com/fnproject/fn/api/datastore/sql/migratex" - "github.com/fnproject/fn/api/datastore/sql/migrations" - "github.com/fnproject/fn/api/models" - "github.com/jmoiron/sqlx" - "github.com/fnproject/fn/api/datastore" "github.com/fnproject/fn/api/datastore/sql/dbhelper" + "github.com/fnproject/fn/api/datastore/sql/migratex" + "github.com/fnproject/fn/api/datastore/sql/migrations" "github.com/fnproject/fn/api/id" "github.com/fnproject/fn/api/logs" + "github.com/fnproject/fn/api/models" + "github.com/jmoiron/sqlx" "github.com/sirupsen/logrus" ) @@ -492,6 +491,7 @@ func (ds *SQLStore) GetApps(ctx context.Context, filter *models.AppFilter) (*mod if err != nil { return nil, err } + /* #nosec */ query = ds.db.Rebind(fmt.Sprintf("SELECT DISTINCT id, name, config, annotations, syslog_url, created_at, updated_at FROM apps %s", query)) rows, err := ds.db.QueryxContext(ctx, query, args...) if err != nil { @@ -636,6 +636,7 @@ func (ds *SQLStore) GetFns(ctx context.Context, filter *models.FnFilter) (*model return res, err } + /* #nosec */ query := fmt.Sprintf("%s %s", fnSelector, filterQuery) query = ds.db.Rebind(query) rows, err := ds.db.QueryxContext(ctx, query, args...) @@ -670,6 +671,7 @@ func (ds *SQLStore) GetFns(ctx context.Context, filter *models.FnFilter) (*model } func (ds *SQLStore) GetFnByID(ctx context.Context, fnID string) (*models.Fn, error) { + /* #nosec */ query := ds.db.Rebind(fmt.Sprintf("%s WHERE id=?", fnSelector)) row := ds.db.QueryRowxContext(ctx, query, fnID) @@ -684,9 +686,8 @@ func (ds *SQLStore) GetFnByID(ctx context.Context, fnID string) (*models.Fn, err } func (ds *SQLStore) RemoveFn(ctx context.Context, fnID string) error { - return ds.Tx(func(tx *sqlx.Tx) error { - + /* #nosec */ query := tx.Rebind(fmt.Sprintf("%s WHERE id=?", fnSelector)) row := tx.QueryRowxContext(ctx, query, fnID) @@ -753,6 +754,7 @@ func (ds *SQLStore) InsertCall(ctx context.Context, call *models.Call) error { } func (ds *SQLStore) GetCall1(ctx context.Context, appID, callID string) (*models.Call, error) { + /* #nosec */ query := fmt.Sprintf(`%s WHERE id=? AND app_id=?`, callSelector) query = ds.db.Rebind(query) row := ds.db.QueryRowxContext(ctx, query, callID, appID) @@ -769,6 +771,7 @@ func (ds *SQLStore) GetCall1(ctx context.Context, appID, callID string) (*models } func (ds *SQLStore) GetCall(ctx context.Context, fnID, callID string) (*models.Call, error) { + /* #nosec */ query := fmt.Sprintf(`%s WHERE id=? AND fn_id=?`, callSelector) query = ds.db.Rebind(query) row := ds.db.QueryRowxContext(ctx, query, callID, fnID) @@ -811,6 +814,7 @@ func (ds *SQLStore) GetCalls(ctx context.Context, filter *models.CallFilter) (*m func (ds *SQLStore) GetCalls1(ctx context.Context, filter *models.CallFilter) ([]*models.Call, error) { res := []*models.Call{} query, args := buildFilterCallQuery(filter) + /* #nosec */ query = fmt.Sprintf("%s %s", callSelector, query) query = ds.db.Rebind(query) rows, err := ds.db.QueryxContext(ctx, query, args...) @@ -962,8 +966,10 @@ func where(b *bytes.Buffer, args []interface{}, colOp string, val interface{}) [ } args = append(args, val) if len(args) == 1 { + /* #nosec */ fmt.Fprintf(b, `WHERE %s`, colOp) } else { + /* #nosec */ fmt.Fprintf(b, ` AND %s`, colOp) } return args @@ -1093,6 +1099,7 @@ func (ds *SQLStore) UpdateTrigger(ctx context.Context, trigger *models.Trigger) func (ds *SQLStore) GetTrigger(ctx context.Context, appId, fnId, triggerName string) (*models.Trigger, error) { var trigger models.Trigger + /* #nosec */ query := ds.db.Rebind(fmt.Sprintf("%s WHERE name=? AND app_id=? AND fn_id=?", fnSelector)) row := ds.db.QueryRowxContext(ctx, query, triggerName, appId, fnId) @@ -1189,6 +1196,7 @@ func (ds *SQLStore) GetTriggers(ctx context.Context, filter *models.TriggerFilte return res, err } + /* #nosec */ query := fmt.Sprintf("%s WHERE %s", triggerSelector, filterQuery) query = ds.db.Rebind(query) rows, err := ds.db.QueryxContext(ctx, query, args...) diff --git a/api/datastore/sql/sqlite/sqlite.go b/api/datastore/sql/sqlite/sqlite.go index 0dd14efb2..b32743adc 100644 --- a/api/datastore/sql/sqlite/sqlite.go +++ b/api/datastore/sql/sqlite/sqlite.go @@ -1,14 +1,14 @@ package sqlite import ( - "fmt" - "github.com/fnproject/fn/api/datastore/sql/dbhelper" - "github.com/jmoiron/sqlx" - "github.com/mattn/go-sqlite3" "net/url" "os" "path/filepath" "strings" + + "github.com/fnproject/fn/api/datastore/sql/dbhelper" + "github.com/jmoiron/sqlx" + "github.com/mattn/go-sqlite3" ) type sqliteHelper int @@ -24,7 +24,7 @@ func (sqliteHelper) Supports(scheme string) bool { func (sqliteHelper) PreConnect(url *url.URL) (string, error) { // make all the dirs so we can make the file.. dir := filepath.Dir(url.Path) - err := os.MkdirAll(dir, 0755) + err := os.MkdirAll(dir, 0750) if err != nil { return "", err } @@ -38,12 +38,11 @@ func (sqliteHelper) PostCreate(db *sqlx.DB) (*sqlx.DB, error) { } func (sqliteHelper) CheckTableExists(tx *sqlx.Tx, table string) (bool, error) { - query := tx.Rebind(fmt.Sprintf(`SELECT count(*) + query := tx.Rebind(`SELECT count(*) FROM sqlite_master - WHERE name = '%s' - `, table)) + WHERE name = ?`) - row := tx.QueryRow(query) + row := tx.QueryRow(query, table) var count int err := row.Scan(&count) diff --git a/api/mqs/bolt/bolt.go b/api/mqs/bolt/bolt.go index 5ead9dbd6..4ff126ee9 100644 --- a/api/mqs/bolt/bolt.go +++ b/api/mqs/bolt/bolt.go @@ -70,12 +70,12 @@ func (boltProvider) String() string { func (boltProvider) New(url *url.URL) (models.MessageQueue, error) { dir := filepath.Dir(url.Path) log := logrus.WithFields(logrus.Fields{"mq": url.Scheme, "dir": dir}) - err := os.MkdirAll(dir, 0755) + err := os.MkdirAll(dir, 0750) if err != nil { log.WithError(err).Errorln("Could not create data directory for mq") return nil, err } - db, err := bolt.Open(url.Path, 0666, &bolt.Options{Timeout: 1 * time.Second}) + db, err := bolt.Open(url.Path, 0600, &bolt.Options{Timeout: 1 * time.Second}) if err != nil { log.WithError(err).Errorln("Could not open BoltDB file for MQ") return nil, err diff --git a/api/server/init.go b/api/server/init.go index 77b8597c3..e7ae4d976 100644 --- a/api/server/init.go +++ b/api/server/init.go @@ -5,6 +5,7 @@ import ( "io/ioutil" "os" "os/signal" + "path/filepath" "runtime" "strconv" "strings" @@ -24,7 +25,7 @@ func getEnv(key, fallback string) string { if value, ok := os.LookupEnv(key); ok { return value } else if value, ok := os.LookupEnv(key + "_FILE"); ok { - dat, err := ioutil.ReadFile(value) + dat, err := ioutil.ReadFile(filepath.Clean(value)) if err == nil { return string(dat) } @@ -42,7 +43,7 @@ func getEnvInt(key string, fallback int) int { } return i } else if value, ok := os.LookupEnv(key + "_FILE"); ok { - dat, err := ioutil.ReadFile(value) + dat, err := ioutil.ReadFile(filepath.Clean(value)) if err == nil { var err error var i int diff --git a/images/fn-test-utils/fn-test-utils.go b/images/fn-test-utils/fn-test-utils.go index 5c82ed8b8..a183f8a57 100644 --- a/images/fn-test-utils/fn-test-utils.go +++ b/images/fn-test-utils/fn-test-utils.go @@ -10,6 +10,7 @@ import ( "log" "net/http" "os" + "path/filepath" "strconv" "strings" "time" @@ -364,7 +365,7 @@ func getChunk(size int) []byte { func readFile(name string, size int) (string, error) { // read the whole file into memory - out, err := ioutil.ReadFile(name) + out, err := ioutil.ReadFile(filepath.Clean(name)) if err != nil { return "", err } @@ -376,7 +377,7 @@ func readFile(name string, size int) (string, error) { } func createFile(name string, size int) error { - f, err := os.Create(name) + f, err := os.Create(filepath.Clean(name)) if err != nil { return err }