Skip to content

Dockerfile: Support socat use cases #13208

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 3 commits into from
Oct 22, 2020
Merged

Conversation

bbros-dev
Copy link
Contributor

@bbros-dev bbros-dev commented Oct 19, 2020

In some contexts it is necessary to provide access to Gitea via TCP ports and unix sockets.
Gitea (gitea web) can be configured to listen for connections via unix-socket or TCP port, but not both.
When Gitea is installed to the host this limitation can be worked around by installing socat on the host.
When running Gitea from a container this limitation cannot be workaround.

Add socat to Gitea container.

In some contexts it is necessary to provide access to Gitea via TCP ports and unix sockets.
Gitea (`gitea web`) can be configured to listen for connections via unix-socket or TCP port, but not both.
When Gitea is installed to the host this limitation can be worked around by installing socat on the host.
When running Gitea from a container this limitation cannot be workaround.

Add socat to Gitea 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 PR, just wondering why such a specific version of socat?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 19, 2020
@bbros-dev
Copy link
Contributor Author

Thanks for PR, just wondering why such a specific version of socat?

Our practice to try eliminate surprises, and socat has that versioning convention.

@lafriks
Copy link
Member

lafriks commented Oct 19, 2020

It does not build as alpine has 1.7.3.4-r0 version currently. We should stick to same convention as other packages, either none of them with specific version or all of them :)

@lafriks lafriks added type/enhancement An improvement of existing functionality topic/distribution This PR changes something about the packaging of Gitea labels Oct 19, 2020
@lafriks lafriks added this to the 1.14.0 milestone Oct 19, 2020
@codecov-io
Copy link

codecov-io commented Oct 19, 2020

Codecov Report

Merging #13208 into master will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #13208   +/-   ##
=======================================
  Coverage   42.16%   42.17%           
=======================================
  Files         684      684           
  Lines       75540    75540           
=======================================
+ Hits        31851    31856    +5     
+ Misses      38465    38462    -3     
+ Partials     5224     5222    -2     
Impacted Files Coverage Δ
modules/charset/charset.go 68.53% <0.00%> (-4.50%) ⬇️
models/unit.go 46.57% <0.00%> (-2.74%) ⬇️
routers/repo/view.go 37.47% <0.00%> (-0.65%) ⬇️
services/pull/pull.go 41.27% <0.00%> (+0.49%) ⬆️
modules/log/event.go 61.79% <0.00%> (+1.88%) ⬆️
modules/queue/workerpool.go 62.04% <0.00%> (+2.04%) ⬆️
modules/util/timer.go 85.71% <0.00%> (+42.85%) ⬆️

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 e918636...74e4afd. Read the comment docs.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Oct 19, 2020
@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 Oct 21, 2020
@6543
Copy link
Member

6543 commented Oct 21, 2020

🚀

@techknowlogick techknowlogick merged commit ff50274 into go-gitea:master Oct 22, 2020
@sapk
Copy link
Member

sapk commented Oct 31, 2020

I think we should revert this commit for security reason.
Introducing socat could allow an attacker with a limited shell to pop a permanent two-way connection.
The very few users that need socat can easily create a custom image based on gitea one with socat or even add a new step to the startup script to load socat on gitea image, or even simply mounting the binary, or any other method that could add socat.
And they can do so by mitigating the risk by any mean they need.

This is not a big security risk but we shouldn't make it more easy for an attacker by default.

sapk added a commit to sapk-fork/gitea that referenced this pull request Oct 31, 2020
@6543 6543 added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Oct 31, 2020
@6543
Copy link
Member

6543 commented Oct 31, 2020

☝️ because of #13369

lafriks pushed a commit that referenced this pull request Oct 31, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
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. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/distribution This PR changes something about the packaging of Gitea type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants