From e5ad87a585b776e09bbe57de0141524aa7868c3d Mon Sep 17 00:00:00 2001
From: Daniel <dhaavi@users.noreply.github.com>
Date: Wed, 3 Jun 2020 21:21:34 +0200
Subject: [PATCH] Increase resilience of api auth by waiting long and trying
 more often

Also, improve messages sent to user.
---
 firewall/api.go | 93 ++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 69 insertions(+), 24 deletions(-)

diff --git a/firewall/api.go b/firewall/api.go
index b435e7db..ead1495d 100644
--- a/firewall/api.go
+++ b/firewall/api.go
@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/safing/portbase/api"
 	"github.com/safing/portbase/dataroot"
@@ -40,48 +41,71 @@ func startAPIAuth() {
 	log.Tracef("filter: api port set to %d", apiPort)
 }
 
-func apiAuthenticator(s *http.Server, r *http.Request) (grantAccess bool, err error) {
+func apiAuthenticator(s *http.Server, r *http.Request) (err error) {
 	if devMode() {
-		return true, nil
+		return nil
 	}
 
 	// get local IP/Port
 	localIP, localPort, err := parseHostPort(s.Addr)
 	if err != nil {
-		return false, fmt.Errorf("failed to get local IP/Port: %s", err)
+		return fmt.Errorf("failed to get local IP/Port: %s", err)
 	}
 
 	// get remote IP/Port
 	remoteIP, remotePort, err := parseHostPort(r.RemoteAddr)
 	if err != nil {
-		return false, fmt.Errorf("failed to get remote IP/Port: %s", err)
+		return fmt.Errorf("failed to get remote IP/Port: %s", err)
 	}
 
+	// It is very important that this works, retry extensively (every 250ms for 5s)
+	for tries := 0; tries < 20; tries++ {
+		err = authenticateAPIRequest(
+			r.Context(),
+			&packet.Info{
+				Inbound:  false, // outbound as we are looking for the process of the source address
+				Version:  packet.IPv4,
+				Protocol: packet.TCP,
+				Src:      remoteIP,   // source as in the process we are looking for
+				SrcPort:  remotePort, // source as in the process we are looking for
+				Dst:      localIP,
+				DstPort:  localPort,
+			},
+		)
+		if err != nil {
+			return nil
+		}
+
+		// wait a little
+		time.Sleep(250 * time.Millisecond)
+	}
+
+	return err
+}
+
+func authenticateAPIRequest(ctx context.Context, pktInfo *packet.Info) error {
 	var procsChecked []string
 
 	// get process
-	proc, _, err := process.GetProcessByConnection(
-		r.Context(),
-		&packet.Info{
-			Inbound:  false, // outbound as we are looking for the process of the source address
-			Version:  packet.IPv4,
-			Protocol: packet.TCP,
-			Src:      remoteIP,   // source as in the process we are looking for
-			SrcPort:  remotePort, // source as in the process we are looking for
-			Dst:      localIP,
-			DstPort:  localPort,
-		},
-	)
+	proc, _, err := process.GetProcessByConnection(ctx, pktInfo)
 	if err != nil {
-		return false, fmt.Errorf("failed to get process: %s", err)
+		return fmt.Errorf("failed to get process: %s", err)
 	}
+	originalPid := proc.Pid
 
 	// go up up to two levels, if we don't match
-	for i := 0; i < 3; i++ {
-		// check if the requesting process is in database root / updates dir
-		if strings.HasPrefix(proc.Path, dataRoot.Path) {
-			return true, nil
+	for i := 0; i < 5; i++ {
+		// check for eligible PID
+		switch proc.Pid {
+		case process.UnidentifiedProcessID, process.SystemProcessID:
+			break
+		default: // normal process
+			// check if the requesting process is in database root / updates dir
+			if strings.HasPrefix(proc.Path, dataRoot.Path) {
+				return nil
+			}
 		}
+
 		// add checked process to list
 		procsChecked = append(procsChecked, proc.Path)
 
@@ -89,13 +113,34 @@ func apiAuthenticator(s *http.Server, r *http.Request) (grantAccess bool, err er
 			// get parent process
 			proc, err = process.GetOrFindProcess(context.Background(), proc.ParentPid)
 			if err != nil {
-				return false, fmt.Errorf("failed to get process: %s", err)
+				return fmt.Errorf("failed to get process: %s", err)
 			}
 		}
 	}
 
-	log.Debugf("filter: denying api access to %s - also checked %s (trusted root is %s)", procsChecked[0], strings.Join(procsChecked[1:], " "), dataRoot.Path)
-	return false, nil
+	switch originalPid {
+	case process.UnidentifiedProcessID:
+		log.Warningf("filter: denying api access: failed to identify process")
+		return fmt.Errorf("%wFailed to identify the requesting process. You can enable the Development Mode to disable API authentication for development purposes.", api.ErrAPIAccessDeniedMessage)
+
+	case process.SystemProcessID:
+		log.Warningf("filter: denying api access: request by system")
+		return fmt.Errorf("%wSystem access to the Portmaster API is not permitted. You can enable the Development Mode to disable API authentication for development purposes.", api.ErrAPIAccessDeniedMessage)
+
+	default: //
+		log.Warningf("filter: denying api access to %s - also checked %s (trusted root is %s)", procsChecked[0], strings.Join(procsChecked[1:], " "), dataRoot.Path)
+		return fmt.Errorf(
+			`%wThe requesting process is not authorized to access the Portmaster API.
+Checked process paths:
+%s
+
+The authorithed root path is %s.
+You can enable the Development Mode to disable API authentication for development purposes.`,
+			api.ErrAPIAccessDeniedMessage,
+			dataRoot.Path,
+			strings.Join(procsChecked, "\n"),
+		)
+	}
 }
 
 func parseHostPort(address string) (net.IP, uint16, error) {