safing / portmaster

🏔 Love Freedom - ❌ Block Mass Surveillance
https://safing.io
GNU General Public License v3.0
9.4k stars 305 forks source link

Make changed rules apply to existing connections, not only new ones. #467

Closed SirRFI closed 1 year ago

SirRFI commented 2 years ago

What happened:

Adding app specific rules (block IP) work only after restarting the app.

What did you expect to happen?:

The rule should take effect immediately.

How did you reproduce it?:

  1. Run Windows 11 PRO (21H2, build 22000.318) on a Virtual Machine, which has bridged connection to the network.
  2. Start simple HTTP server on host OS:
    
    package main

import ( "fmt" "net/http" "time" )

func main() { http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { fmt.Println(time.Now().Format("15:04:05.000")+" Received request") })

fmt.Println(time.Now().Format("15:04:05.000")+" Serving")
http.ListenAndServe(":8000", nil)

}

2. Run simple HTTP client on guest OS (ran it as exe):
```go
package main

import (
    "fmt"
    "net/http"
    "time"
)

func main() {
    client := http.Client{Timeout: time.Millisecond*900}

    for {
        go func() {
            fmt.Println(time.Now().Format("15:04:05.000")+" Sending request...")
            _, err := client.Get("http://192.168.0.3:8000/")
            if err != nil {
                fmt.Println(time.Now().Format("15:04:05.000")+" Request failed")
                return
            }
            fmt.Println(time.Now().Format("15:04:05.000")+" Request succeed")
        }()
        time.Sleep(time.Second)
    }
}
  1. In Portmaster (on guest OS) add rule to block IP 192.168.0.3 for the HTTP client program (client.exe) - this is when the client should start failing
  2. Restart the HTTP client - the rule will work, and so program will be failing
  3. Remove the block rule from Portmaster - rule removal will take effect immediatelly, and so program will work again

portmaster

I tried this on browser, but being unsure whether cache is in place, I wrote the sample programs above.

Debug Information:

Version 0.7.10 ``` Portmaster version 0.7.10 commit tags/v0.7.10-0-gbb3591a7aeb47d6ccc0182936c4a0523ccdd89e4 built with go1.15.8 (gc) windows/amd64 using options main.go by user@docker on 06.12.2021 Licensed under the AGPLv3 license. The source code is available here: https://github.com/safing/portmaster ```
Platform: Microsoft Windows 11 Pro 10.0.22000 Build 22000 ``` System: Microsoft Windows 11 Pro windows (Standalone Workstation) 10.0.22000 Build 22000 Kernel: 10.0.22000 Build 22000 x86_64 ```
Status: Trusted ``` ActiveSecurityLevel: Trusted SelectedSecurityLevel: Off ThreatMitigationLevel: Trusted CaptivePortal: OnlineStatus: Online ```
No Module Error
Unexpected Logs ``` 211212 00:02:36.480 pat/notify:115 > WARN 097 compat: detected secure dns bypass %s issue with C:\Windows\System32\svchost.exe 211212 00:02:36.686 pat/notify:115 > WARN 102 compat: detected secure dns bypass %s issue with C:\Windows\System32\svchost.exe 211212 00:02:43.299 olver-mdns:094 > WARN 122 intel(mdns): failed to create udp6 listen multicast socket: listen udp6 [ff02::fb]:5353: setsockopt: not supported by windows 211212 00:08:30.941 CURRENT TIME ```
dhaavi commented 2 years ago

Hey @SirRFI, thanks for the detailed report!

I think that your example program actually only uses one connection and does not create new network connections. http.Get() uses the default http.Client, which reuses connections, afaik. So the Portmaster does not block it, because there is no new connection.

Can you try again, but create a new client for every request, or enable DisableKeepAlives?

SirRFI commented 2 years ago

Actually I used HTTP for simplicity and to see if my initial browser-based test is wrong. Indeed, introducing the changes you suggested changed the behavior. With the following, new requests will be blocked immediately:

package main

import (
    "fmt"
    "net/http"
    "time"
)

func main() {
    mux := http.NewServeMux()
    mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
        fmt.Println(time.Now().Format("15:04:05.000")+" Received request")
    })

    server := http.Server{Addr: ":8000", Handler: mux}
    server.SetKeepAlivesEnabled(false)

    fmt.Println(time.Now().Format("15:04:05.000")+" Serving")
    server.ListenAndServe()
}
package main

import (
    "fmt"
    "net/http"
    "time"
)

func main() {
    for {
        go func() {
            fmt.Println(time.Now().Format("15:04:05.000")+" Sending request...")
            client := http.Client{Timeout: time.Millisecond*900}
            _, err := client.Get("http://192.168.0.3:8000/")
            if err != nil {
                fmt.Println(time.Now().Format("15:04:05.000")+" Request failed")
                return
            }
            fmt.Println(time.Now().Format("15:04:05.000")+" Request succeed")
        }()
        time.Sleep(time.Second)
    }
}

What about ongoing connections, like in the first example? Does it mean that Portmaster rules affect only new connections? I expected the ongoing connection to be blocked, so first example is still somewhat valid.

dhaavi commented 2 years ago

Does it mean that Portmaster rules affect only new connections?

Yes, that is correct.

I expected the ongoing connection to be blocked, so first example is still somewhat valid.

The thing is, we can't change the past, so we can't "change" a connection to blocked. Also, depending on what triggered the decision, the Portmaster might not be able to come to the same conclusion as before as the data is missing.

However, what we'd like to do in the future is to force applications to make a new connection when settings change. This way we don't change the past and we can re evaluate all connections.

We currently don't do that because it would be very complex in regards to the system integration we have now.

SirRFI commented 2 years ago

The thing is, we can't change the past, so we can't "change" a connection to blocked. [...] However, what we'd like to do in the future is to force applications to make a new connection when settings change.

How about terminating connections that would be affected by newly added block rules? Or Portmaster does not remember information about connections to do such thing?

SimpleWall's behavior for comparison (the 1st pair of Go code, second wouldn't even display client.exe for some reason, but only connections) simplewall

dhaavi commented 2 years ago

Well, re-evaluating connections is not that easy, as this might depend on the first packet of the connection, when analyzing encryption other things of the connection.

We are thinking about ways to just kill all connections of an app when something changes and the app then just has to retry. But this is not trivial with our current system integration, so it will take a while.

I will convert this to a suggestion and talk more with the team about it.

Raphty commented 1 year ago

I am cleaning out old issues. If you feel this issue should not have been closed let me know.

Please keep in mind, the free version of Portmaster only has limited support. For free users our active Discord community as well as the chat bot are the fastest and best way to get their help. https://discord.gg/safing If you find our work brings value to you, please consider supporting it by purchasing Plus or Pro Packages https://safing.io/pricing/. If you are already a subscriber, first Thank You! and also if you want priority support pleas send in an email and let me know your username so I can prioritize your request accordingly.