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

Wip keep server host cert #712

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

dcasier
Copy link

@dcasier dcasier commented Nov 13, 2024

What do you think about being able to keep and consult the certificate of the host server?

Personally, I use it to make sure that the host corresponds to the right IP (with several hosts having the same certificate authority)

@ronf
Copy link
Owner

ronf commented Nov 14, 2024

Can you say a little more about your use case here? When a server presents a certificate, the existing SSH client code in AsyncSSH already verifies that one of the principals in the certificate (if there are any) matches the hostname or IP of the server you requested to connect to. So, it shouldn't be necessary for you to do that check yourself.

On the server side, a similar thing happens when accepting client certs over public key auth. In that case, the server verifies that the certificate presented has a principal which matches the user name that the client requested to authenticate as.

Also, on the server side AsyncSSH already stores key/certificate options that you can query to check if particular features are enabled for that specific key used to authenticate (via directives in authorized_keys) or the certificate used to authenticate. You can use functions like get_key_permission() and check_certificate_permission() on SSHServerConnection to check if certain features are enabled for a given user key/certificate.

@dcasier
Copy link
Author

dcasier commented Nov 14, 2024

I need to validate that there was no IP spoofing (in an environment where it is possible to have signed certificates, for a given fqdn).

I had thought of 2 things, otherwise:

  • Being able to overload conn_factory (that's what I'm doing, currently)
  • Add cert as a parameter of validate_host_ca_key (but that's not really the role of this function)

Edit : it's on an admin IP, not the one on the DNS record

@ronf
Copy link
Owner

ronf commented Nov 15, 2024

Sorry, I'm still not quite following what you're trying to do. How does your check differ from what AsyncSSH is already doing to validate the destination of the connect() call against the principals in the returned server certificate? Can you provide some example code of what the connect() call and your validation would look like, if you had a way to query the server cert?

@dcasier
Copy link
Author

dcasier commented Nov 15, 2024

In my case, a machine does not boot with all the network layer, it has 2 IPs:

  • IP X (no fqdn)
  • IP Y (fqdn and rsa-cert associated, but blocked on boot)
    Another machine connects via X and must verify that the machine has the certificate for the associated FQDN to open the flow to Y.
    When connecting via X, asyncssh, sees a certificate for the fqdn for Y, the verification failed.

@ronf
Copy link
Owner

ronf commented Nov 16, 2024

Thanks for the explanation - that helped. If the contain is going to IP X and that's not in the cert, you'll need more than just storing a copy of the certificate, as AsyncSSH's principal check will fail when the connection is opened. As you said, the only function which you can override here, though, is validate_host_ca_key, which only sees the CA key and not the certificate.

Perhaps you could use a host key alias here. If you know you are going to need to validate against hostname Y when you are connecting to IP X, you can add a host_key_alias argument on the connect call which specifies you are expect to see in the cert, and then all the usual validate checks done against the hostname will be against that name rather than IP X. That will affect both the host certificate validation and the lookup in known_hosts, so you'll need to make sure that there's a known_hosts entry for hostname Y (instead of IP X) when specifying things like what CA keys to trust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants