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

Feat: Add SSHFP support for egress connections #228

Closed
wants to merge 2 commits into from

Conversation

coimbrap
Copy link

@coimbrap coimbrap commented Aug 16, 2021

Allow SSHFP fields to be used for connections between The Bastion and backends.

For SSH we just replace $ip by $hostto and for SCP we use ip2host with $ip to obtain hostname.

When the host cannot be resolved or no SSHFP fields available (dns unreachable or no SSHFP / PTR entry):, classical TOFU is used

ip2host call :

my $hostto = OVH::Bastion::ip2host($ip)->value || $ip;

Permit SSHFP usage by The Bastion SSH client for ssh command

Signed-off-by: Pierre Coimbra <[email protected]>
Permit SSHFP usage by The Bastion SSH client for scp command

Signed-off-by: Pierre Coimbra <[email protected]>
@coimbrap coimbrap changed the title Feat: Add SSHFP for egress connections Feat: Add SSHFP support for egress connections Aug 16, 2021
@Alkorin Alkorin added the tests:short Launch tests (deb12 only, w/o cc) label Aug 16, 2021
@speed47
Copy link
Collaborator

speed47 commented Sep 16, 2021

Hello,

Thanks for your PR. The SSHFP support for egress is not trivial to implement, as all the bastion really cares about is IPs. This is how ACLs are stored, and when hostnames are used, they're immediately resolved by the code and the resolved IP is used.

In your version of the patch, I see two potential issues:

    1. If a hostname has several associated IPs (several "A" DNS records), the resolved IP, which is used to check the users authorizations, can be different than the IP tthat SSH will use in the end to connect, as it'll resolve the domain again and possibly use one of the other IPs. This introduces a potential vulnerability where somebody who has control other the domain's zone can trick the bastion in allowing a connection that would otherwise be denied.
    1. Resolving the hostname by querying the PTR of an IP can give you a different result than the original hostname, hence checking the SSHFP against the wrong domain.

There is still probably a way to do it, by storing the user-specified hostname and passing it to ssh instead of the IP (as you did for SSH but not for SCP), but I wouldn't do it by default, as the first potential issue can still be present. So I would add a global policy bastion option to either allow, deny or implicity allow this behaviour (and deny by default), plus a command-line parameter for the users, to explicitly request this behaviour if allowed by policy.

@speed47
Copy link
Collaborator

speed47 commented Dec 21, 2021

Closing this PR, idea pushed to backlog as #271

@speed47 speed47 closed this Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests:short Launch tests (deb12 only, w/o cc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants