Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved search for network addresses within a given string #355

Merged
merged 3 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
9 changes: 9 additions & 0 deletions cmd/nettop/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,15 @@ var (
false,
[]string{"qotd", "expected_netpol_output.json"},
},
{
"CronJobWithNontrivialNetworkAddresses",
[][]string{{"openshift", "openshift-operator-lifecycle-manager-resources.yaml"}},
yamlFormat,
true,
[]string{"-v"},
false,
[]string{"openshift", "expected_netpol_output.yaml"},
},
{
"SpecifyDNSPort",
[][]string{{"acs-security-demos"}},
Expand Down
40 changes: 39 additions & 1 deletion pkg/analyzer/info_to_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"net/url"
"strconv"
"strings"
"unicode"

ocroutev1 "github.com/openshift/api/route/v1"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -218,16 +219,35 @@ func appendNetworkAddresses(networkAddresses, values []string) []string {
// because it decides if the given string is an evidence for a potentially required connectivity.
// If it succeeds, a "cleaned" network address is returned as a string, together with the value true.
// Otherwise (there does not seem to be a network address in "value"), it returns "" with the value false.
// As value may be in the form of e.g. "key:val" where "val" holds the network address, we will check several possible suffixes.
func networkAddressFromStr(value string) (string, bool) {
suffixes := possibleSuffixes(value)
for _, suffix := range suffixes {
addr, ok := networkAddressFromSuffix(suffix)
if ok {
return addr, ok
}
}
return "", false
}

func networkAddressFromSuffix(value string) (string, bool) {
host, err := getHostFromURL(value)
if err != nil {
return "", false // value cannot be interpreted as a URL
}

hostNoPort := host
colonPos := strings.Index(host, ":")
if colonPos >= 0 {
if colonPos >= 0 { // host includes port number or port name
hostNoPort = host[:colonPos]
port := host[colonPos+1:] // now validate the port
if len(validation.IsValidPortName(port)) > 0 {
portInt, _ := strconv.Atoi(port)
if len(validation.IsValidPortNum(portInt)) > 0 {
return "", false
}
}
}

errs := validation.IsDNS1123Subdomain(hostNoPort)
Expand All @@ -242,6 +262,24 @@ func networkAddressFromStr(value string) (string, bool) {
return host, true
}

// Sometimes the given value includes the network address as its suffix.
// For example, a command-line arg may look like "server-addr=my_server:5000"
// If we are unable to convert "value" to a network address, we may also want to check its suffixes.
// This function returns all suffixes that start with a letter, and have ':', ' ' or '=' just before this initial letter.
func possibleSuffixes(value string) []string {
res := []string{value}

var prevRune rune
for i, r := range value {
if i > 1 && unicode.IsLetter(r) && (prevRune == ':' || prevRune == '=' || prevRune == ' ') {
res = append(res, value[i:])
}
prevRune = r
}

return res
}

// Attempts to parse the given string as a URL, and extract its Host part.
// Returns an error if the string cannot be interpreted as a URL
func getHostFromURL(urlStr string) (string, error) {
Expand Down
10 changes: 7 additions & 3 deletions pkg/analyzer/info_to_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ func TestNetworkAddressValue(t *testing.T) {
strings.Repeat("abc", 500): {"", false},
"not%a*url": {"", false},
"123": {"", false},
"olm-operator-heap-:https://olm-operator-metrics:8443/debug/pprof/heap": {"olm-operator-metrics:8443", true},
"-server=my-server:5024": {"my-server:5024", true},
"-server=my-server:502%4": {"", false}, // port number is invalid
}

for val, expectedAnswer := range valuesToCheck {
Expand All @@ -53,8 +56,9 @@ func TestScanningDeploymentWithArgs(t *testing.T) {
res, err := k8sWorkloadObjectFromInfo(resourceInfo)
require.Nil(t, err)
require.Equal(t, "carts", res.Resource.Name)
require.Len(t, res.Resource.NetworkAddrs, 1)
require.Equal(t, "carts-db:27017", res.Resource.NetworkAddrs[0])
require.Len(t, res.Resource.NetworkAddrs, 2)
require.Equal(t, "false", res.Resource.NetworkAddrs[0])
require.Equal(t, "carts-db:27017", res.Resource.NetworkAddrs[1])
require.Len(t, res.Resource.Labels, 1)
require.Equal(t, "carts", res.Resource.Labels["name"])
}
Expand Down Expand Up @@ -124,7 +128,7 @@ func TestScanningCronJob(t *testing.T) {
require.Nil(t, err)
require.Equal(t, "collect-profiles", res.Resource.Name)
require.Equal(t, cronJob, res.Resource.Kind)
require.Len(t, res.Resource.NetworkAddrs, 1)
require.Len(t, res.Resource.NetworkAddrs, 3)
require.Len(t, res.Resource.Labels, 0)
}

Expand Down
97 changes: 97 additions & 0 deletions tests/openshift/expected_netpol_output.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
apiVersion: networking.k8s.io/v1
items:
- apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
creationTimestamp: null
name: catalog-operator-netpol
namespace: openshift-operator-lifecycle-manager
spec:
ingress:
- from:
- podSelector: {}
ports:
- port: 8443
protocol: TCP
podSelector:
matchLabels:
app: catalog-operator
policyTypes:
- Ingress
- Egress
- apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
creationTimestamp: null
name: collect-profiles-netpol
namespace: openshift-operator-lifecycle-manager
spec:
egress:
- ports:
- port: 8443
protocol: TCP
to:
- podSelector:
matchLabels:
app: catalog-operator
- ports:
- port: 8443
protocol: TCP
to:
- podSelector:
matchLabels:
app: olm-operator
- ports:
- port: 53
protocol: UDP
to:
- namespaceSelector: {}
podSelector: {}
policyTypes:
- Ingress
- Egress
- apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
creationTimestamp: null
name: olm-operator-netpol
namespace: openshift-operator-lifecycle-manager
spec:
ingress:
- from:
- podSelector: {}
ports:
- port: 8443
protocol: TCP
podSelector:
matchLabels:
app: olm-operator
policyTypes:
- Ingress
- Egress
- apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
creationTimestamp: null
name: packageserver-netpol
namespace: openshift-operator-lifecycle-manager
spec:
podSelector:
matchLabels:
app: packageserver
policyTypes:
- Ingress
- Egress
- apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
creationTimestamp: null
name: default-deny-in-namespace-openshift-operator-lifecycle-manager
namespace: openshift-operator-lifecycle-manager
spec:
podSelector: {}
policyTypes:
- Ingress
- Egress
kind: NetworkPolicyList
metadata: {}
Loading