Skip to content

docs: add SSH passthrough instructions to with-docker-rootless (#17505) #17508

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

Merged
merged 4 commits into from
Nov 1, 2021

Conversation

rmsc
Copy link
Contributor

@rmsc rmsc commented Oct 31, 2021

The passthrough is based upon AuthorizedKeysCommand and a custom shell wrapper that forwards commands to the container.

Close #17505

The passthrough is based upon AuthorizedKeysCommand and a custom shell
wrapper that forwards commands to the container.
Copy link
Member

@techknowlogick techknowlogick left a comment

Choose a reason for hiding this comment

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

Thanks for contributing this PR :)

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Oct 31, 2021
@techknowlogick techknowlogick added type/docs This PR mainly updates/creates documentation and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 31, 2021
@techknowlogick techknowlogick added this to the 1.16.0 milestone Oct 31, 2021
@zeripath
Copy link
Contributor

zeripath commented Nov 1, 2021

I'm slightly confused here.

The results of the AuthorizedKeysCommand will be a line like:

command="/path/to/gitea/in/docker/container --config=/path/to/app.ini serv key-1",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa AAAA... user@localhost

The host SSH would then attempt to run that command.

How are you ensuring that that command can be run?

I worry that the gitea-shell you're suggesting here implies that your host isn't running the command? (or more likely this is only working because you still have an /app/gitea/gitea shadow on the host.)


I think your passthrough idea is trying to suggest a different kind of passthrough.

The original docs suggested:

 client -- SSH --> host -- SSH --> gitea_container. 

These appear to be trying to do:

client -- SSH --> host -- docker --> gitea_container

Is that right?


If so, then instead of the shadow /app/gitea/gitea being a SSH passthrough you'd simply need to be a docker passthrough
like your gitea-shell idea.

Your docker shell idea could simply shadow /app/gitea/gitea and it would all just work.


Other thoughts:

If actual network passthrough was desired, there is nothing preventing you from having the gitea executable on the host system with an app.ini that has a different [server] SSH_AUTHORIZED_KEYS_COMMAND_TEMPLATE and appropriate INTERNAL_ROOT_URL. gitea keys simply communicates over HTTP with the running gitea server at /api/private/ssh/authorized_keys. (You could not run the gitea serv command directly though.)

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Nov 1, 2021
@rmsc
Copy link
Contributor Author

rmsc commented Nov 1, 2021

The host SSH would then attempt to run that command.

Not exactly, the host SSH would pass it verbatim as an argument to gitea-shell -c. Which would run it in the container.

How are you ensuring that that command can be run?

I'm not. In the end the command is passed to the container. If it fails, it fails there and never on the host.

/usr/bin/docker exec -i --env SSH_ORIGINAL_COMMAND="$SSH_ORIGINAL_COMMAND" gitea sh "$@"

The shell wrapper above then calls docker exec, which runs sh "$@" inside the container, not on the host.

When cloning a repo, for instance, what ends up happening is that the following is run inside the gitea container:

SSH_ORIGINAL_COMMAND="git-upload-pack 'user/repo.git'" sh -c "/path/to/gitea/in/docker/container --config=/path/to/app.ini serv key-1"

I worry that the gitea-shell you're suggesting here implies that your host isn't running the command? (or more likely this is only working because you still have an /app/gitea/gitea shadow on the host.)

I hope the above clarifies how this works. I never had an /app/gitea/gitea in the first place 😄

I think your passthrough idea is trying to suggest a different kind of passthrough.
(...)
Is that right?

Yes, I suppose so.

If so, then instead of the shadow /app/gitea/gitea being a SSH passthrough you'd simply need to be a docker passthrough like your gitea-shell idea.

Your docker shell idea could simply shadow /app/gitea/gitea and it would all just work.

I suppose that might work, BUT:

  • Having an /app/gitea/gitea on the host feels like a hack. It's a fixed hardcoded non-standard path, whereas the shell wrapper could be anywhere and still work.
  • A shell wrapper runs all commands inside the container, which is more secure. Having a shadowed gitea on the host means that if a malicious entity could somehow override the command in authorized_keys (say by hacking the SSH_AUTHORIZED_KEYS_COMMAND_TEMPLATE), it could run other commands on the host's shell. This definitely defeats the benefits of having containers in the first place.

Other thoughts:

If actual network passthrough was desired, there is nothing preventing you from having the gitea executable on the host system with an app.ini that has a different [server] SSH_AUTHORIZED_KEYS_COMMAND_TEMPLATE and appropriate INTERNAL_ROOT_URL. gitea keys simply communicates over HTTP with the running gitea server at /api/private/ssh/authorized_keys. (You could not run the gitea serv command directly though.)

I don't think having a gitea executable on the host is desirable at all when running a containerized system.

That said, I can see a network passthrough being needed between different hosts. Perhaps your suggestion above could be made to work with a containerized gitea on the 'gateway' host?

@zeripath
Copy link
Contributor

zeripath commented Nov 1, 2021

The host SSH would then attempt to run that command.

Not exactly, the host SSH would pass it verbatim as an argument to gitea-shell -c.

Aha! I'd forgotten that the the user's shell is used to parse the command. This now makes sense.

I think your passthrough idea is trying to suggest a different kind of passthrough.
(...)
Is that right?

Yes, I suppose so.

OK we should probably be explicit that this will require that the host git user needs to be allowed to run docker exec. This may/may not be the default.

Other thoughts:

If actual network passthrough was desired, there is nothing preventing you from having the gitea executable on the host system with an app.ini that has a different [server] SSH_AUTHORIZED_KEYS_COMMAND_TEMPLATE and appropriate INTERNAL_ROOT_URL. gitea keys simply communicates over HTTP with the running gitea server at /api/private/ssh/authorized_keys. (You could not run the gitea serv command directly though.)

...

That said, I can see a network passthrough being needed between different hosts. Perhaps your suggestion above could be made to work with a containerized gitea on the 'gateway' host?

You could use a combination of the two techniques. IIRC the AuthorizedKeysCommand is run by root - so you should be able to use the docker shell technique to run gitea keys but just pass a different app.ini that has the SSH_AUTHORIZED_KEYS_COMMAND_TEMPLATE to use the network passthrough ssh command which doesn't have to refer to apppath at all.

@rmsc
Copy link
Contributor Author

rmsc commented Nov 1, 2021

OK we should probably be explicit that this will require that the host git user needs to be allowed to run docker exec. This may/may not be the default.

That's a good point, thanks! I'll update the documentation.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Nov 1, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #17508 (9c413c2) into main (599ff1c) will increase coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 9c413c2 differs from pull request most recent head bfe553d. Consider uploading reports for the commit bfe553d to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main   #17508      +/-   ##
==========================================
+ Coverage   45.50%   45.51%   +0.01%     
==========================================
  Files         793      793              
  Lines       88772    88772              
==========================================
+ Hits        40396    40405       +9     
+ Misses      41863    41847      -16     
- Partials     6513     6520       +7     
Impacted Files Coverage Δ
modules/notification/mail/mail.go 35.29% <0.00%> (-2.95%) ⬇️
modules/process/manager.go 72.83% <0.00%> (-2.47%) ⬇️
modules/notification/ui/ui.go 60.86% <0.00%> (-1.45%) ⬇️
models/notification.go 65.28% <0.00%> (-0.88%) ⬇️
services/pull/pull.go 41.78% <0.00%> (-0.41%) ⬇️
models/issue_comment.go 51.31% <0.00%> (-0.30%) ⬇️
models/repo_list.go 79.21% <0.00%> (+0.78%) ⬆️
modules/log/file.go 75.20% <0.00%> (+1.60%) ⬆️
modules/log/event.go 62.50% <0.00%> (+1.85%) ⬆️
modules/queue/workerpool.go 52.29% <0.00%> (+4.96%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 599ff1c...bfe553d. Read the comment docs.

@zeripath zeripath merged commit e2995ef into go-gitea:main Nov 1, 2021
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
…tea#17505) (go-gitea#17508)

The passthrough is based upon AuthorizedKeysCommand and a custom shell wrapper that forwards commands to the container over the docker pipe.
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/docs This PR mainly updates/creates documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tested rootless SSH passthrough configuration to the docs
5 participants