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

gitea and read-only filsystems #11855

Closed
micheelengronne opened this issue Jun 11, 2020 · 19 comments · Fixed by #11876
Closed

gitea and read-only filsystems #11855

micheelengronne opened this issue Jun 11, 2020 · 19 comments · Fixed by #11876
Labels
type/enhancement An improvement of existing functionality
Milestone

Comments

@micheelengronne
Copy link

Gitea wants to modify files even if they are present and with the content gitea wants to put in.

Therefore, Gitea breaks on read-only filesystems (like an hardened docker container.)

My problem is simple.

This method wants to modify a .gitconfig file:

func Init(ctx context.Context) error {

It runs even if the .gitconfig file exists with the correct content.

Is it possible to stop that behaviour ?

Thanks.

@CirnoT
Copy link
Contributor

CirnoT commented Jun 11, 2020

We rely on git config --global to make changes to Git config file

@micheelengronne
Copy link
Author

Yes, but can we disable this method when the file already exists ?

@zeripath
Copy link
Contributor

Just set the HOME environment variable as appropriate to somewhere that can be changed.

@CirnoT
Copy link
Contributor

CirnoT commented Jun 11, 2020

Yes, but can we disable this method when the file already exists ?

No, because we don't know whether correct values are set or not. The fact that file exists tells us nothing. The .gitconfig is supposed to be writable should new version introduce some new config to be set there.

@micheelengronne
Copy link
Author

My usecase happens in a container environment. A new version is introduced by creating a new container image, not updating the existing one.

@micheelengronne
Copy link
Author

micheelengronne commented Jun 12, 2020

I tried to do an init container that sets everything and a container with gitea web but still the gitea web cli wants to use this method.

It is a security issue as it forces to use a read-write filesystem for just a file that can easily be commmited in a git.

Can we, at least, have an option to use gitea web and other production-running state commands in a read-only filesystem ?

@zeripath
Copy link
Contributor

@micheelengronne have you set the HOME environment variable appropriately?

@micheelengronne
Copy link
Author

micheelengronne commented Jun 12, 2020

Changing the HOME variable is not a solution either. In an immutable hardened container, only data should change, nothing else.

@micheelengronne
Copy link
Author

So, what can change is the content of ROOT according to the config cheat sheet.

@micheelengronne
Copy link
Author

micheelengronne commented Jun 12, 2020

I am sorry to be strict on that. But this is an absolute condition to make gitea run in security hardened infrastructures. Containers must work with read-only FS and read-only configurations and secrets.

@zeripath
Copy link
Contributor

Right the issue is that Git is making the change to the file.

We are limited in how we can tell Git to look for its .gitconfig.

Git uses HOME to determine the global gitconfig which we use to set various global settings and to look up various things.

So I ask again, have you tried setting the HOME variable to a mutable place?

@zeripath
Copy link
Contributor

If that works then can consider if there is a place for using ROOT to artificially change the HOME for git etc. but without knowing that then we're likely leading down a blind alley and we might have to think about if we need to set things a different way eg. through the default args system

@micheelengronne
Copy link
Author

micheelengronne commented Jun 12, 2020

To be sure we talk about the same thing. If I move HOME, I move my user directory, right ? So, I move all configurations in it ?

Did I understand correctly ?

In that way, configurations are not immutables and that will not pass hardened security tests.

Why can't we have a way to include a .gitconfig file containing the same content and for the method to check if the file exists with the correct content prior to create it ?

@zeripath
Copy link
Contributor

No I mean just set the HOME environment variable when you run gitea

@micheelengronne
Copy link
Author

But that will screw up my user directory.

For instance, my user is git. I put this file content in /home/git/.gitconfig.

[user]
	name = Gitea
	email = [email protected]
[core]
	quotepath = false
	commitGraph = true
[gc]
	writeCommitGraph = true

Then the init method should not try to write this file.

Do you suggest, I move HOME to /home/git/mutable and make this directory writable ?

If it works that's better than nothing but that will still break the security hardening check as .gitconfig is also a configuration file.

I try.

@micheelengronne
Copy link
Author

That works but that's not really the best solution. It would be far better to test if the file exists with the correct values before trying to create it.

If a gitea update occurs that change the file content, the CI job that creates the docker (podman) image would break so future compatibilities would be ensured by that way.

@micheelengronne
Copy link
Author

micheelengronne commented Jun 12, 2020

BTW, the 2 first git executions do what I think is the correct way to do.

for configKey, defaultValue := range map[string]string{"user.name": "Gitea", "user.email": "[email protected]"} {

They check if the values exist and they are the same than the ones provided and they execute the git command if one of these assertions is false.

The problem is for these lines:

if _, stderr, err := process.GetManager().Exec("git.Init(git config --global core.quotepath false)",

if _, stderr, err := process.GetManager().Exec("git.Init(git config --global core.commitGraph true)",

if _, stderr, err := process.GetManager().Exec("git.Init(git config --global gc.writeCommitGraph true)",

They do not check so they always try to write.

I am not fluent enough in go to do it myself right now.

@zeripath
Copy link
Contributor

So any configuration in $HOME/.gitconfig can be overridden by a repository's local "project" git config ($GITEA_REPOSITORIES/owner/name.git/config) including in particular diff.tool. Gitea needs to be able to change these config files in order to manage mirrors and remotes amongst other things - and we regularly create temporary repositories which git gives a local project config. I'm not saying that we shouldn't make changing $HOME/.gitconfig unnecessary - but unless you can get to the point that these local project config files are controllable or removable - which I haven't been able to find a way of doing - locking down the $HOME/.gitconfig is of very limited use. $HOME/.gitconfig is the GLOBAL config only in that it provides defaults to the local project git configs - it cannot and does not override local project git config.

@lafriks lafriks added the type/enhancement An improvement of existing functionality label Jun 13, 2020
@lafriks lafriks added this to the 1.13.0 milestone Jun 13, 2020
zeripath added a commit to zeripath/gitea that referenced this issue Jun 13, 2020
techknowlogick added a commit that referenced this issue Jun 13, 2020
* Only write to global gitconfig if necessary

Fix #11855

Signed-off-by: Andrew Thornton <[email protected]>

* placate lint

Signed-off-by: Andrew Thornton <[email protected]>

Co-authored-by: techknowlogick <[email protected]>
@micheelengronne
Copy link
Author

I know that. But that was not the point. Anyway, thanks for the commit.

ydelafollye pushed a commit to ydelafollye/gitea that referenced this issue Jul 31, 2020
* Only write to global gitconfig if necessary

Fix go-gitea#11855

Signed-off-by: Andrew Thornton <[email protected]>

* placate lint

Signed-off-by: Andrew Thornton <[email protected]>

Co-authored-by: techknowlogick <[email protected]>
@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
type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants