Refactoring and more comments

firewall/interception/windowskext/handler.go

@@ -1,3 +1,4 @@
+//go:build windows
 // +build windows
 
 package windowskext
@@ -10,6 +11,7 @@ import (
 	"github.com/tevino/abool"
 
 	"github.com/safing/portbase/log"
+	"github.com/safing/portmaster/network"
 	"github.com/safing/portmaster/network/packet"
 )
 
@@ -43,6 +45,11 @@ type VerdictRequest struct {
 	packetSize uint32
 }
 
+type VerdictInfo struct {
+	id      uint32          // ID from RegisterPacket
+	verdict network.Verdict // verdict for the connection
+}
+
 // Handler transforms received packets to the Packet interface.
 func Handler(packets chan packet.Packet) {
 	if !ready.IsSet() {

firewall/interception/windowskext/kext.go

@@ -70,8 +70,8 @@ func Start() error {
 	kextHandle, err = openDriver(filename)
 
 	// close the service handles
-	windows.DeleteService(service)
+	_ = windows.DeleteService(service)
-	windows.CloseServiceHandle(service)
+	_ = windows.CloseServiceHandle(service)
 
 	// driver was not installed
 	if err != nil {
@@ -95,7 +95,11 @@ func Stop() error {
 	if err != nil {
 		log.Errorf("winkext: failed to close the handle: %s", err)
 	}
-	_, _ = exec.Command("sc", "stop", driverName).Output()
+
+	_, err = exec.Command("sc", "stop", driverName).Output() // This is a question of taste, but it is a robust and solid solution
+	if err != nil {
+		log.Errorf("winkext: failed to stop the service: %q", err)
+	}
 	return nil
 }
 
@@ -118,8 +122,10 @@ func RecvVerdictRequest() (*VerdictRequest, error) {
 	}
 
 	timestamp := time.Now()
+	// Initialize struct for the output data
 	var new VerdictRequest
 
+	// Make driver request
 	data := asByteArray(&new)
 	bytesRead, err := deviceIoControlRead(kextHandle, IOCTL_RECV_VERDICT_REQ, data)
 	if err != nil {
@@ -147,11 +153,9 @@ func SetVerdict(pkt *Packet, verdict network.Verdict) error {
 		return ErrKextNotReady
 	}
 
-	verdictInfo := struct {
+	verdictInfo := VerdictInfo{pkt.verdictRequest.id, verdict}
-		id      uint32
-		verdict network.Verdict
-	}{pkt.verdictRequest.id, verdict}
 
+	// Make driver request
 	atomic.AddInt32(urgentRequests, 1)
 	data := asByteArray(&verdictInfo)
 	_, err := deviceIoControlWrite(kextHandle, IOCTL_SET_VERDICT, data)
@@ -169,6 +173,7 @@ func GetPayload(packetID uint32, packetSize uint32) ([]byte, error) {
 		return nil, ErrNoPacketID
 	}
 
+	// Check if driver is initialized
 	kextLock.RLock()
 	defer kextLock.RUnlock()
 	if !ready.IsSet() {
@@ -177,11 +182,13 @@ func GetPayload(packetID uint32, packetSize uint32) ([]byte, error) {
 
 	buf := make([]byte, packetSize)
 
+	// Combine id and length
 	payload := struct {
 		id     uint32
 		length uint32
 	}{packetID, packetSize}
 
+	// Make driver request
 	atomic.AddInt32(urgentRequests, 1)
 	data := asByteArray(&payload)
 	bytesRead, err := deviceIoControlReadWrite(kextHandle, IOCTL_GET_PAYLOAD, data, unsafe.Slice(&buf[0], packetSize))
@@ -192,6 +199,7 @@ func GetPayload(packetID uint32, packetSize uint32) ([]byte, error) {
 		return nil, err
 	}
 
+	// check the result and return
 	if bytesRead == 0 {
 		return nil, errors.New("windows kext did not return any data")
 	}
@@ -206,11 +214,14 @@ func GetPayload(packetID uint32, packetSize uint32) ([]byte, error) {
 func ClearCache() error {
 	kextLock.RLock()
 	defer kextLock.RUnlock()
+
+	// Check if driver is initialized
 	if !ready.IsSet() {
 		log.Error("kext: failed to clear the cache: kext not ready")
 		return ErrKextNotReady
 	}
 
+	// Make driver request
 	_, err := deviceIoControlRead(kextHandle, IOCTL_CLEAR_CACHE, nil)
 	return err
 }

firewall/interception/windowskext/service.go

@@ -15,10 +15,13 @@ func createService(manager windows.Handle, portmasterKextPath *uint16) (windows.
 	if err != nil {
 		return 0, fmt.Errorf("Bad service: %s", err)
 	}
+	// Check if it's already created
 	service, err := windows.OpenService(manager, &u16filename[0], windows.SERVICE_ALL_ACCESS)
 	if err == nil {
 		return service, nil
 	}
+
+	// Create the service
 	service, err = windows.CreateService(manager, &u16filename[0], &u16filename[0], windows.SERVICE_ALL_ACCESS, windows.SERVICE_KERNEL_DRIVER, windows.SERVICE_DEMAND_START, windows.SERVICE_ERROR_NORMAL, portmasterKextPath, nil, nil, nil, nil, nil)
 	if err != nil {
 		return 0, err
@@ -36,6 +39,7 @@ func driverInstall(portmasterKextPath string) (windows.Handle, error) {
 	}
 	defer windows.CloseServiceHandle(manager)
 
+	// Try to create the service. Retry if it fails.
 	var service windows.Handle
 retryLoop:
 	for i := 0; i < 3; i++ {
@@ -49,20 +53,18 @@ retryLoop:
 		return 0, fmt.Errorf("Failed to create service: %s", err)
 	}
 
-	err = windows.StartService(service, 0, nil)
 	// Start the service:
+	err = windows.StartService(service, 0, nil)
+
 	if err != nil {
 		err = windows.GetLastError()
-		if err == windows.ERROR_SERVICE_ALREADY_RUNNING {
+		if err != windows.ERROR_SERVICE_ALREADY_RUNNING {
-			// windows.SetLastError(0)
-		} else {
 			// Failed to start service; clean-up:
 			var status windows.SERVICE_STATUS
 			_ = windows.ControlService(service, windows.SERVICE_CONTROL_STOP, &status)
 			_ = windows.DeleteService(service)
 			_ = windows.CloseServiceHandle(service)
 			service = 0
-			//windows.SetLastError(err)
 		}
 	}