Merge pull request #35 from phires/gosec-fixes

Gosec fixes

backends/p_guerrilla_db_redis.go

@@ -3,10 +3,11 @@ package backends
 import (
 	"bytes"
 	"compress/zlib"
+	"crypto/rand"
 	"database/sql"
+	"encoding/binary"
 	"fmt"
 	"io"
-	"math/rand"
 	"runtime/debug"
 	"strings"
 	"sync"
@@ -152,25 +153,7 @@ func (g *GuerrillaDBAndRedisBackend) prepareInsertQuery(rows int, db *sql.DB) *s
 	if g.cache[rows-1] != nil {
 		return g.cache[rows-1]
 	}
-	sqlstr := "INSERT INTO " + g.config.Table + "" +
-		"(" +
-		"`date`, " +
-		"`to`, " +
-		"`from`, " +
-		"`subject`, " +
-		"`body`, " +
-		"`charset`, " +
-		"`mail`, " +
-		"`spam_score`, " +
-		"`hash`, " +
-		"`content_type`, " +
-		"`recipient`, " +
-		"`has_attach`, " +
-		"`ip_addr`, " +
-		"`return_path`, " +
-		"`is_tls`" +
-		")" +
-		" values "
+	sqlstr := fmt.Sprintf("INSERT INTO %s (`date`, `to`, `from`, `subject`, `body`, `charset`, `mail`, `spam_score`, `hash`, `content_type`, `recipient`, `has_attach`, `ip_addr`, `return_path`, `is_tls`) VALUES ", g.config.Table) // #nosec G201 -- SQL injection safe
 	values := "(NOW(), ?, ?, ?, ? , 'UTF-8' , ?, 0, ?, '', ?, 0, ?, ?, ?)"
 	// add more rows
 	comma := ""
@@ -267,7 +250,8 @@ func (g *GuerrillaDBAndRedisBackend) insertQueryBatcher(
 		vals = nil
 		count = 0
 	}
-	rand.Seed(time.Now().UnixNano())
+	// No seeding needed for crypto/rand
+	//rand.Seed(time.Now().UnixNano())
 	defer func() {
 		if r := recover(); r != nil {
 			Log().Error("insertQueryBatcher caught a panic", r, string(debug.Stack()))
@@ -325,7 +309,7 @@ func (g *GuerrillaDBAndRedisBackend) sqlConnect() (*sql.DB, error) {
 		return nil, err
 	} else {
 		// do we have access?
-		_, err = db.Query("SELECT mail_id FROM " + g.config.Table + " LIMIT 1")
+		_, err = db.Query(fmt.Sprintf("SELECT mail_id FROM %s LIMIT 1", g.config.Table))
 		if err != nil {
 			Log().Error("cannot select table:", err)
 			return nil, err
@@ -335,7 +319,7 @@ func (g *GuerrillaDBAndRedisBackend) sqlConnect() (*sql.DB, error) {
 }
 
 func (c *redisClient) redisConnection(redisInterface string) (err error) {
-	if c.isConnected == false {
+	if !c.isConnected {
 		c.conn, err = RedisDialer("tcp", redisInterface)
 		if err != nil {
 			// handle error
@@ -484,7 +468,13 @@ func GuerrillaDbRedis() Decorator {
 					trimToLimit(e.MailFrom.String(), 255),
 					e.TLS)
 				// give the values to a random query batcher
-				feeders[rand.Intn(len(feeders))] <- vals
+				var index int
+				err := binary.Read(rand.Reader, binary.LittleEndian, &index)
+				if err != nil {
+					panic(err)
+				}
+				index %= len(feeders)
+				feeders[index] <- vals
 				return p.Process(e, task)
 
 			} else {

backends/p_sql.go

@@ -92,7 +92,7 @@ func (s *SQLProcessor) connect() (*sql.DB, error) {
 	}
 
 	// do we have permission to access the table?
-	_, err = db.Query("SELECT mail_id FROM " + s.config.Table + " LIMIT 1")
+	_, err = db.Query(fmt.Sprintf("SELECT mail_id FROM %s LIMIT 1", s.config.Table))
 	if err != nil {
 		return nil, err
 	}

server.go

@@ -132,6 +132,7 @@ func (s *server) configureTLS() error {
 			Certificates: []tls.Certificate{cert},
 			ClientAuth:   tls.VerifyClientCertIfGiven,
 			ServerName:   sConfig.Hostname,
+			MinVersion:   tls.VersionTLS10, // #nosec G402 -- we need to keep at least TLS1.0 for backward compatibility
 		}
 		if len(sConfig.TLS.Protocols) > 0 {
 			if min, ok := TLSProtocols[sConfig.TLS.Protocols[0]]; ok {

tests/client.go

@@ -18,7 +18,7 @@ func Connect(serverConfig guerrilla.ServerConfig, deadline time.Duration) (net.C
 	if serverConfig.TLS.AlwaysOn {
 		// start tls automatically
 		conn, err = tls.Dial("tcp", serverConfig.ListenInterface, &tls.Config{
-			InsecureSkipVerify: true,
+			InsecureSkipVerify: true, // #nosec G402 -- local test only with self-signed cert
 			ServerName:         "127.0.0.1",
 		})
 	} else {

tests/testcert/generate_cert.go

@@ -139,7 +139,7 @@ func GenerateCert(host string, validFrom string, validFor time.Duration, isCA bo
 		log.Fatalf("Failed to create certificate: %s", err)
 	}
 
-	certOut, err := os.Create(dirPrefix + host + ".cert.pem")
+	certOut, err := os.Create(dirPrefix + host + ".cert.pem") // #nosec G304 -- doesn't matter for tests
 	if err != nil {
 		log.Fatalf("failed to open cert.pem for writing: %s", err)
 	}
@@ -154,7 +154,7 @@ func GenerateCert(host string, validFrom string, validFor time.Duration, isCA bo
 		return
 	}
 
-	keyOut, err := os.OpenFile(dirPrefix+host+".key.pem", os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
+	keyOut, err := os.OpenFile(dirPrefix+host+".key.pem", os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) // #nosec G304 -- doesn't matter for tests
 	if err != nil {
 		log.Print("failed to open key.pem for writing:", err)
 		return