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

cmds/core/netcat: add support for udp #2737

Merged
merged 4 commits into from
Aug 21, 2023
Merged

Conversation

binjip978
Copy link
Contributor

No description provided.

Signed-off-by: Siarhiej Siemianczuk <pdp.eleven11@gmail.com>
@codecov
Copy link

codecov bot commented Aug 12, 2023

Codecov Report

Patch coverage: 66.66% and project coverage change: -0.15% ⚠️

Comparison is base (ea44028) 75.54% compared to head (cf2ef29) 75.40%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2737      +/-   ##
==========================================
- Coverage   75.54%   75.40%   -0.15%     
==========================================
  Files         416      416              
  Lines       42373    42401      +28     
==========================================
- Hits        32012    31972      -40     
- Misses      10361    10429      +68     
Files Changed Coverage Δ
cmds/core/netcat/netcat.go 76.82% <66.66%> (+12.01%) ⬆️

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@binjip978 binjip978 added the Awaiting reviewer Waiting for a reviewer. label Aug 12, 2023
Copy link
Member

@rminnich rminnich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to leave the comment about 1500 if you don't see a need for it.

one bigger question: the golang net package tried to follow the plan 9 convention that code could set up a socket and just use Read()/Write() once it was set up, no metter the type. IOW you did not need a WriteToUDP, for example, because, once the socket is set up, you should be able to use Read and Write. Are you pretty certain you need to break out UDP functions by name, once you have the socket? I do not know but it's a shame if we need to add so much code for every different transport.

Thanks.

cmds/core/netcat/netcat.go Outdated Show resolved Hide resolved
@rminnich
Copy link
Member

Hi, can you look at https://github.com/rminnich/u-root/pull/new/netcat-udp

It does the same thing, but only needs the run() function

Signed-off-by: Siarhiej Siemianczuk <pdp.eleven11@gmail.com>
@binjip978
Copy link
Contributor Author

Hi, can you look at https://github.com/rminnich/u-root/pull/new/netcat-udp

It does the same thing, but only needs the run() function

I updated udp listen test with server -> client message and net.Conn approach should not work now.

@rminnich
Copy link
Member

I am convinced we should be able to make this work without runDatagram() but perhaps you can show my why I am wrong()

but, bear in mind, all these functions
func ListenIP(network string, laddr *IPAddr) (*IPConn, error)
func ListenMulticastUDP(network string, ifi *Interface, gaddr *UDPAddr) (*UDPConn, error)
func ListenPacket(network, address string) (PacketConn, error)
func ListenTCP(network string, laddr *TCPAddr) (*TCPListener, error)
func ListenUDP(network string, laddr *UDPAddr) (*UDPConn, error)
func ListenUnix(network string, laddr *UnixAddr) (*UnixListener, error)
func ListenUnixgram(network string, laddr *UnixAddr) (*UnixConn, error)
return a type that implements net.Conn interface.

UDPConn even says so:
UDPConn is the implementation of the Conn and PacketConn interfaces for UDP
network connections.

so this really ought to be usable without adding another function.

@binjip978
Copy link
Contributor Author

binjip978 commented Aug 17, 2023

I am convinced we should be able to make this work without runDatagram() but perhaps you can show my why I am wrong()

but, bear in mind, all these functions func ListenIP(network string, laddr *IPAddr) (*IPConn, error) func ListenMulticastUDP(network string, ifi *Interface, gaddr *UDPAddr) (*UDPConn, error) func ListenPacket(network, address string) (PacketConn, error) func ListenTCP(network string, laddr *TCPAddr) (*TCPListener, error) func ListenUDP(network string, laddr *UDPAddr) (*UDPConn, error) func ListenUnix(network string, laddr *UnixAddr) (*UnixListener, error) func ListenUnixgram(network string, laddr *UnixAddr) (*UnixConn, error) return a type that implements net.Conn interface.

UDPConn even says so: UDPConn is the implementation of the Conn and PacketConn interfaces for UDP network connections.

so this really ought to be usable without adding another function.

We somehow need to obtain remote address, so UDPConn implements Conn, but it working only one way (client -> server, client knows server address).

In this simple program I got 0 write udp [::]:1234: write: destination address required

package main

import (
	"fmt"
	"log"
	"net"
)

func main() {
	var conn net.Conn
	conn, err := net.ListenUDP("udp", &net.UDPAddr{Port: 1234})
	if err != nil {
		panic(err)
	}

	b := make([]byte, 1024)
	for {
		fmt.Println()
		n, err := conn.Read(b)
		if err != nil {
			log.Println(err)
		}

		fmt.Printf("Received: %s\n", b[:n])

		n, err = conn.Write([]byte("Hello from server"))
		if err != nil {
			log.Println(n, err)
		}
	}
}

But in net.Conn interface we can get remote address, but we can't specify it in conn.Write and upd message will not reach client.

A bit more about error:

EDESTADDRREQ
The socket is not connection-mode, and no peer address is
set.

@rminnich
Copy link
Member

you can set a remote address on a UDP connection. It then becomes a "connected" socket. See this writeup
https://blog.cloudflare.com/everything-you-ever-wanted-to-know-about-udp-sockets-but-were-afraid-to-ask-part-1/

so I think that's what we need here, and why you are getting this error
The socket is not connection-mode, and no peer address is

@binjip978
Copy link
Contributor Author

you can set a remote address on a UDP connection. It then becomes a "connected" socket. See this writeup https://blog.cloudflare.com/everything-you-ever-wanted-to-know-about-udp-sockets-but-were-afraid-to-ask-part-1/

so I think that's what we need here, and why you are getting this error The socket is not connection-mode, and no peer address is

Yeah, this is interesting "Established-over-unconnected technique", but a bit hacky and probably will require build constraints, at least author claims it works on Linux. But I'm not sure if it will simplify things, will require syscall package.

@rminnich
Copy link
Member

it's worked on just about everything for many decades. I learned about it long after it was a standard. It surprised me too. "connected UDP socket? What?" but it's real.

@rminnich
Copy link
Member

courtesy chatgpt, no idea if it is right

struct sockaddr_in target_address;
memset(&target_address, 0, sizeof(target_address));
target_address.sin_family = AF_INET;
target_address.sin_port = htons(12345); // Replace with the target port number
target_address.sin_addr.s_addr = inet_addr("127.0.0.1"); // Replace with the target IP address

// Connect the UDP socket to the target address
if (connect(udp_socket, (struct sockaddr *)&target_address, sizeof(target_address)) == -1) {
    perror("connect");
    close(udp_socket);
    exit(EXIT_FAILURE);
}

@rminnich
Copy link
Member

func DialUDP(network string, laddr, raddr *UDPAddr) (*UDPConn, error)
DialUDP acts like Dial for UDP networks.

The network must be a UDP network name; see func Dial for details.

If laddr is nil, a local address is automatically chosen. If the IP field of
raddr is nil or an unspecified IP address, the local system is assumed.

I suspect this will try a connect. I only got so far following the code down the rabbit hole however.

@binjip978
Copy link
Contributor Author

Maybe own type with some embeddings is a good idea to do it in one function. I will try it on weekend.

@rminnich
Copy link
Member

ok but before you do the own type, I recommend you try DialUDP, b/c I think that's what we want

@binjip978
Copy link
Contributor Author

ok but before you do the own type, I recommend you try DialUDP, b/c I think that's what we want

But DialUDP where? I already using it in runDatagram function?

Signed-off-by: Siarhiej Siemianczuk <pdp.eleven11@gmail.com>
@binjip978 binjip978 merged commit 5523fcb into u-root:main Aug 21, 2023
23 of 24 checks passed
@binjip978 binjip978 deleted the netcat-udp branch August 21, 2023 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting reviewer Waiting for a reviewer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants