Skip to content

Commit

Permalink
fix(microsoft#139): do not check for host endianness in IP conversion (
Browse files Browse the repository at this point in the history
…microsoft#443)

Function `determineEndian()` always returns the same value since the
host byte order is always little-endian, so it can be skipped.
What's more, the byte order of IP address values passed by eBPF programs
doesn't need to be changed, because Golang's `net.IP` type expects
big-endian byte arrays.

**Details:**

We always enter this if-branch `binary.LittleEndian.PutUint32(ip, nn)`
which looks like it performs conversion TO little-endian, while it
actually expects to receive input already IN little-endian.
Note, the input number's bytes are kept in the same order - this
function merely converts a uint32 to a byte array. The result is we
convert a big-endian number to a big-endian byte array.

```
func (littleEndian) PutUint32(b []byte, v uint32) {
	_ = b[3] // early bounds check to guarantee safety of writes below
	b[0] = byte(v)
	b[1] = byte(v >> 8)
	b[2] = byte(v >> 16)
	b[3] = byte(v >> 24)
}
```
Crucially, `net.IP` expects big-endian byte arrays. For example, IP
`15.14.0.0` (`0x0F0E0000` in host order, and `0x00000E0F` in network
order) should be passed in as `[0F, 0E, 00, 00]` (most significant byte
at the lowest memory address) and so doesn't need inverting. Example
here: https://play.golang.com/p/ykiP6mLB_gG

In summary, we currently do not perform byte order conversion and
shouldn't start to, either in GO or eBPF code.

Fixes microsoft#139

This PR looks almost identical to
microsoft#161 which was rejected because
it didn't include byte order conversion in eBPF, but turns out that's
not needed. It also uses the correct uint-to-array conversion functions.

- [x] I have read the [contributing
documentation](https://retina.sh/docs/contributing).
- [x] I signed and signed-off the commits (`git commit -S -s ...`). See
[this
documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
on signing commits.
- [x] I have correctly attributed the author(s) of the code.
- [x] I have tested the changes locally.
- [x] I have followed the project's style guidelines.
- [x] I have updated the documentation, if necessary.
- [x] I have added tests, if applicable.

Signed-off-by: Igor Klemenski <[email protected]>
Signed-off-by: Carpe-Wang <[email protected]>
  • Loading branch information
rectified95 authored and Carpe-Wang committed Jun 11, 2024
1 parent f6751c5 commit 6ead12f
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 31 deletions.
1 change: 1 addition & 0 deletions init/retina/main_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func main() {
if err != nil {
l.Fatal("Failed to get config", zap.Error(err))
}

// Enable telemetry if applicationInsightsID is provided
if applicationInsightsID != "" && cfg.EnableTelemetry {
opts.EnableTelemetry = true
Expand Down
33 changes: 2 additions & 31 deletions pkg/utils/utils_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,28 +49,15 @@ func htons(i uint16) uint16 {
// https://gist.github.com/ammario/649d4c0da650162efd404af23e25b86b
func Int2ip(nn uint32) net.IP {
ip := make(net.IP, 4)
switch determineEndian() {
case binary.BigEndian:
binary.BigEndian.PutUint32(ip, nn)
default:
// default is little endian
binary.LittleEndian.PutUint32(ip, nn)
}
binary.LittleEndian.PutUint32(ip, nn)
return ip
}

func Ip2int(ip []byte) (res uint32, err error) {
if len(ip) == 16 {
return res, errors.New("IPv6 not supported")
}
switch determineEndian() {
case binary.BigEndian:
res = binary.BigEndian.Uint32(ip)
default:
// default is little endian.
res = binary.LittleEndian.Uint32(ip)
}
return res, nil
return binary.LittleEndian.Uint32(ip), nil
}

// HostToNetShort converts a 16-bit integer from host to network byte order, aka "htons"
Expand All @@ -80,22 +67,6 @@ func HostToNetShort(i uint16) uint16 {
return binary.BigEndian.Uint16(b)
}

func determineEndian() binary.ByteOrder {
var endian binary.ByteOrder
buf := [2]byte{}
*(*uint16)(unsafe.Pointer(&buf[0])) = uint16(0xABCD)

switch buf {
case [2]byte{0xCD, 0xAB}:
endian = binary.LittleEndian
case [2]byte{0xAB, 0xCD}:
endian = binary.BigEndian
default:
fmt.Println("Couldn't determine endianness")
}
return endian
}

// GetDefaultOutgoingLinks gets the outgoing interface by executing an equivalent to `ip route show default 0.0.0.0/0`
func GetDefaultOutgoingLinks() ([]netlink.Link, error) {
routes, err := netlink.RouteList(nil, netlink.FAMILY_ALL)
Expand Down

0 comments on commit 6ead12f

Please sign in to comment.