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

Properly set working directory in authorized_keys #15973

Closed
2 of 6 tasks
duxovni opened this issue May 24, 2021 · 10 comments
Closed
2 of 6 tasks

Properly set working directory in authorized_keys #15973

duxovni opened this issue May 24, 2021 · 10 comments
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@duxovni
Copy link

duxovni commented May 24, 2021

  • Gitea version (or commit ref): 1.14.2
  • Git version: 2.20.1
  • Operating system: Debian 10
  • Using gitea from the official downloads
  • Running gitea using the sample systemd service
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No

Description

My app.ini doesn't explicitly configure any of the directories gitea uses, which means they're all set as their default values which are relative to AppWorkPath. In the systemd service, the GITEA_WORK_DIR environment variable is explicitly set as /var/lib/gitea, which is where I set up all of gitea's directories, so everything works fine.

However, I'm running gitea using the system SSH server. In authorized_keys, that environment variable isn't set, which means that AppWorkPath defaults to the location of the gitea binary (/usr/local/bin, as recommended in the official install instructions). With an incorrect AppWorkPath, gitea can't find any of the files it needs to run (aside from /etc/gitea/app.ini, which is preserved as a command-line option in authorized_keys, but which doesn't explicitly configure any of the other directories on my system). Therefore, nobody can access their repositories over SSH.

To fix this, we need to do one of:

  • add a setting in app.ini to explicitly configure AppWorkPath
  • add the --work-path command-line flag to the command in authorized_keys, as was suggested in Need to add --work-path to authorized_keys #9481 but not implemented
  • encourage placing the gitea binary in the desired work path, rather than in /usr/local/bin
  • if the repository path, data path, etc, ought to be explicitly configured, then get rid of the defaults and die with an appropriate error when they're not configured
@duxovni
Copy link
Author

duxovni commented May 25, 2021

I have a two-line patch that applies the second approach, adding --work-path in authorized_keys, if that's the approach we want to go for.

Making AppWorkPath configurable in app.ini feels to me like it'd be a cleaner approach; having it default to the directory the binary is in feels really weird. But I'd need to do more diving to understand how gitea startup works and at what point app.ini gets read.

@noerw
Copy link
Member

noerw commented May 25, 2021

The second approach you have a patch for seems reasonable. It breaks when the gitea installation is moved, but in that case the "regenerate authorized_keys" task should be run anyway, so I see no problem with that approach.

When you send a PR, you'll get more eyes on this :)

@noerw noerw added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label May 25, 2021
@zeripath
Copy link
Contributor

I don't understand why you need this.

gitea serv and gitea hook shouldn't be using anything that depends on AppWorkPath

@noerw
Copy link
Member

noerw commented May 26, 2021

@zeripath RepoRootPath is indirectly determined by AppWorkPath via default values

gitcmd.Dir = setting.RepoRootPath

@zeripath
Copy link
Contributor

Ah! Thanks @noerw.

Is that all that depends on the AppWorkDir? If so we provide configuration for that:[repository] ROOT

Another option is to recompile gitea as per https://docs.gitea.io/en-us/install-from-source/#changing-default-paths with LDFLAGS='-X \"code.gitea.io/gitea/modules/setting.AppWorkPath=/var/lib/gitea\"' to ensure that AppWorkPath is always correct.

I'm kinda loathe to suggest we need to add yet another complication to the authorized_keys file. Yeah this one is easy but for the vast majority of cases this will just slow down authorized_keys parsing.

@duxovni
Copy link
Author

duxovni commented May 27, 2021

If it's bad to leave [repository] ROOT as it's default value, then that probably shouldn't be the default; in that case gitea should require that it be explicitly configured. At minimum, gitea should fail with an explanatory error message when START_SSH_SERVER is false and the repository root isn't explicitly configured. If that approach is preferable we can note that here and change the code accordingly.

@zeripath
Copy link
Contributor

It's not always bad to leave ROOT at the default value and if I recall correctly on the install page we always set it. The issue is running gitea as if it's a FHS complaint binary - when it's not been compiled to be that (or being wrapped as such) without setting ROOT.

I appreciate that a lot of people want gitea to be FHS compliant and I wish it was too. That is why I have spent an inordinate amount of time making it possible to make gitea be compiled as FHS compliant and writing a wrapper script for it if people won't compile it.

I really don't think that adding more options to hack around this is the right approach. I'm happy to suggest we change the documentation to add a specific warning to set ROOT in these circumstances or use one of the many other options that I have worked hard to make possible, but if it's only ROOT then it seems silly to have to every authorized keys file have to set it explicitly. Especially when the vast majority of people don't need it - in fact only a minority of people should need to set -c if they're building properly because the default custom path can be compiled in.

Seriously Gitea is not FHS compliant out of the box - it can be compiled to be so or shimmed around but it's designed to work around a different model - the click and run model. I've suggested multiple times that we build and release FHS compliant versions.

However if you really feel strongly that this is a significant user issue, and you cannot or will not set ROOT or recompile I will not stand in the way.

@duxovni
Copy link
Author

duxovni commented May 28, 2021

That makes sense, thank you for the explanation. I'd also love to see an FHS-compliant official build, but honestly a wrapper script that sets GITEA_WORK_DIR would work fine for my case. I think it'd be worth including that in the documentation, and in particular, noting that the wrapper script needs to use exec -a (or whatever non-bash equivalent) so that the authorized_keys commands will also refer to the wrapper script and not to the real gitea binary.

@zeripath
Copy link
Contributor

@zeripath
Copy link
Contributor

zeripath commented Aug 8, 2021

I think we can consider this fixed by #16003 - any user that needs work-path set in their authorized_keys can do this by setting the template to do this.

@zeripath zeripath closed this as completed Aug 8, 2021
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants