-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Specify work-path in SSH authorized keys #22754
Conversation
Gusted
commented
Feb 4, 2023
- This should prevent SSH failures from happening as described in Warn on SSH connection for incorrect configuration #19317
- This should prevent SSH failures from happening as described in go-gitea#19317
- Backport go-gitea#22754 - This should prevent SSH failures from happening as described in go-gitea#19317
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a bodge but I guess we have to do it. It would be better to not have to depend on the work-path at all in when using gitea serv/hook and I'm surprised that this has sneaked in once again.
I'll say this again: I don't immediately understand why the work-path is even needed in gitea serv and I would much rather it was not necessary. If anyone can determine why this has happened it would be very helpful because I'd rather get rid of this requirement COMPLETELY instead of adding more crap to the authorized_keys. There may be a lot of keys in the authorized_keys file and this will make that file bigger and slower to parse. Whilst the authorizedkeyscommand functionality exists precisely to avoid slowdowns here - many people will not be aware and I'm fairly sure that our dockers still use authorized_keys instead of that command. |
I do not understand either, is there any detailed comment about why the SSH failures happens? If it's a rare case, site admin could fine tune the config according to the document: "Possible keys are: AppPath, AppWorkPath, CustomConf, CustomPath", there are even more options for them. |
RepoRootPath's default value depending on |
Yup, there are too many The question is: by default, these paths just work, the WorkPath is also auto detected by Gitea's binary file. In my opinion, if and only if a site admin:
In this case:
That's why I think it's not necessary to make SSH_AUTHORIZED_KEYS_COMMAND_TEMPLATE more complex at the moment. Just my opinion, not blocker. Correct me if I am wrong. |
Since the file is controlled by Gitea, I think we can merge this at the moment. And we can rewrite it after all paths have adjusted. |
Please resolve the conflicts. |
How about we attempt to see if the AppDataPath is dependent on the WorkPath and only if it is dependent should we add the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking per @zeripath 's comment. Mergers please feel free to dismiss this as needed. Just needed to set the blocker so it isn't merged without the additional investigation requested.
If a user's `app.ini` contains a `APP_DATA_PATH` which refers to a non-absolute path then `gitea serv` etc. become dependent on the `AppWorkPath`. `gitea serv` may then require `--work-path` to be set in the `authorized_keys` if the `AppWorkPath` determined by `gitea web` and `gitea serv` are different. This would occur if `GITEA_WORK_DIR` is set, `--work-path` is used to run `gitea web` or if the AppPath cannot be determined at start-up. This PR adds some code to attempt to automatically determine if this is necessary and adds some documentation to suggest adding `--work-path` to the template. This should prevent SSH failures from happening as described in go-gitea#19317 Replace go-gitea#22754 Signed-off-by: Andrew Thornton <[email protected]>
Quote my comments here (and close this one) Instead of hacking and patching, we need a fundamental solution. |
# The problem There were many "path tricks": * By default, Gitea uses its program directory as its work path * Gitea tries to use the "work path" to guess its "custom path" and "custom conf (app.ini)" * Users might want to use other directories as work path * The non-default work path should be passed to Gitea by GITEA_WORK_DIR or "--work-path" * But some Gitea processes are started without these values * The "serv" process started by OpenSSH server * The CLI sub-commands started by site admin * The paths are guessed by SetCustomPathAndConf again and again * The default values of "work path / custom path / custom conf" can be changed when compiling # The solution * Use `InitWorkPathAndCommonConfig` to handle these path tricks, and use test code to cover its behaviors. * When Gitea's web server runs, write the WORK_PATH to "app.ini", this value must be the most correct one, because if this value is not right, users would find that the web UI doesn't work and then they should be able to fix it. * Then all other sub-commands can use the WORK_PATH in app.ini to initialize their paths. * By the way, when Gitea starts for git protocol, it shouldn't output any log, otherwise the git protocol gets broken and client blocks forever. The "work path" priority is: WORK_PATH in app.ini > cmd arg --work-path > env var GITEA_WORK_DIR > builtin default The "app.ini" searching order is: cmd arg --config > cmd arg "work path / custom path" > env var "work path / custom path" > builtin default ##⚠️ BREAKING If your instance's "work path / custom path / custom conf" doesn't meet the requirements (eg: work path must be absolute), Gitea will report a fatal error and exit. You need to set these values according to the error log. ---- Close #24818 Close #24222 Close #21606 Close #21498 Close #25107 Close #24981 Maybe close #24503 Replace #23301 Replace #22754 And maybe more
# The problem There were many "path tricks": * By default, Gitea uses its program directory as its work path * Gitea tries to use the "work path" to guess its "custom path" and "custom conf (app.ini)" * Users might want to use other directories as work path * The non-default work path should be passed to Gitea by GITEA_WORK_DIR or "--work-path" * But some Gitea processes are started without these values * The "serv" process started by OpenSSH server * The CLI sub-commands started by site admin * The paths are guessed by SetCustomPathAndConf again and again * The default values of "work path / custom path / custom conf" can be changed when compiling # The solution * Use `InitWorkPathAndCommonConfig` to handle these path tricks, and use test code to cover its behaviors. * When Gitea's web server runs, write the WORK_PATH to "app.ini", this value must be the most correct one, because if this value is not right, users would find that the web UI doesn't work and then they should be able to fix it. * Then all other sub-commands can use the WORK_PATH in app.ini to initialize their paths. * By the way, when Gitea starts for git protocol, it shouldn't output any log, otherwise the git protocol gets broken and client blocks forever. The "work path" priority is: WORK_PATH in app.ini > cmd arg --work-path > env var GITEA_WORK_DIR > builtin default The "app.ini" searching order is: cmd arg --config > cmd arg "work path / custom path" > env var "work path / custom path" > builtin default ##⚠️ BREAKING If your instance's "work path / custom path / custom conf" doesn't meet the requirements (eg: work path must be absolute), Gitea will report a fatal error and exit. You need to set these values according to the error log. ---- Close go-gitea#24818 Close go-gitea#24222 Close go-gitea#21606 Close go-gitea#21498 Close go-gitea#25107 Close go-gitea#24981 Maybe close go-gitea#24503 Replace go-gitea#23301 Replace go-gitea#22754 And maybe more # Conflicts: # cmd/web.go