NewSocks5Proxy hostKey type is private want to use custom ssh HostKeyCallback

[xor-gate]

Hi all,

I was playing around with this nice package. But needed to dig deep to come to the conclusion the hostType private interface is a single function private interface where the public caller needs to implement

https://github.com/cloudfoundry/socks5-proxy/blob/main/socks5_proxy.go#L22

When I want to extend my CLI tool with SSHFP (ssh DNS host fingerprint) functionality it is not clear. When using this package it is not 100% clear what needs to be passed to the NewSocks5Proxy hostKey parameter. I hacked my CLI tool with a custom Get function and cache the fetched host key. Which is almost the same implementation as the public HostKey type.

UPDATE: It is simpler than I thought with the following code to use the standard mechanism. But this doesn't allow custom host key handling.

hostKey := proxy.NewHostKey()
sshSocks5Proxy := proxy.NewSocks5Proxy(&hostKey, nil, time.Minute)

The package I want to integrate is https://github.com/xor-gate/sshfp so the host key can be trusted using a SSHFP DNS record.

Hopefully this is clear enough, or else please ask.

Long story short: Why is the interface of hostKey type private?

[jpalermo]

I'm not super familiar with the code, but I think the interface is private because it's intended to only be used within the socks5 proxy package.

But the interface being private shouldn't prevent you from creating a new struct that implements that interface correct? Because in golang you don't declare the interface you are implementing, you just provide a struct that has functions where the signature matches.

I would think you could create a new package that has a struct that implements a Get function that matches the current hostkey interface and use that as the argument to NewSocks5Proxy and within that Get function you could fetch the host key from DNS like you want?

[xor-gate]

Thanks for your message. This is indeed how I did get it to work with my CLI tool, dig into the code and search how the hostKey private type is implemented. I understand why you could make parameter types private, but never seen this in open source APIs. I have created a initial draft which doesn't break the API for other tools. It renames hostKey type to SSHHostKeyFetcher as it is a better name for the interface IMHO. Naming it to HostKey public breaks compatibilty because there is already the default HostKey type when users don't know how to handle the lowlevel things with the ssh package.

Let me know what you think.

[jpalermo]

I don't think the interface needs to be public for you to create a new hostkey struct.

You should just be able to create a new struct in your package with whatever name you want, and then give it a function with the interface:

Get(username, privateKey, serverURL string) (ssh.PublicKey, error)

And then you can pass an instance of that struct as the first argument to NewSocks5Proxy

[xor-gate]

Yes it compiles fine when finding out how the interface type looks like. But I have never seen this type of Golaing API programming in the wild (open-source). When people like me want to use it, they are stuck with the default hostKey type or diving deep into the code to do the same and extend it with some custom implementation. What's wrong making a open-source library extendable and better documented?

[jpalermo]

I'm still having a hard time understanding the benefits of making the interface public. Let me explain what I understand the two different options to be, and their pros and cons, and maybe you can then show where I'm not understanding:

Here is the interface we're talking about:

type hostKey interface {
	Get(username, privateKey, serverURL string) (ssh.PublicKey, error)
}

If this interface is private:

1. If you want to call NewSocks5Proxy, you need to either generate a new hostkey using hostKey := proxy.NewHostKey(), or create your own struct that provides the Get method required of the interface.
2. The interface can only be used within the socks5 proxy package.

If the interface is public:

1. If you want to call NewSocks5Proxy, you need to either generate a new hostkey using hostKey := proxy.NewHostKey(), or create your own struct that provides the Get method required of the interface.
2. You could use the hostkey interface in another package.
3. Since it is now a public interface, if we wanted to add additional methods to it at some point, it would be a breaking change and we'd need to bump the major version of the library.

So bullet point Number 1 is the same in either case. If being public or private, your ability to call NewSocks5Proxy does not change.

Bullet point Number 2 in the public case is the only reason to make it public. To allow the usage of the interface in other packages. But the interface doesn't appear to have much value. Is somebody writing another package that wants to rely on this interface?

Bullet point Number 3 in the public case is the cost of Number 2. If you make it public, it's now part of the contract, and changing it (adding methods, removing methods, changing methods) now becomes costly.

I don't understand the value of Number 2, who would want to do this?

So I feel like I'm missing something here @xor-gate. Either there is more value from Number 2, being able to use the interface in a different package that I'm not understanding. Or there is a benefit I've completely missed in my list.

Could you help me understand what I'm missing?

[xor-gate]

Yes this is a good summary of both situations. I totaly understand in terms of the cloudfoundry usage it doesn't make any sense to make it public. But:

Bullet point Number 2 in the public case is the only reason to make it public. To allow the usage of the interface in other packages. But the interface doesn't appear to have much value. Is somebody writing another package that wants to rely on this interface?

I wrote a commandline tool around this nice module to see if it could replace the native openssh executable. If you understand the SSH protocol about host keys the current implementation is for cloud environment where the SSH client is blindly accepting the SSH server host key this is almost the same as ssh.InsecureIgnoreHostKey except it caches the key for further usage. I did write a Golang SSHFP implementation which side-channels checks hostkey fingerprints based on DNS(SEC). For enhanced security reasons it would even be better to expose the ssh.HostKeyCallback, but this modified the cloudfoundry/socks5-proxy to much. In CI/CD cloud environments its almost impossible to manage the ephemeral container environments SSH servers host key management. But for secure systems one should not blindly allow host keys even in non-interactive environments.

Hope this clears things out.

[jpalermo]

Sorry for the delay, didn't have a chance to look at the code until now.

I see in the tool that you wrote, you implemented a struct that implements the hostkey interface expected by the proxy package:

type SSHHostPublicKeyFetcher struct {
	ssh.PublicKey
}

func (f *SSHHostPublicKeyFetcher) Get(username, privateKey, serverURL string) (ssh.PublicKey, error) {
	return f.PublicKey, nil
}

This is how I would expect somebody to accomplish what you were trying to do. And implementing SSHFP for a different hostkey fetcher makes sense too.

But how would having the hostkey interface public make either code easier?