Skip to content

Commit

Permalink
refactor(transparent-proxy): improvements in table builders (#10870)
Browse files Browse the repository at this point in the history
- Add Multiport module validation to `Functionality` struct

  This commit enhances the `Functionality` struct by incorporating
  validation for the Multiport module. This update allows the system to
  determine whether the Multiport module is supported, enabling more
  efficient handling of port exclusions.

- Add `NewValueOrRangeList` function

  This commit introduces the `NewValueOrRangeList` function, which
  provides a flexible and type-safe way to create a `ValueOrRangeList`
  from different input types. This function supports input as a slice of
  `uint16`, a single `uint16` value, or a string, allowing for more
  versatile and efficient handling of port and value ranges.

- Refactor logic for inserting rules at specific positions

  This refactoring removes the need to manage rule positions externally.
  Now, the `rules.NewInsertRule` function is used to directly create
  rules that need to be inserted at specific positions within the chain.
  This change simplifies the code and reduces the complexity of managing
  rule positions manually.
  
  Key changes:
  - Replaced manual position tracking with `rules.NewInsertRule` function.
  - Simplified rule insertion logic by handling positioning within the
    rule builders.

- Rename `rules.NewRule` to `rules.NewAppendRule`

  This commit refactors the function name `rules.NewRule` to
  `rules.NewAppendRule` to better reflect its purpose of creating rules
  that are appended to the end of an existing chain. This change improves
  code readability and clarity, making it more evident when a rule is
  being appended versus inserted.

  Key Changes:
  - Renamed `rules.NewRule` to `rules.NewAppendRule` for clarity.
  - Updated all occurrences of `rules.NewRule` in the codebase to the new
    name.

- Enhance `Address` and `Source` functions to handle empty parameters

  This commit improves the `Address` and `Source` functions to handle
  cases where empty parameters are passed. This change helps prevent the
  creation of invalid or empty parameters that could lead to runtime
  errors or unexpected behavior in the iptables rule generation process.

  Key Changes:
  - Modified `Address` function to return `nil` if the input address is an
    empty string.
  - Updated `Source` function to return `nil` if the input parameter is
    `nil`.
  - These changes ensure that the functions gracefully handle empty inputs
    and do not create invalid parameters.

- Refactor: Use `ProtocolL4` abstraction instead of hardcoded strings

  This commit replaces the hardcoded "tcp" and "udp" strings with the
  `ProtocolL4` abstraction. By using the `ProtocolL4` type, the code
  becomes more maintainable and reduces the risk of errors related to
  protocol string mismatches.

  - Introduced the `ProtocolL4` type to represent Layer 4 protocols (TCP
    and UDP).
  - Updated all instances of hardcoded "tcp" and "udp" strings to use the
    `ProtocolL4` constants (`ProtocolTCP` and `ProtocolUDP`).
  - Implemented the `ParseProtocolL4` function to convert strings to
    `ProtocolL4` types, ensuring consistent protocol handling throughout
    the codebase.

Signed-off-by: Bart Smykla <[email protected]>
  • Loading branch information
bartsmykla authored Jul 11, 2024
1 parent 4b01e64 commit faedd08
Show file tree
Hide file tree
Showing 12 changed files with 230 additions and 85 deletions.
56 changes: 45 additions & 11 deletions pkg/transparentproxy/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,41 @@ type Owner struct {
// ranges and multiple values can be mixed e.g. 1000,1005:1006 meaning 1000,1005,1006
type ValueOrRangeList string

// NewValueOrRangeList creates a ValueOrRangeList from a given value or range of
// values. It accepts a parameter of type []uint16, uint16, or string and
// converts it to a ValueOrRangeList, which is a comma-separated string
// representation of the values.
//
// Args:
// - v (T): The input value which can be a slice of uint16, a single uint16,
// or a string.
//
// Returns:
// - ValueOrRangeList: A comma-separated string representation of the input
// values.
//
// The function panics if an unsupported type is provided, although the type
// constraints should prevent this from occurring.
func NewValueOrRangeList[T ~[]uint16 | ~uint16 | ~string](v T) ValueOrRangeList {
switch value := any(v).(type) {
case []uint16:
var ports []string
for _, port := range value {
ports = append(ports, strconv.Itoa(int(port)))
}
return ValueOrRangeList(strings.Join(ports, ","))
case uint16:
return ValueOrRangeList(strconv.Itoa(int(value)))
case string:
return ValueOrRangeList(value)
default:
// Shouldn't be possible to catch this
panic(errors.Errorf("invalid value type: %T", value))
}
}

type Exclusion struct {
Protocol string
Protocol ProtocolL4
Address string
UIDs ValueOrRangeList
Ports ValueOrRangeList
Expand Down Expand Up @@ -771,19 +804,20 @@ func parseExcludePortsForUIDs(exclusionRules []string) ([]Exclusion, error) {
return nil, errors.Wrap(err, "invalid UID range")
}

var protocols []string
var protocols []ProtocolL4
if protocolOpts == "" || protocolOpts == "*" {
protocols = []string{"tcp", "udp"}
protocols = []ProtocolL4{ProtocolTCP, ProtocolUDP}
} else {
for _, p := range strings.Split(protocolOpts, ",") {
pCleaned := strings.ToLower(strings.TrimSpace(p))
if pCleaned != "tcp" && pCleaned != "udp" {
return nil, errors.Errorf(
"invalid or unsupported protocol: '%s'",
pCleaned,
)
for _, s := range strings.Split(protocolOpts, ",") {
if p := ParseProtocolL4(s); p != ProtocolUndefined {
protocols = append(protocols, p)
continue
}
protocols = append(protocols, pCleaned)

return nil, errors.Errorf(
"invalid or unsupported protocol: '%s'",
s,
)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type FunctionalityModules struct {
// more connection tracking information than the "state" match.
// ref. iptables-extensions(8) > conntrack
Conntrack bool
Multiport bool
}

type FunctionalityChains struct {
Expand Down Expand Up @@ -120,6 +121,7 @@ func verifyFunctionality(
Udp: verifyModule(ctx, iptables, ModuleUdp),
Comment: verifyModule(ctx, iptables, ModuleComment),
Conntrack: verifyModule(ctx, iptables, ModuleConntrack),
Multiport: verifyModule(ctx, iptables, ModuleMultiport),
},
}

Expand Down
13 changes: 13 additions & 0 deletions pkg/transparentproxy/config/config_suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package config_test

import (
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func Test(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Transparent Proxy Config Suite")
}
30 changes: 30 additions & 0 deletions pkg/transparentproxy/config/config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package config

import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

var _ = Describe("NewValueOrRangeList", func() {
DescribeTable("should create ValueOrRangeList",
func(input interface{}, expected string) {
// when
var result ValueOrRangeList

switch v := input.(type) {
case []uint16:
result = NewValueOrRangeList(v)
case uint16:
result = NewValueOrRangeList(v)
case string:
result = NewValueOrRangeList(v)
}

// then
Expect(string(result)).To(Equal(expected))
},
Entry("from uint16 slice", []uint16{80, 443}, "80,443"),
Entry("from single uint16", uint16(8080), "8080"),
Entry("from string", "1000-2000", "1000-2000"),
)
})
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func buildMangleTable(cfg config.InitializedConfigIPvX) *tables.MangleTable {
if cfg.DropInvalidPackets {
mangle.Prerouting().AddRules(
rules.
NewRule(
NewAppendRule(
Match(Conntrack(Ctstate(INVALID))),
Jump(Drop()),
).
Expand Down
Loading

0 comments on commit faedd08

Please sign in to comment.