Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 41 additions & 5 deletions client/internal/auth/pkce_flow.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"net"
"net/http"
"net/url"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -46,9 +47,10 @@ type PKCEAuthorizationFlow struct {
func NewPKCEAuthorizationFlow(config internal.PKCEAuthProviderConfig) (*PKCEAuthorizationFlow, error) {
var availableRedirectURL string

// find the first available redirect URL
excludedRanges := getSystemExcludedPortRanges()

for _, redirectURL := range config.RedirectURLs {
if !isRedirectURLPortUsed(redirectURL) {
if !isRedirectURLPortUsed(redirectURL, excludedRanges) {
availableRedirectURL = redirectURL
break
}
Expand Down Expand Up @@ -282,15 +284,22 @@ func createCodeChallenge(codeVerifier string) string {
return base64.RawURLEncoding.EncodeToString(sha2[:])
}

// isRedirectURLPortUsed checks if the port used in the redirect URL is in use.
func isRedirectURLPortUsed(redirectURL string) bool {
// isRedirectURLPortUsed checks if the port used in the redirect URL is in use or excluded on Windows.
func isRedirectURLPortUsed(redirectURL string, excludedRanges []excludedPortRange) bool {
parsedURL, err := url.Parse(redirectURL)
if err != nil {
log.Errorf("failed to parse redirect URL: %v", err)
return true
Comment thread
lixmal marked this conversation as resolved.
}

addr := fmt.Sprintf(":%s", parsedURL.Port())
port := parsedURL.Port()

if isPortInExcludedRange(port, excludedRanges) {
log.Warnf("port %s is in Windows excluded port range, skipping", port)
return true
}

addr := fmt.Sprintf(":%s", port)
conn, err := net.DialTimeout("tcp", addr, 3*time.Second)
if err != nil {
return false
Expand All @@ -304,6 +313,33 @@ func isRedirectURLPortUsed(redirectURL string) bool {
return true
}

// excludedPortRange represents a range of excluded ports.
type excludedPortRange struct {
start int
end int
}

// isPortInExcludedRange checks if the given port is in any of the excluded ranges.
func isPortInExcludedRange(port string, excludedRanges []excludedPortRange) bool {
if len(excludedRanges) == 0 {
return false
}

portNum, err := strconv.Atoi(port)
if err != nil {
log.Debugf("invalid port number %s: %v", port, err)
return false
}

for _, r := range excludedRanges {
if portNum >= r.start && portNum <= r.end {
return true
}
}

return false
}

func renderPKCEFlowTmpl(w http.ResponseWriter, authError error) {
tmpl, err := template.New("pkce-auth-flow").Parse(templates.PKCEAuthMsgTmpl)
if err != nil {
Expand Down
8 changes: 8 additions & 0 deletions client/internal/auth/pkce_flow_other.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//go:build !windows

package auth

// getSystemExcludedPortRanges returns nil on non-Windows platforms.
func getSystemExcludedPortRanges() []excludedPortRange {
return nil
}
147 changes: 147 additions & 0 deletions client/internal/auth/pkce_flow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ package auth

import (
"context"
"fmt"
"net"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/netbirdio/netbird/client/internal"
Expand Down Expand Up @@ -69,3 +72,147 @@ func TestPromptLogin(t *testing.T) {
})
}
}

func TestIsPortInExcludedRange(t *testing.T) {
tests := []struct {
name string
port string
excludedRanges []excludedPortRange
expectedBlocked bool
}{
{
name: "Port in excluded range",
port: "8080",
excludedRanges: []excludedPortRange{{start: 8000, end: 8100}},
expectedBlocked: true,
},
{
name: "Port at start of range",
port: "8000",
excludedRanges: []excludedPortRange{{start: 8000, end: 8100}},
expectedBlocked: true,
},
{
name: "Port at end of range",
port: "8100",
excludedRanges: []excludedPortRange{{start: 8000, end: 8100}},
expectedBlocked: true,
},
{
name: "Port before range",
port: "7999",
excludedRanges: []excludedPortRange{{start: 8000, end: 8100}},
expectedBlocked: false,
},
{
name: "Port after range",
port: "8101",
excludedRanges: []excludedPortRange{{start: 8000, end: 8100}},
expectedBlocked: false,
},
{
name: "Empty excluded ranges",
port: "8080",
excludedRanges: []excludedPortRange{},
expectedBlocked: false,
},
{
name: "Nil excluded ranges",
port: "8080",
excludedRanges: nil,
expectedBlocked: false,
},
{
name: "Multiple ranges - port in second range",
port: "9050",
excludedRanges: []excludedPortRange{
{start: 8000, end: 8100},
{start: 9000, end: 9100},
},
expectedBlocked: true,
},
{
name: "Multiple ranges - port not in any range",
port: "8500",
excludedRanges: []excludedPortRange{
{start: 8000, end: 8100},
{start: 9000, end: 9100},
},
expectedBlocked: false,
},
{
name: "Invalid port string",
port: "invalid",
excludedRanges: []excludedPortRange{{start: 8000, end: 8100}},
expectedBlocked: false,
},
{
name: "Empty port string",
port: "",
excludedRanges: []excludedPortRange{{start: 8000, end: 8100}},
expectedBlocked: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := isPortInExcludedRange(tt.port, tt.excludedRanges)
assert.Equal(t, tt.expectedBlocked, result, "Port exclusion check mismatch")
})
}
}

func TestIsRedirectURLPortUsed(t *testing.T) {
listener, err := net.Listen("tcp", "127.0.0.1:0")
require.NoError(t, err)
defer func() {
_ = listener.Close()
}()

usedPort := listener.Addr().(*net.TCPAddr).Port

tests := []struct {
name string
redirectURL string
excludedRanges []excludedPortRange
expectedUsed bool
}{
{
name: "Port in excluded range",
redirectURL: "http://127.0.0.1:8080/",
excludedRanges: []excludedPortRange{{start: 8000, end: 8100}},
expectedUsed: true,
},
{
name: "Port actually in use",
redirectURL: fmt.Sprintf("http://127.0.0.1:%d/", usedPort),
excludedRanges: nil,
expectedUsed: true,
},
{
name: "Port not in use and not excluded",
redirectURL: "http://127.0.0.1:65432/",
excludedRanges: nil,
expectedUsed: false,
},
{
name: "Invalid URL without port",
redirectURL: "not-a-valid-url",
excludedRanges: nil,
expectedUsed: false,
},
{
name: "Port excluded even if not in use",
redirectURL: "http://127.0.0.1:8050/",
excludedRanges: []excludedPortRange{{start: 8000, end: 8100}},
expectedUsed: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := isRedirectURLPortUsed(tt.redirectURL, tt.excludedRanges)
assert.Equal(t, tt.expectedUsed, result, "Port usage check mismatch")
})
}
}
64 changes: 64 additions & 0 deletions client/internal/auth/pkce_flow_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
//go:build windows

package auth

import (
"fmt"
"strconv"
"strings"

log "github.com/sirupsen/logrus"
"golang.org/x/sys/windows/registry"
)

// getSystemExcludedPortRanges retrieves the excluded port ranges from Windows.
func getSystemExcludedPortRanges() []excludedPortRange {
ranges, err := getExcludedPortRangesFromRegistry()
if err == nil && len(ranges) > 0 {
return ranges
}

return ranges
}

// getExcludedPortRangesFromRegistry retrieves excluded port ranges from Windows registry.
func getExcludedPortRangesFromRegistry() ([]excludedPortRange, error) {
k, err := registry.OpenKey(registry.LOCAL_MACHINE,
`SYSTEM\CurrentControlSet\Services\Tcpip\Parameters`,
registry.QUERY_VALUE)
if err != nil {
return nil, fmt.Errorf("open registry key: %w", err)
}
defer func() {
if err := k.Close(); err != nil {
log.Debugf("failed to close registry key: %v", err)
}
}()

reservedPorts, _, err := k.GetStringsValue("ReservedPorts")
if err != nil {
return nil, fmt.Errorf("read ReservedPorts: %w", err)
}

var ranges []excludedPortRange
for _, portSpec := range reservedPorts {
parts := strings.Split(portSpec, "-")
if len(parts) != 2 {
continue
}

start, err := strconv.Atoi(strings.TrimSpace(parts[0]))
if err != nil {
continue
}

end, err := strconv.Atoi(strings.TrimSpace(parts[1]))
if err != nil {
continue
}

ranges = append(ranges, excludedPortRange{start: start, end: end})
}

return ranges, nil
}
Loading
Loading