Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a Linux testing tool to test TCP DNS override functionality. The tool creates a new Linux utility that can inject into network namespaces and test DNS-over-HTTPS (DoH) tunneling behavior.
- Implements a TUN interface management tool for testing DNS override
- Adds bridge implementation to integrate with the firestack intra package
- Creates socket pair communication for namespace injection testing
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| linux-tool/toy/main.go | Main testing tool that handles TUN device creation, namespace injection, and DNS tunnel setup |
| linux-tool/glue/glue.go | Bridge implementation providing required interfaces for firestack integration and TUN device utilities |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| func sendTUN(name string) { | ||
| sock := int(3) |
There was a problem hiding this comment.
The magic number '3' should be documented or replaced with a named constant to explain why file descriptor 3 is being used.
| func sendTUN(name string) { | |
| sock := int(3) | |
| // SOCKET_FD is the file descriptor number expected to be a socket passed by the parent process. | |
| const SOCKET_FD = 3 | |
| func sendTUN(name string) { | |
| sock := SOCKET_FD |
| return fd, err | ||
| } | ||
| var req [64]byte | ||
| copy(req[:15], []byte(name)) |
There was a problem hiding this comment.
The copy operation limits the interface name to 15 bytes, but Linux interface names can be up to 15 characters (IFNAMSIZ-1). If the name is exactly 15 characters, this could cause issues since there's no null termination. Consider using copy(req[:16], []byte(name+"\0")) or validate the name length.
| copy(req[:15], []byte(name)) | |
| if len(name) > 15 { | |
| return -1, syscall.EINVAL | |
| } | |
| copy(req[:16], []byte(name+"\x00")) |
| } | ||
| } | ||
| func OpenTUN(name string) (int, error) { | ||
| fd, err := syscall.Open("/dev/net/tun", syscall.O_RDWR, 600) |
There was a problem hiding this comment.
The file mode '600' should use a constant like 0600 for clarity, or better yet, use os.FileMode constants to make the octal permissions explicit.
| fd, err := syscall.Open("/dev/net/tun", syscall.O_RDWR, 600) | |
| fd, err := syscall.Open("/dev/net/tun", syscall.O_RDWR, 0600) |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| func (bridge *MyBridge) OnProxiesStopped() {} | ||
|
|
||
| func Ioctl(fd int, req uint64, data []byte) (int, error) { | ||
| p := unsafe.Pointer(&data[0]) |
Check warning
Code scanning / gosec
Use of unsafe calls should be audited Warning
| func GenFDCmsg(fds []uint32) []byte { | ||
| cmsgLen := 16 + len(fds)*4 | ||
| cmsg := make([]byte, cmsgLen) | ||
| binary.NativeEndian.PutUint64(cmsg[:], uint64(cmsgLen)) |
Check failure
Code scanning / gosec
integer overflow conversion uint64 -> uint32 Error
| func sendTUN(name string) { | ||
| sock := int(3) | ||
| defer syscall.Close(sock) | ||
| syscall.SetNonblock(sock, true) |
Check warning
Code scanning / gosec
Errors unhandled Warning
| ifInfo, err := net.InterfaceByName(name) | ||
| p(err) | ||
| var msg [4]byte | ||
| binary.NativeEndian.PutUint32(msg[:], uint32(ifInfo.MTU)) |
Check failure
Code scanning / gosec
integer overflow conversion uint64 -> uint32 Error
| p(err) | ||
| var msg [4]byte | ||
| binary.NativeEndian.PutUint32(msg[:], uint32(ifInfo.MTU)) | ||
| cmsg := glue.GenFDCmsg([]uint32{uint32(tun)}) |
Check failure
Code scanning / gosec
integer overflow conversion uint64 -> uint32 Error
| pair, err := syscall.Socketpair(syscall.AF_UNIX, syscall.SOCK_SEQPACKET, 0) | ||
| p(err) | ||
| defer func() { | ||
| syscall.Close(pair[0]) |
Check warning
Code scanning / gosec
Errors unhandled Warning
| p(err) | ||
| defer func() { | ||
| syscall.Close(pair[0]) | ||
| syscall.Close(pair[1]) |
Check warning
Code scanning / gosec
Errors unhandled Warning
| syscall.Close(pair[0]) | ||
| syscall.Close(pair[1]) | ||
| }() | ||
| syscall.SetNonblock(pair[0], true) |
Check warning
Code scanning / gosec
Errors unhandled Warning
| c1 := exec.Command("/usr/bin/nsenter", "--target", fmt.Sprintf("%d", targetPid), "--user", "--net", | ||
| "--preserve-credentials", "--keep-caps", | ||
| elfPath, "-mode", "sendfd", "-tun", tunName) |
Check failure
Code scanning / gosec
Subprocess launched with a potential tainted input or cmd arguments Error
2 participants
#77 (comment)