netconf basic callhome functionality

This a super basic callhome functionality based on rfc8071. Tested with real netconf device (in SSH mode), didn't test with TLS.

Opening as a DRAFT, if it is something you may be interested in maybe we can have some discussion on how to proceed further.

Feb 28 '23 22:02 GiacomoCortesi

Wow. I always tried to keep call home in mind when designing the transport mechanisms but I didn't put much thought into it, but this actually worked out to be really nice.

Thanks for the PR. I wait until this is out of draft to review but I am happy with the general direction here!

Mar 03 '23 19:03 nemith

This a super basic callhome functionality based on rfc8071. Tested with real netconf device (in SSH mode), didn't test with TLS.

Opening as a DRAFT, if it is something you may be interested in maybe we can have some discussion on how to proceed further.

Hey @nemith, how do we move forward with this PR? The current functionality looks good to me, unless you are thinking something different or you want something else to be added, in that regard I'll be happy to discuss any changes.

You want to track this in a proposal as I've done with notifications?

Jun 13 '23 13:06 GiacomoCortesi

I think the functionality is fine however I am marching to a 1.0 release with a stable API and don’t want to be changing the api that much later on. So I want to spend some real effort in making sure the api is right.

Is this something you need for your use case? If so I can prioritize it. If it’s not maybe it can wait as I am actively testing this package inside Roblox’s network and fixing things as we go along. We aren’t using or have any plans for call home.

Jun 13 '23 13:06 nemith

I think the functionality is fine however I am marching to a 1.0 release with a stable API and don’t want to be changing the api that much later on. So I want to spend some real effort in making sure the api is right.

Is this something you need for your use case? If so I can prioritize it. If it’s not maybe it can wait as I am actively testing this package inside Roblox’s network and fixing things as we go along. We aren’t using or have any plans for call home.

Unfortunately Callhome is a mandatory feature for the project I'm working on. I can double check the implementation and see if we can apply the same handler approach we used for notifications also for handling callhome clients connections.

If it helps you following the flow I can put down the proposal with some code snippets.

Jun 13 '23 14:06 GiacomoCortesi

Ok let’s prioritize it. I can propose some api changes and possibly do a call home branch if needed.

Sent from Proton Mail for iOS

On Tue, Jun 13, 2023 at 11:36, Giacomo Cortesi @.***(mailto:On Tue, Jun 13, 2023 at 11:36, Giacomo Cortesi < wrote:

I think the functionality is fine however I am marching to a 1.0 release with a stable API and don’t want to be changing the api that much later on. So I want to spend some real effort in making sure the api is right.

Is this something you need for your use case? If so I can prioritize it. If it’s not maybe it can wait as I am actively testing this package inside Roblox’s network and fixing things as we go along. We aren’t using or have any plans for call home.

Unfortunately Callhome is a mandatory feature for the project I'm working on. I can double check the implementation and see if we can apply the same handler approach we used for notifications also for handling callhome clients connections.

If it helps you following the flow I can put down the proposal with some code snippets.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

Jun 13 '23 15:06 nemith

So re-reading the RFC an thinking this through a bit.

We need a CallHome server that listens on a TCP port. The port should be configurable but there should maybe be some constants for 4334, 4335, and 4336. We should also probably be able to be passed in a net.Listener as well.
Once we get a connection there is a lot of things that need to be configured / inspected before we create a transport and then a netconf session for it.

5-tuple connection information (local/remote address port, protocol). This should be in the conn that a net.Listener gives us so no need for a new struct
For TLS: we need then initiate a connection and validate certificates and potentially provide our own certificate conditionally based on the information above or based on the server certificate.
For SSH we need to initiate the connection with auth ssh key, username, password, etc and then validate the host key in some way
For Restconf The RFC is for restconf and maybe not a huge goal but i think supporting restconf could be possible but more setting for http headers need to be coniditionally set
*All this needs to be programmable (i.e a callback/handler of some sort).

We then need to wrap these around a netconf.Session (or for restconf i guess just a http.Client?, perhaps we sholdn't worry about Restconf yet... 😮‍💨 ). Sessions need to have some config as well (for notification handlers for example, capability exchange, etc)
These sessions then may need to be looked up and utilized from the call home server object (based on some/multiple identifiers?) so that non-notification use cases can find a connection and issue rpc messages down it and receive results. This could be done externally to the home server object as the idenitifier could be specialized.

I need to think thought this all bit more and see how the API should roughtly be. It probably won't be much different from what you have but there is a lot to concider to properly support this.

The other option is for really simple use cases there isn't much to a callhome server and it could easily be implemented outside of the netconf package. Perhaps really is what is needed is functions for TLS and SSH that take a conn and give back a transport? This may be the simpliest fix for now?

Thoughts?

Jun 16 '23 14:06 nemith

So re-reading the RFC an thinking this through a bit.

We need a CallHome server that listens on a TCP port. The port should be configurable but there should maybe be some constants for 4334, 4335, and 4336. We should also probably be able to be passed in a net.Listener as well.

Agreed, address (and port) is already configurable through the WithAddress CallHomeOption. If no address is specified it defaults to: 0.0.0.0:4334 It make sense to add a WithListener CallHomeOption that allows to pass in the CallHomeServer a net listener.

Once we get a connection there is a lot of things that need to be configured / inspected before we create a transport and then a netconf session for it.

5-tuple connection information (local/remote address port, protocol). This should be in the conn that a net.Listener gives us so no need for a new struct

For TLS: we need then initiate a connection and validate certificates and potentially provide our own certificate conditionally based on the information above or based on the server certificate.

This shall be already possible by passing a proper TLSCallHomeTransport configuration

For SSH we need to initiate the connection with auth ssh key, username, password, etc and then validate the host key in some way

This is already possible by passing a proper SSHCallHomeTransport, e.g. :

	chcList := []*netconf.CallHomeClientConfig{
		{
			Transport: &netconf.SSHCallHomeTransport{
				Config: &ssh.ClientConfig{
					User: "foo",
					Auth: []ssh.AuthMethod{
						ssh.Password("bar"),
					},
					// as specified in rfc8071 3.1 C5 netconf client must validate host keys
					HostKeyCallback: ssh.InsecureIgnoreHostKey(),
				},
			},
			Address: "192.168.121.17",
		},
	}

For Restconf The RFC is for restconf and maybe not a huge goal but i think supporting restconf could be possible but more setting for http headers need to be coniditionally set

I need to think about RESTCONF a little bit, never took it into account so far.

*All this needs to be programmable (i.e a callback/handler of some sort).

I can try to rethink the current implementation that uses the two channels: clientsChannel chan *CallHomeClient errorChannel chan *ClientError replacing them with handlers. I agree it would be a better implementation decision.

We then need to wrap these around a netconf.Session (or for restconf i guess just a http.Client?, perhaps we sholdn't worry about Restconf yet... 😮‍💨 ). Sessions need to have some config as well (for notification handlers for example, capability exchange, etc)

Definitely, current implementation does not allow for customising session creation with options, that needs to be changed for sure (maybe adding them to CallHomeClientConfig struct? or just by allowing to setup a notification handler after session creation?)

These sessions then may need to be looked up and utilized from the call home server object (based on some/multiple identifiers?) so that non-notification use cases can find a connection and issue rpc messages down it and receive results. This could be done externally to the home server object as the idenitifier could be specialized.

Client identification is subtle, it's currently based on IP address only and I agree it is limited. I'm not sure how can we handle it from the outside since we still need to provide a configuration for incoming connections. Since when a callhome client connect we only know it's IP address and port, not sure what kind of other identifier we could use to provide callhome client configuration.

I need to think thought this all bit more and see how the API should roughtly be. It probably won't be much different from what you have but there is a lot to concider to properly support this.

Thank u for taking the time to look deeper in something you don't really need for yourself 🥇

The other option is for really simple use cases there isn't much to a callhome server and it could easily be implemented outside of the netconf package. Perhaps really is what is needed is functions for TLS and SSH that take a conn and give back a transport? This may be the simpliest fix for now?

mmm even though to create the transport we still need the initial client configuration, meaning the callhome server need to know whether to build a SSH or TLS transport on top of the TCP connection so it doesn't solve the identifier issue if I well understand.

Jun 20 '23 15:06 GiacomoCortesi