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

Added support for importing or migrating repos via SSH #12397

Closed
wants to merge 0 commits into from

Conversation

TheoLassonder
Copy link

This request addresses issue #1635.

It adds support for importing or migrating repos via SSH by allowing ssh:// URLs to be specified in the "New Migration" screen and the "Mirror Settings" section of the repo "Settings" screen.

It supports ssh://user@server/absolute/path/to/repo.git URLs and also ssh://user@server/~/relative/path/to/repo.git URLs, but not ssh://user@server:relative/path/to/repo.git ones. This limitation is because the latter URL is rejected by url.Parse.

Allowing migration of ssh:// URLs is useful if you need to migration or mirror repos that are only available via SSH. There exist workarounds to access such repos, but they are cumbersome and can be a security risk.

@silverwind
Copy link
Member

silverwind commented Aug 4, 2020

This is a useful feature but I think one should be able to enter their SSH private key into the UI for the "Clone Authorization". Obviously, gitea should encode that key with a secret when storing it long-term, like for mirroring. If no key is specified, it should still be attempted using the keys on the application user.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 4, 2020
@silverwind
Copy link
Member

silverwind commented Aug 4, 2020

Thought I guess even without ssh key entry, this may be better than nothing.

One suggestion would be to support user@host:user/repo.git format. While this is not a well-formed URL, it's what GitHub and Gitea output as clone URL. Should be easy enough to detect and add a ssh:// prefix and replace : with / before actually cloning.

@TheoLassonder
Copy link
Author

TheoLassonder commented Aug 5, 2020

@silverwind Allowing users to enter their own SSH private keys is more difficult to implement (see also #12065). The feature proposed here is much simpler, and useful in itself.

Your suggestion of also allowing a user to enter the user@host:user/repo.git is a good one. Surprisingly (to me at least), It needs to be translated to ssh://user@host/~/user/repo.git, notssh://user@host/user/repo.git because the latter is interpreted by Git as a reference to the absolute path /user/repo.git.

However, it would arguably be annoying if Gitea changed the URL that the user typed in (even if it's strictly speaking not a valid one). But if Gitea didn't change it, then that raises a conundrum, because everywhere in Gitea the repo path is parsed with url.Parse, that is, it's assumed to be a valid URL (except when creating a migration/mirror, where the repo path can also be a local directory name, even if a local directory name is rejected if you try to update in the Settings page). But Git accepts paths that are not URLs, so Gitea is being unnecessarily strict. I'd say it's reasonable that Gitea accepts paths that Git also accepts.

So I'd propose to not do the translation in this pull request, but instead look towards making the interpretation of repo paths consistent in Gitea as a separate exercise.

@zeripath
Copy link
Contributor

zeripath commented Aug 5, 2020

Like CanImportLocal I think we're gonna need a similar setting here.

As we have no way of checking that the user is allowed to use the keys that are available to Gitea we cannot assume that this is safe.

This cannot go in without a way to switch it off.

@TheoLassonder
Copy link
Author

@zeripath At first I thought that too, but it's perhaps quite different from migrating/mirroring a local repo. Allowing to migrate a local repo is a real security problem, because you can for example specify a path to someones else's private repo. So effectively you cannot have private repos if you allow importing local repos.

SSH is different - if you install the Gitea user's public SSH key in a remote account, then that means any Gitea user can migrate/mirror repos from that account. But is that actually a problem?

@zeripath
Copy link
Contributor

zeripath commented Aug 5, 2020

Yes it could be a problem.

You're assuming that there will be only one key in the running Gitea server user's .ssh and that everywhere that is installed is safe to be used by users of the system. It's not hard to imagine a situation where that .ssh contains a key to push data to say the CI or to mirror the data elsewhere - an enterprising user could use this to clone using that ssh key data that they could not otherwise get to.

Now you could lock it down and have Gitea designate a keypair that it would use and then have the public key shown or even have it generate a keypair per user - You could then prefix the git command (NB: this is only available from Git 2.3.0+):

GIT_SSH_COMMAND='ssh -i private_key_file -o IdentitiesOnly=yes' git clone user@host:repo.git

or even use .ssh/config files.

However you can't just assume that any key that Gitea has access to is a key that you want to expose to any user.

@silverwind
Copy link
Member

silverwind commented Aug 5, 2020

If we'd have a option for key entry on the interface, the question whether this feature would expose the key(s) of the gitea user would not arise I'd say and it would work similar to HTTP that also requires input of username/password.

I think we'll need to support username/password auth for ssh clones too with an option to alternatively input a private key. I think it's also possible to set up sshd without any authorization, so that is something that also needs to work, so auth option for SSH should be:

@zeripath
Copy link
Contributor

zeripath commented Aug 5, 2020

I'd rather avoid us installing private keys - it would be better if we were to generate keypairs and then expose the public key.

The issue with providing username/password pairs is that we still need to prevent ssh proferring the keys. That would still need git > 2.3.0+ and the GIT_SSH_COMMAND environment variable:

GIT_SSH_COMMAND='ssh -o PreferredAuthentications=password -o PubkeyAuthentication=no' git ...

@zeripath
Copy link
Contributor

zeripath commented Aug 5, 2020

I guess the question is: Do we want to this properly in a way that medium-sized and larger users of Gitea are going to be happy to use it? Or do we provide a simplistic solution that will work for smaller users?

This PR (currently) represents the kind of solution that will work for smaller users but clearly is not going to be suitable for people like codeberg.org or gitea.com, and likely wouldn't work for a medium sized company.

It would therefore need to be possible to disable this and in fact I think that even the most perfect feature-complete solution is going to conflict with someone's policies so it will still need to be configurable.

@TheoLassonder
Copy link
Author

Thanks for the comments. The current PR is indeed only suitable for smaller users, but of course doesn't preclude adding more general support for SSH.

I've just pushed a commit that adds the option to allow non-admin users to import via SSH using server credentials.

@TheoLassonder
Copy link
Author

TheoLassonder commented Aug 8, 2020

On reflection, what @zeripath is suggesting may be a better way to go. Perhaps this PR should change to not have the .ini parameter as in commit 05decca, but instead do the following:

  • Allow import/mirror via SSH only for admins (for now)
  • Add a screen to create SSH key pairs called something like "SSH Import Keys" in the "Site Administration" (so only for admins). This lets you generate new SSH key pairs
  • On the "New Migration" screen, when you're an admin, add a selection to choose which generated SSH key pair to use
  • The migration/mirror process uses the selected key pair only

As @zeripath mentions, GIT_SSH_COMMAND requires Git 2.3.0+ (which e.g. our systems don't have), but a nice work-around exists.

@silverwind
Copy link
Member

silverwind commented Aug 8, 2020

it would be better if we were to generate keypairs and then expose the public key.

That may not be feasible in case you do not control the remote (which could be a third party which only has authorized one existing key you have access to). So I still think private key entry is the most flexible. We could AES-encrypt it.

@techknowlogick techknowlogick added this to the 1.14.0 milestone Sep 5, 2020
@stale
Copy link

stale bot commented Nov 7, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label Nov 7, 2020
@lunny lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Nov 8, 2020
@stale stale bot removed the issue/stale label Nov 8, 2020
@lunny
Copy link
Member

lunny commented Jan 27, 2021

Please resolve the conflicts.

@Hooch180
Copy link

Will this be implemented?

@lunny lunny modified the milestones: 1.14.0, 1.15.0 Feb 22, 2021
@zeripath
Copy link
Contributor

I think this is going to miss 1.15 - I will take another look at it in 1.16 and see if I can come up with a safe way of doing this. I suspect the very limited approach I discussed above could be acceptable.

@zeripath zeripath modified the milestones: 1.15.0, 1.16.0 Jun 23, 2021
@ansemjo ansemjo mentioned this pull request Aug 7, 2021
5 tasks
@lunny
Copy link
Member

lunny commented Nov 9, 2021

Please resolve the conflicts.

@lunny lunny modified the milestones: 1.16.0, 1.17.0 Nov 14, 2021
@lunny lunny removed this from the 1.17.0 milestone May 25, 2022
@Nezteb
Copy link

Nezteb commented Oct 7, 2022

@TheoLassonder Would you like assistance with this?

@TheoLassonder
Copy link
Author

@Nezteb Thanks for offering. I thought I'd quickly merge the conflicts but the Gitea code base has change a lot since this pull request was created, and essentially I'd need to start over again. However, unfortunately I don't really have the time anymore. Sorry

@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants