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

Enable SFTP chroot support #308

Merged
merged 12 commits into from May 11, 2018
Merged

Conversation

manojampalam
Copy link

@manojampalam manojampalam commented May 4, 2018

  • Added chroot implementation that simply stores the path in internal state and sets an environment variable
  • Spawned processes pickup chroot from environment variable
  • Core change in realpath and resolved_path_utf16 now take into account chroot path.
  • Unit tests
  • Other miscellaneous changes to account for chroot enabled logic in core code

PowerShell/Win32-OpenSSH#190
PowerShell/Win32-OpenSSH#292

@manojampalam
Copy link
Author

@NoMoreFood FYI

@bingbing8 bingbing8 closed this May 7, 2018
@bingbing8 bingbing8 reopened this May 7, 2018
if ((wcslen(final_path) < wcslen(chroot_pathw)) ||
memcmp(final_path, chroot_pathw, 2 * wcslen(chroot_pathw)) != 0 ||
final_path[wcslen(chroot_pathw)] != '\\') {
debug4("access denied due to attempt to escape chroot jail");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Having it as debug() will help ssh admins to run some metrics and figureout the potential attacks..

@NoMoreFood
Copy link

I'm prepping for two weeks of paternity leave so I'm a bit preoccupied at my day job this week; I'll try to take take a look soon though. The biggest thing I'm worried about is how you addressed attempted new file creation outside of the jail by escaping using a relative path (e.g. ../../) when the target file does not exist. For example:

scp source.file user@host:../../target.file

Even in my proof of concept, I didn't handle this ideally (i.e., it allowed the file to be created, but didn't return it's handle). In this scenario, the best I could think of doing was to split the path (i.e. trim off the file name), get the directory handle to obtain the normalized path, and then use that as a comparison.

Regardless, do you think you've addressed this sort of thing in your code?

@manojampalam
Copy link
Author

manojampalam commented May 9, 2018

@NoMoreFood a very good point. I was only testing with sftp so far which would first resolve the path via realpath that would reject stepping up and out of root. So yes, with scp, I would end up creating the file and subsequently denying access to it (just like in your case)

I can fix this gap, by doing realpath resolution in resolved_path_utf16 which will block this. Will push out that change and add a test case tomorrow.

Manoj Ampalam added 2 commits May 9, 2018 22:36
@bagajjal bagajjal reopened this May 10, 2018
Manoj Ampalam added 2 commits May 10, 2018 14:34
@manojampalam
Copy link
Author

Alright, resolved_path_utf16 now goes through realpath where the core chroot resolution logic takes place. realpath rejects any resolutions outside of chroot jail.

Tested with scp too.

@manojampalam
Copy link
Author

Merging these to unblock session refactoring changes. Do review and I can address feedback in a later PR.

@manojampalam manojampalam merged commit 7b28a31 into PowerShell:latestw_all May 11, 2018
@manojampalam manojampalam deleted the chroot branch May 11, 2018 21:45
@NoMoreFood
Copy link

NoMoreFood commented May 23, 2018

I haven't look at the code for any potential holes, but my testing looks good so far.

Symbolic links still allow you to escape the jail, but it may be reasonable to just to consider this a feature limitation (and document it accordingly). Normal users can't create symbolic links unless security policy is altered from the defaults so it's not a major security concern in my opinion.

@swills1
Copy link

swills1 commented May 24, 2018

So how do we actually use this? I mean what are the instructions. I have a sftp user that logs in and sees everything. I want them to be locked into one directory. What do I literally need to do to make that happen?

@bagajjal
Copy link
Collaborator

@swills1 - The code changes only restrict sftp session, scp session but not ssh sessions. You can modify sshd_config and use MatchUser block to restrict sftp session, scp session.

@swills1
Copy link

swills1 commented May 24, 2018

@bagajjal I don't see MatchUser block example?

@bagajjal
Copy link
Collaborator

@swills1
Copy link

swills1 commented May 24, 2018

@bagajjal doesn't seem to work.

Match User test
    ChrootDirectory C/test
    # Disable tunneling, authentication agent, TCP and X11 forwarding.
    PermitTunnel no
    AllowAgentForwarding no
    AllowTcpForwarding no
    X11Forwarding no

@bagajjal
Copy link
Collaborator

Try this,

Match User test
ForceCommand internal-sftp
ChrootDirectory C:\test

For test user, you are restricting to use SFTP.

@swills1
Copy link

swills1 commented May 24, 2018

Ah, I am just trying to keep them locked to one folder. Right now they see everything, and I can't have that. They already have to use sFTP or they can't login.

@tianxiaaiwojs
Copy link

for other people also here: please refer to https://github.com/PowerShell/Win32-OpenSSH/wiki/sshd_config

all user name should be lower case. this spend me very long time to check.

@whereisaaron
Copy link

Hi @tianxiaaiwojs, your link URL is different from the text displayed, you meant to link to:
https://github.com/PowerShell/Win32-OpenSSH/wiki/sshd_config

@Algvaldivia
Copy link

Hi everyone,
There is a "real" explained and not confusing way to grant access to only a specific folder for each specific user.
Can anyone show an example of a real a detailed configuration to do this, or it is impossible.

  • Not One folder vs Many users. One folder, one user.

My configuration (fails at all) but I want to share it, expecting for someone who can show what's wrong? What we need to know to get that kind of restriction, that everybody wants but the most of us can't..

  • For the sample I have 3 users and a group, named SFTP_Users, all the three users belong to this group.
  • The main SFTP root folder is at C:\SFTP
  • Each user have a child-folder inside : C:\SFTP<username>
  • Each user only has access to their folder
  • But I do not want to show the content of the root folder, I want to show and provide access for each users to their folder (inside only)

The Open SSH Server main binaries files reside at : C:\Program Files\OpenSSH-Win64
The configuration file resides at : C:\ProgramData\ssh
The configuration settings that I added at the end of sshd_config is:

Forcing SFTP functionality, needed for ChrootDirectory parameter below

ChrootDirectory C:\SFTP\

override default of no subsystems

Subsystem sftp internal-sftp

allow to connect to OpenSSH only for users in this domain group

AllowGroups sftp_users

enable password authentication (SSH keys cannot be used)

AuthenticationMethods password

Match user ftpuser1
ChrootDirectory C:\SFTP\ftpuser1

Match user ftpuser2
ChrootDirectory C:\SFTP\ftpuser2

Match user ftpuser3
ChrootDirectory C:\SFTP\ftpuser3

Match user ftpuser4
ChrootDirectory C:\SFTP\ftpuser4

ForceCommand internal-sftp
PermitTunnel no
AllowAgentForwarding no
AllowTcpForwarding no
X11Forwarding no
GatewayPorts no
PermitTTY no
==================== END OF THE FILE =========================

My configuration do not return any good behavior or results, I'm tired to seek ideas across all of the threads and to make changes without any evolution or results.

If someone who really have a similar situation and his configuration is working as expected and consider sharing it with us,
Please, are welcome.

Thanks.

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.

None yet

8 participants