Skip to content

Commit

Permalink
Fixes for gosec
Browse files Browse the repository at this point in the history
  • Loading branch information
aledbf committed Dec 4, 2020
1 parent 9553b27 commit d781d99
Show file tree
Hide file tree
Showing 12 changed files with 43 additions and 30 deletions.
5 changes: 4 additions & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ jobs:
- name: Run Gosec Security Scanner
uses: securego/gosec@master
with:
args: -exclude=G104,G304 -exclude-dir=test ./...
# G601 for zz_generated.deepcopy.go
# G306 TODO: Expect WriteFile permissions to be 0600 or less
# G307 TODO: Deferring unsafe method "Close"
args: -exclude=G601,G104,G204,G304,G306,G307 -tests=false -exclude-dir=test -exclude-dir=images/ -exclude-dir=docs/ ./...

build:
name: Build
Expand Down
2 changes: 1 addition & 1 deletion cmd/nginx/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package main
import (
"context"
"fmt"
"math/rand"
"math/rand" // #nosec
"net/http"
"net/http/pprof"
"os"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,18 @@ package main

import (
"fmt"
"math/rand"
"net/http"
"strconv"
"strings"

"k8s.io/apimachinery/pkg/util/uuid"
)

// Sample authentication service returning several HTTP headers in response
func main() {
http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
if strings.ContainsAny(r.Header.Get("User"), "internal") {
w.Header().Add("UserID", strconv.Itoa(rand.Int()))
w.Header().Add("UserID", fmt.Sprintf("%v", uuid.NewUUID()))
w.Header().Add("UserRole", "admin")
w.Header().Add("Other", "not used")
fmt.Fprint(w, "ok")
Expand Down
12 changes: 6 additions & 6 deletions internal/admission/controller/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ func convertV1beta1AdmissionReviewToAdmissionAdmissionReview(in *admissionv1beta
} else {
out.Request = nil
}
out.Response = (*admissionv1.AdmissionResponse)(unsafe.Pointer(in.Response))
out.Response = (*admissionv1.AdmissionResponse)(unsafe.Pointer(in.Response)) // #nosec
}

func convertV1beta1AdmissionRequestToAdmissionAdmissionRequest(in *admissionv1beta1.AdmissionRequest, out *admissionv1.AdmissionRequest) {
out.UID = types.UID(in.UID)
out.Kind = in.Kind
out.Resource = in.Resource
out.SubResource = in.SubResource
out.RequestKind = (*metav1.GroupVersionKind)(unsafe.Pointer(in.RequestKind))
out.RequestResource = (*metav1.GroupVersionResource)(unsafe.Pointer(in.RequestResource))
out.RequestKind = (*metav1.GroupVersionKind)(unsafe.Pointer(in.RequestKind)) // #nosec
out.RequestResource = (*metav1.GroupVersionResource)(unsafe.Pointer(in.RequestResource)) // #nosec
out.RequestSubResource = in.RequestSubResource
out.Name = in.Name
out.Namespace = in.Namespace
Expand All @@ -70,16 +70,16 @@ func convertAdmissionAdmissionReviewToV1beta1AdmissionReview(in *admissionv1.Adm
} else {
out.Request = nil
}
out.Response = (*admissionv1beta1.AdmissionResponse)(unsafe.Pointer(in.Response))
out.Response = (*admissionv1beta1.AdmissionResponse)(unsafe.Pointer(in.Response)) // #nosec
}

func convertAdmissionAdmissionRequestToV1beta1AdmissionRequest(in *admissionv1.AdmissionRequest, out *admissionv1beta1.AdmissionRequest) {
out.UID = types.UID(in.UID)
out.Kind = in.Kind
out.Resource = in.Resource
out.SubResource = in.SubResource
out.RequestKind = (*metav1.GroupVersionKind)(unsafe.Pointer(in.RequestKind))
out.RequestResource = (*metav1.GroupVersionResource)(unsafe.Pointer(in.RequestResource))
out.RequestKind = (*metav1.GroupVersionKind)(unsafe.Pointer(in.RequestKind)) // #nosec
out.RequestResource = (*metav1.GroupVersionResource)(unsafe.Pointer(in.RequestResource)) // #nosec
out.RequestSubResource = in.RequestSubResource
out.Name = in.Name
out.Namespace = in.Namespace
Expand Down
4 changes: 2 additions & 2 deletions internal/file/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ limitations under the License.
package file

import (
"crypto/sha1"
"crypto/sha1" // #nosec
"encoding/hex"
"io/ioutil"

Expand All @@ -26,7 +26,7 @@ import (

// SHA1 returns the SHA1 of a file.
func SHA1(filename string) string {
hasher := sha1.New()
hasher := sha1.New() // #nosec
s, err := ioutil.ReadFile(filename)
if err != nil {
klog.ErrorS(err, "Error reading file", "path", filename)
Expand Down
16 changes: 10 additions & 6 deletions internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ func (n *NGINXController) getStreamServices(configmapName string, proto apiv1.Pr
reserverdPorts := sets.NewInt(rp...)
// svcRef format: <(str)namespace>/<(str)service>:<(intstr)port>[:<("PROXY")decode>:<("PROXY")encode>]
for port, svcRef := range configmap.Data {
externalPort, err := strconv.Atoi(port)
externalPort, err := strconv.Atoi(port) // #nosec
if err != nil {
klog.Warningf("%q is not a valid %v port number", port, proto)
continue
Expand Down Expand Up @@ -342,11 +342,13 @@ func (n *NGINXController) getStreamServices(configmapName string, proto apiv1.Pr
continue
}
var endps []ingress.Endpoint
targetPort, err := strconv.Atoi(svcPort)
/* #nosec */
targetPort, err := strconv.Atoi(svcPort) // #nosec
if err != nil {
// not a port number, fall back to using port name
klog.V(3).Infof("Searching Endpoints with %v port name %q for Service %q", proto, svcPort, nsName)
for _, sp := range svc.Spec.Ports {
for i := range svc.Spec.Ports {
sp := svc.Spec.Ports[i]
if sp.Name == svcPort {
if sp.Protocol == proto {
endps = getEndpoints(svc, &sp, proto, n.store.GetServiceEndpoints)
Expand All @@ -356,7 +358,8 @@ func (n *NGINXController) getStreamServices(configmapName string, proto apiv1.Pr
}
} else {
klog.V(3).Infof("Searching Endpoints with %v port number %d for Service %q", proto, targetPort, nsName)
for _, sp := range svc.Spec.Ports {
for i := range svc.Spec.Ports {
sp := svc.Spec.Ports[i]
if sp.Port == int32(targetPort) {
if sp.Protocol == proto {
endps = getEndpoints(svc, &sp, proto, n.store.GetServiceEndpoints)
Expand Down Expand Up @@ -939,7 +942,8 @@ func (n *NGINXController) serviceEndpoints(svcKey, backendPort string) ([]ingres
return upstreams, nil
}

for _, servicePort := range svc.Spec.Ports {
for i := range svc.Spec.Ports {
servicePort := svc.Spec.Ports[i]
// targetPort could be a string, use either the port name or number (int)
if strconv.Itoa(int(servicePort.Port)) == backendPort ||
servicePort.TargetPort.String() == backendPort ||
Expand Down Expand Up @@ -1498,7 +1502,7 @@ func shouldCreateUpstreamForLocationDefaultBackend(upstream *ingress.Backend, lo
}

func externalNamePorts(name string, svc *apiv1.Service) *apiv1.ServicePort {
port, err := strconv.Atoi(name)
port, err := strconv.Atoi(name) // #nosec
if err != nil {
// not a number. check port names.
for _, svcPort := range svc.Spec.Ports {
Expand Down
2 changes: 1 addition & 1 deletion internal/ingress/controller/nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ func (n NGINXController) generateTemplate(cfg ngx_config.Configuration, ingressC
klog.Warningf("Missing Service for SSL Passthrough backend %q", pb.Backend)
continue
}
port, err := strconv.Atoi(pb.Port.String())
port, err := strconv.Atoi(pb.Port.String()) // #nosec
if err != nil {
for _, sp := range svc.Spec.Ports {
if sp.Name == pb.Port.String() {
Expand Down
8 changes: 4 additions & 4 deletions internal/ingress/controller/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ package template

import (
"bytes"
"crypto/sha1"
"crypto/sha1" // #nosec
"encoding/base64"
"encoding/hex"
"encoding/json"
"fmt"
"io/ioutil"
"math/rand"
"math/rand" // #nosec
"net"
"net/url"
"os"
Expand Down Expand Up @@ -929,7 +929,7 @@ func buildAuthSignURL(authSignURL, authRedirectParam string) string {
}

func buildAuthSignURLLocation(location, authSignURL string) string {
hasher := sha1.New()
hasher := sha1.New() // #nosec
hasher.Write([]byte(location))
hasher.Write([]byte(authSignURL))
return "@" + hex.EncodeToString(hasher.Sum(nil))
Expand All @@ -944,7 +944,7 @@ func init() {
func randomString() string {
b := make([]rune, 32)
for i := range b {
b[i] = letters[rand.Intn(len(letters))]
b[i] = letters[rand.Intn(len(letters))] // #nosec
}

return string(b)
Expand Down
2 changes: 1 addition & 1 deletion internal/ingress/metric/collectors/socket.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func NewSocketCollector(pod, namespace, class string, metricsPerHost bool) (*Soc
return nil, err
}

err = os.Chmod(socket, 0777)
err = os.Chmod(socket, 0777) // #nosec
if err != nil {
return nil, err
}
Expand Down
3 changes: 2 additions & 1 deletion internal/ingress/status/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,8 @@ func (s *statusSync) runningAddresses() ([]string, error) {
}

addrs := make([]string, 0)
for _, pod := range pods.Items {
for i := range pods.Items {
pod := pods.Items[i]
// only Running pods are valid
if pod.Status.Phase != apiv1.PodRunning {
continue
Expand Down
12 changes: 8 additions & 4 deletions internal/net/ssl/ssl.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"bytes"
"crypto/rand"
"crypto/rsa"
"crypto/sha1"
"crypto/sha1" // #nosec
"crypto/tls"
"crypto/x509"
"crypto/x509/pkix"
Expand Down Expand Up @@ -125,7 +125,7 @@ func CreateSSLCert(cert, key []byte, uid string) (*ingress.SSLCert, error) {
}
}

hasher := sha1.New()
hasher := sha1.New() // #nosec
hasher.Write(pemCert.Raw)

return &ingress.SSLCert{
Expand Down Expand Up @@ -504,9 +504,12 @@ func NewTLSListener(certificate, key string) *TLSListener {
keyPath: key,
lock: sync.Mutex{},
}

l.load()
watch.NewFileWatcher(certificate, l.load)
watch.NewFileWatcher(key, l.load)

_, _ = watch.NewFileWatcher(certificate, l.load)
_, _ = watch.NewFileWatcher(key, l.load)

return &l
}

Expand All @@ -521,6 +524,7 @@ func (tl *TLSListener) GetCertificate(*tls.ClientHelloInfo) (*tls.Certificate, e
func (tl *TLSListener) TLSConfig() *tls.Config {
return &tls.Config{
GetCertificate: tl.GetCertificate,
MinVersion: tls.VersionTLS12,
}
}

Expand Down
2 changes: 1 addition & 1 deletion internal/nginx/maxmind.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func downloadDatabase(dbName string) error {

defer outFile.Close()

if _, err := io.Copy(outFile, tarReader); err != nil {
if _, err := io.CopyN(outFile, tarReader, header.Size); err != nil {
return err
}

Expand Down

0 comments on commit d781d99

Please sign in to comment.