Static check linting improvements. (Velocidex#594)

.golangci.yml

@@ -0,0 +1,17 @@
+run:
+  tests: false
+  build-tags:
+    - server_vql
+    - extras
+    - release
+    - yara
+    - codeanalysis
+
+  allow-parallel-runners: true
+
+linters-settings:
+  govet:
+    settings:
+      printf:
+        funcs:
+          - (www.velocidex.com/golang/velociraptor/logging.LogContext).Error

Makefile

@@ -49,3 +49,6 @@ debug:
 
 debug_client:
 	dlv debug --build-flags="-tags 'server_vql extras'" ./bin/ -- client -v
+
+lint:
+	golangci-lint run

acls/acls.go

@@ -207,7 +207,7 @@ func GetPolicy(
 	}
 
 	acl_obj := &acl_proto.ApiClientACL{}
-	user_path_manager := paths.UserPathManager{principal}
+	user_path_manager := paths.UserPathManager{Name: principal}
 	err = db.GetSubject(config_obj, user_path_manager.ACL(), acl_obj)
 	if err != nil {
 		return nil, err
@@ -228,7 +228,7 @@ func GetEffectivePolicy(
 	}
 
 	acl_obj := &acl_proto.ApiClientACL{}
-	user_path_manager := paths.UserPathManager{principal}
+	user_path_manager := paths.UserPathManager{Name: principal}
 	err = db.GetSubject(config_obj, user_path_manager.ACL(), acl_obj)
 	if err != nil {
 		return nil, err
@@ -251,7 +251,7 @@ func SetPolicy(
 		return err
 	}
 
-	user_path_manager := paths.UserPathManager{principal}
+	user_path_manager := paths.UserPathManager{Name: principal}
 	return db.SetSubject(config_obj, user_path_manager.ACL(), acl_obj)
 }
 

acls/utils.go

@@ -1,8 +1,7 @@
 package acls
 
 import (
-	"errors"
-	"fmt"
+	"github.com/pkg/errors"
 
 	acl_proto "www.velocidex.com/golang/velociraptor/acls/proto"
 	config_proto "www.velocidex.com/golang/velociraptor/config/proto"
@@ -18,7 +17,7 @@ func GrantRoles(
 	for _, role := range roles {
 		if !utils.InString(new_policy.Roles, role) {
 			if !ValidateRole(role) {
-				return errors.New(fmt.Sprintf("Invalid role %v", role))
+				return errors.Errorf("Invalid role %v", role)
 			}
 			new_policy.Roles = append(new_policy.Roles, role)
 		}

api/api.go

@@ -411,15 +411,15 @@ func (self *ApiServer) NotifyClients(
 
 	if in.NotifyAll {
 		self.server_obj.Info("sending notification to everyone")
-		services.GetNotifier().NotifyAllListeners(self.config)
+		err = services.GetNotifier().NotifyAllListeners(self.config)
 	} else if in.ClientId != "" {
 		self.server_obj.Info("sending notification to %s", in.ClientId)
-		services.GetNotifier().NotifyListener(self.config, in.ClientId)
+		err = services.GetNotifier().NotifyListener(self.config, in.ClientId)
 	} else {
 		return nil, status.Error(codes.InvalidArgument,
 			"client id should be specified")
 	}
-	return &empty.Empty{}, nil
+	return &empty.Empty{}, err
 }
 
 func (self *ApiServer) LabelClients(
@@ -596,8 +596,6 @@ func (self *ApiServer) VFSRefreshDirectory(
 	in *api_proto.VFSRefreshDirectoryRequest) (
 	*flows_proto.ArtifactCollectorResponse, error) {
 
-	utils.Debug(in)
-
 	user_name := GetGRPCUserInfo(self.config, ctx).Name
 	permissions := acls.COLLECT_CLIENT
 	perm, err := acls.CheckAccess(self.config, user_name, permissions)
@@ -812,8 +810,12 @@ func (self *ApiServer) WriteEvent(
 			return nil, err
 		}
 
-		return &empty.Empty{}, services.GetJournal().PushRowsToArtifact(self.config,
-			rows, in.Query.Name, peer_name, "")
+		// Only return the first row
+		if true {
+			err := services.GetJournal().PushRowsToArtifact(self.config,
+				rows, in.Query.Name, peer_name, "")
+			return &empty.Empty{}, err
+		}
 	}
 
 	return nil, status.Error(codes.InvalidArgument, "no peer certs?")
@@ -864,8 +866,11 @@ func (self *ApiServer) Query(
 				peer_name, permissions))
 		}
 
-		// Cert is good enough for us, run the query.
-		return streamQuery(self.config, in, stream, peer_name)
+		// return the first good match
+		if true {
+			// Cert is good enough for us, run the query.
+			return streamQuery(self.config, in, stream, peer_name)
+		}
 	}
 
 	return status.Error(codes.InvalidArgument, "no peer certs?")
@@ -1075,7 +1080,7 @@ func startAPIServer(
 
 		err = grpcServer.Serve(lis)
 		if err != nil {
-			logger.Error("gRPC Server error", err)
+			logger.Error("gRPC Server error: %v", err)
 		}
 	}()
 
@@ -1115,7 +1120,7 @@ func StartMonitoringService(
 
 		err := server.ListenAndServe()
 		if err != nil && err != http.ErrServerClosed {
-			logger.Error("Prometheus monitoring server: ", err)
+			logger.Error("Prometheus monitoring server: %v", err)
 		}
 	}()
 
@@ -1133,7 +1138,7 @@ func StartMonitoringService(
 
 		err := server.Shutdown(timeout_ctx)
 		if err != nil {
-			logger.Error("Prometheus shutdown error ", err)
+			logger.Error("Prometheus shutdown error: %v", err)
 		}
 	}()
 

api/artifacts.go

@@ -299,12 +299,11 @@ func (self *ApiServer) LoadArtifactPack(
 				continue
 			}
 
-			artifact_definition := string(data)
-
 			// Make sure the artifact is written into the
 			// Packs part to prevent clashes with built in
 			// names.
-			artifact_definition = ensureArtifactPrefix(string(data), prefix)
+			artifact_definition := ensureArtifactPrefix(
+				string(data), prefix)
 
 			request := &api_proto.SetArtifactRequest{
 				Op:       api_proto.SetArtifactRequest_SET,

api/auth.go

@@ -26,6 +26,7 @@ import (
 	api_proto "www.velocidex.com/golang/velociraptor/api/proto"
 	config_proto "www.velocidex.com/golang/velociraptor/config/proto"
 	"www.velocidex.com/golang/velociraptor/json"
+	"www.velocidex.com/golang/velociraptor/logging"
 )
 
 var (
@@ -57,7 +58,13 @@ func GetGRPCUserInfo(
 					userinfo := md.Get("USER")
 					if len(userinfo) > 0 {
 						data := []byte(userinfo[0])
-						json.Unmarshal(data, result)
+						err := json.Unmarshal(data, result)
+						if err != nil {
+							logger := logging.GetLogger(config_obj,
+								&logging.FrontendComponent)
+							logger.Error("GetGRPCUserInfo: %v", err)
+							result.Name = ""
+						}
 					}
 				}
 			}

api/authenticators/azure.go

@@ -256,11 +256,16 @@ func oauthAzurePicture(config_obj *config_proto.Config) http.Handler {
 		response, err := azureOauthConfig.Client(r.Context(), oauth_token).Get(
 			"https://graph.microsoft.com/v1.0/me/photos/48x48/$value")
 		if err != nil {
-			reject(fmt.Errorf("failed getting photo: %s", err.Error()))
+			reject(fmt.Errorf("failed getting photo: %v", err))
 			return
 		}
 		defer response.Body.Close()
 
-		io.Copy(w, response.Body)
+		_, err = io.Copy(w, response.Body)
+		if err != nil {
+			reject(fmt.Errorf("failed getting photo: %v", err))
+			return
+		}
+
 	})
 }

api/authenticators/basic.go

@@ -9,6 +9,7 @@ import (
 	"www.velocidex.com/golang/velociraptor/acls"
 	api_proto "www.velocidex.com/golang/velociraptor/api/proto"
 	config_proto "www.velocidex.com/golang/velociraptor/config/proto"
+	"www.velocidex.com/golang/velociraptor/constants"
 	"www.velocidex.com/golang/velociraptor/json"
 	"www.velocidex.com/golang/velociraptor/logging"
 	"www.velocidex.com/golang/velociraptor/users"
@@ -106,7 +107,7 @@ func (self *BasicAuthenticator) AuthenticateUserHandler(
 		// binary data in metadata.
 		serialized, _ := json.Marshal(user_info)
 		ctx := context.WithValue(
-			r.Context(), "USER", string(serialized))
+			r.Context(), constants.GRPC_USER_CONTEXT, string(serialized))
 
 		// Need to call logging after auth so it can access
 		// the USER value in the context.

api/authenticators/google.go

@@ -95,7 +95,7 @@ func generateStateOauthCookie(w http.ResponseWriter) *http.Cookie {
 	var expiration = time.Now().Add(365 * 24 * time.Hour)
 
 	b := make([]byte, 16)
-	rand.Read(b)
+	_, _ = rand.Read(b)
 	state := base64.URLEncoding.EncodeToString(b)
 	cookie := http.Cookie{Name: "oauthstate", Value: state, Expires: expiration}
 	http.SetCookie(w, &cookie)

api/authenticators/saml.go

@@ -12,6 +12,7 @@ import (
 	"www.velocidex.com/golang/velociraptor/acls"
 	api_proto "www.velocidex.com/golang/velociraptor/api/proto"
 	config_proto "www.velocidex.com/golang/velociraptor/config/proto"
+	"www.velocidex.com/golang/velociraptor/constants"
 	"www.velocidex.com/golang/velociraptor/crypto"
 	"www.velocidex.com/golang/velociraptor/json"
 	"www.velocidex.com/golang/velociraptor/logging"
@@ -102,7 +103,8 @@ Contact your system administrator to get an account, then try again.
 
 			serialized, _ := json.Marshal(user_info)
 			ctx := context.WithValue(
-				r.Context(), "USER", string(serialized))
+				r.Context(), constants.GRPC_USER_CONTEXT,
+				string(serialized))
 			GetLoggingHandler(config_obj)(parent).ServeHTTP(
 				w, r.WithContext(ctx))
 			return

api/builder.go

@@ -294,8 +294,12 @@ func StartFrontendHttps(
 		defer cancel()
 
 		server.SetKeepAlivesEnabled(false)
-		services.GetNotifier().NotifyAllListeners(config_obj)
-		err := server.Shutdown(time_ctx)
+		err := services.GetNotifier().NotifyAllListeners(config_obj)
+		if err != nil {
+			server_obj.Error("Frontend server error", err)
+		}
+
+		err = server.Shutdown(time_ctx)
 		if err != nil {
 			server_obj.Error("Frontend server error", err)
 		}
@@ -356,8 +360,12 @@ func StartFrontendPlainHttp(
 		defer cancel()
 
 		server.SetKeepAlivesEnabled(false)
-		services.GetNotifier().NotifyAllListeners(config_obj)
-		err := server.Shutdown(time_ctx)
+		err := services.GetNotifier().NotifyAllListeners(config_obj)
+		if err != nil {
+			server_obj.Error("Frontend server error", err)
+		}
+
+		err = server.Shutdown(time_ctx)
 		if err != nil {
 			server_obj.Error("Frontend server error", err)
 		}
@@ -401,7 +409,13 @@ func StartFrontendWithAutocert(
 	}
 
 	// We must have port 80 open to serve the HTTP 01 challenge.
-	go http.ListenAndServe(":http", certManager.HTTPHandler(nil))
+	go func() {
+		err := http.ListenAndServe(":http", certManager.HTTPHandler(nil))
+		if err != nil {
+			logger := logging.Manager.GetLogger(config_obj, &logging.GUIComponent)
+			logger.Error("Failed to bind to http server: %v", err)
+		}
+	}()
 
 	wg.Add(1)
 	go func() {
@@ -412,7 +426,7 @@ func StartFrontendWithAutocert(
 
 		err := server.ListenAndServeTLS("", "")
 		if err != nil && err != http.ErrServerClosed {
-			logger.Error("Frontend server error", err)
+			logger.Error("Frontend server error: %v", err)
 		}
 	}()
 
@@ -429,10 +443,10 @@ func StartFrontendWithAutocert(
 		defer cancel()
 
 		server.SetKeepAlivesEnabled(false)
-		services.GetNotifier().NotifyAllListeners(config_obj)
+		_ = services.GetNotifier().NotifyAllListeners(config_obj)
 		err := server.Shutdown(timeout_ctx)
 		if err != nil {
-			logger.Error("Frontend shutdown error ", err)
+			logger.Error("Frontend shutdown error: %v", err)
 		}
 		server_obj.Info("Shutdown frontend")
 	}()
@@ -471,7 +485,7 @@ func StartHTTPGUI(
 
 		err := server.ListenAndServe()
 		if err != nil && err != http.ErrServerClosed {
-			logger.Error("GUI Server error", err)
+			logger.Error("GUI Server error: %v", err)
 		}
 	}()
 
@@ -488,7 +502,7 @@ func StartHTTPGUI(
 		server.SetKeepAlivesEnabled(false)
 		err := server.Shutdown(timeout_ctx)
 		if err != nil {
-			logger.Error("GUI shutdown error ", err)
+			logger.Error("GUI shutdown error: %v", err)
 		}
 	}()
 
@@ -547,7 +561,7 @@ func StartSelfSignedGUI(
 
 		err := server.ListenAndServeTLS("", "")
 		if err != nil && err != http.ErrServerClosed {
-			logger.Error("GUI Server error", err)
+			logger.Error("GUI Server error: %v", err)
 		}
 	}()
 
@@ -564,7 +578,7 @@ func StartSelfSignedGUI(
 		server.SetKeepAlivesEnabled(false)
 		err := server.Shutdown(timeout_ctx)
 		if err != nil {
-			logger.Error("GUI shutdown error ", err)
+			logger.Error("GUI shutdown error: %v", err)
 		}
 	}()
 

api/csrf.go

@@ -24,7 +24,7 @@ func csrfProtect(config_obj *config_proto.Config,
 
 	// Derive a CSRF key from the hash of the server's public key.
 	hasher := sha256.New()
-	hasher.Write([]byte(config_obj.Frontend.PrivateKey))
+	_, _ = hasher.Write([]byte(config_obj.Frontend.PrivateKey))
 	token := hasher.Sum(nil)
 
 	protectionFn := csrf.Protect(token, csrf.Path("/"))