Gogs: Private repos are created as world-readable

[strk]

• Gogs version (or commit ref): 0.9.13.0318
• Git version: 1.7.10.4
• Operating system: Debian GNU/Linux 6.0.10 (squeeze)
• Database:
  
  
  
  • [x] PostgreSQL
  
  
  • [ ] MySQL
  
  
  • [ ] SQLite
  
  
• Can you reproduce the bug at http://try.gogs.io: Cannot check, not having access to filesystem
• Log:

2016/04/09 00:07:57 [T] action.newRepoAction: strk/test-private
2016/04/09 00:07:58 [T] Repository created[10]: strk/test-private

• Filesystem:

$ ls -l
total 4
drwxr-xr-x 8 git git 4096 Apr  9 00:07 test-private.git

Description

Unix permissions on repository files are the same for public and private repositories.
I don't know if this is by design, in dubt... ticket is filed.

[strk]

I think it makes sense for all repo to be under the control of Gogs, but maybe it would make sense to expose a UMASK setting for them ?

[AgentOak]

I think all directories (including gogs-repositories/ itself) should have filemode 750 on disk, regardless of their visibility. (Since you could have public repositories but REQUIRE_SIGNIN_VIEW enabled). For comparison, gitolite enforces filemode 700.

[strk]

Maybe it could be a systemwide configuration: permission for private repos, permission for public repos.

[strk]

The more I think about the issue, the more I'm convinced that Gogs itself should make some permission change on the filesystem on switching between private and public configuration for a repository. I can't think of any other way to give Gogs full management over repositories.

I understand the reasoning about REQUIRE_SIGNIN_VIEW mode, but if I can configure the permissions for private and public repos I can also keep them both not world-readable when enabling that setting (and a check could warn me if I dont' do that).

[strk]

As I finally resolved to add umask settings in the upstart script, would you be interested in a PR for that ? (but I only touched the debian init script, not others).

[AgentOak]

For systemd, putting a UMask= directive in the unit file would suffice.
But umasking the whole process is certainly not the most elegant solution.

I'd still like to see a dev comment on this matter @Unknwon

[unknwon]

I think this is somewhat out of Gogs duty. If someone has access to the server, private/public from Gogs are just useless. All files are managed by git command, Gogs does not CRUD those raw files.

[AgentOak]

Since gogs creates the repositories, it is gogs and only gogs duty to set proper file permissions on them.

If someone has access to the server, private/public from Gogs are just useless

That's what this issue tries to address! At the moment this is indeed very true. Because the repositories are world-readable, every user with access to the server could also access every single repository, even private ones. I think we all agree that's not quite optimal. But there is a solution to this very special problem, invented quite a few years ago: File permissions! By not making the files world-readable, only permitted users (i.e. users in the group "git") are able to access the files.

All files are managed by git command, Gogs does not CRUD those raw files.

Doesn't really change a thing. git isn't managing anything on its own, Gogs is still the application deciding what to do with the files, doesn't matter _how_ it modifies or reads them. Why would git deal with file permissions anyway? It is the responsibility of the caller to have proper user rights and set a umask.

[unknwon]

That's what this issue tries to address! At the moment this is indeed very true. Because the repositories are world-readable, every user with access to the server could also access every single repository, even private ones. I think we all agree that's not quite optimal. But there is a solution to this very special problem, invented quite a few years ago: File permissions! By not making the files world-readable, only permitted users (i.e. users in the group "git") are able to access the files.

I more believe it is the problem you run Gogs under proper user.

Doesn't really change a thing. git isn't managing anything on its own, Gogs is still the application deciding what to do with the files, doesn't matter how it modifies or reads them. Why would git deal with file permissions anyway? It is the responsibility of the caller to have proper user rights and set a umask.

Who creates the file who manages the file permission, and it is git.

[strk]

@Unknwon it seems to me from your comment in here and #2943 and #2940
that you don't give any value to integration of Gogs with other tools,
but rather see Gogs as being the only actor in the system.

This approach limits the use cases for Gogs.

For comparison, see how gitolite addresses the integration:
http://gitolite.com/gitolite/gitolite.html#gitweb-daemon

[unknwon]

@Unknwon it seems to me from your comment in here and #2943 and #2940
that you don't give any value to integration of Gogs with other tools,
but rather see Gogs as being the only actor in the system.

This approach limits the use cases for Gogs.

For comparison, see how gitolite addresses the integration:
http://gitolite.com/gitolite/gitolite.html#gitweb-daemon

Does not seem to relevant to this thread.

[AgentOak]

I more believe it is the problem you run Gogs under proper user.

I don't think you understand the issue. It doesn't matter what user gogs runs under, as gogs gives the repositories the file mode 755. The third digit indicates the permission for all users other than the owning user and group. A 5 means read (4) + execute (1) (which is actually list files in case of a directory), meaning that all users on the system can read the repository.
This is a problem, because most likely you don't want all users (including all running services!) on the server to be able to read the repositories.

Who creates the file who manages the file permission, and it is git.

No, that's just not how things work. How should the git commandline utility know on its own what users should be able to read the repository?
Git creates the repositories files using the most recently set umask. The problem is that gogs doesn't set a umask, so the default umask of 022 is used, leading to the repositories being created with filemode 755.

[edacval]

Just chmod 0770 parent directory where repositories reside and all be happy. Is not a gogs job manage file system permissions, that must be done by server administrator

[strk]

Server administrator can only set permission on files that
exist at one point in time, but as Gogs _creates_ new files
and directories, it should act responsibly and set permissions
appropriately.

That said, yes, setting parent directory permission might work
in most use cases (0750, for my case).

[unknwon]

@Marco01809

I don't think you understand the issue. It doesn't matter what user gogs runs under, as gogs gives the repositories the file mode 755.

@strk

but as Gogs _creates_ new files
and directories, it should act responsibly and set permissions
appropriately.

Said many times, not Gogs, git command line.

[AgentOak]

Just chmod 0770 parent directory where repositories reside and all be happy.

I agree it's a good workaround.

Is not a gogs job manage file system permissions, that must be done by server administrator

The server administrator certainly should take care of such security aspects, but daemons should have secure defaults and not put private data needlessly at risk.

Said many times, not Gogs, git command line.

Ok, so what is it that you do not understand?? I already explained that you actually have to tell git it should create the files with a certain filemode by setting a umask. Git isn't setting filemodes randomly on its own.

[bkcsoft]

Aaand you (as the sysop) have to tell gogs to have a umask, by configuring
your service-file...

[AgentOak]

That'll work as well, but is rather uncommon practice. And still, gogs should have it set out-of-the-box. As it is now, with a standard installation of gogs, a single security issue in any service running on the server could be enough to have all of your private repositories stolen.

[unknwon]

@Marco01809

Ok, so what is it that you do not understand?? I already explained that you actually have to tell git it should create the files with a certain filemode by setting a umask. Git isn't setting filemodes randomly on its own.

What is it that you do not understand? Tell me how to set umask when you do git init?

[AgentOak]

Well, I've never programmed in Go before but lets see if I can help you out on this one.

Turns out there is a documentation specifying how to do syscalls from Go.
https://golang.org/pkg/syscall/#Umask
Sure, doesn't state anything about how umask() works, so lets have a look at the manpage, shall we?
http://man7.org/linux/man-pages/man2/umask.2.html

umask() sets the calling process's file mode creation mask

Special emphasis should be laid upon:

A child process [...] inherits its parent's umask.

So you literally _just call it_ before you start the git process. Just as it used to be on all POSIX-compliant platforms in the past 15 years. In C you would do:

umask(0027);
system("git init");

Hell, you could even do something like
system("sh -c 'umask 027; git init'")
(Not as nice though.)

But wait, there's more!
The git developers must have noticed how insanely inconvenient it is to set a umask manually. They introduced a simpler way to specify which filemode to use: The --shared=0xxx argument. According to the git documentation found at https://git-scm.com/docs/git-init:

--shared[=(false|true|umask|group|all|world|everybody|0xxx)]
0xxx
    0xxx is an octal number and each file will have mode 0xxx. 0xxx will override users' umask(2) value.

Note that --shared has some side-effects, so setting a umask is preferred.

Please let me know anytime if I can help you with reading other documentation.

What makes me wonder however, you are the lead developer of a _self-hosted_ git service and yet don't seem to know anything about this stuff and think file permissions are "somewhat out of gogs duty". Gives you a bad feeling about gogs' security in general.

[bkcsoft]

So you _literally just call_ it before you start the git process.

As your service-file (init.d, sysvinit, systemd, etc) should do if you want that :wink:

Though I agree that the install-docs could/should mention this somewhere...

[bkcsoft]

@Marco01809 File permissions are always outside the scope of any service, since the init-system (systemd, etc) should set the umask when necessary. (unless said service is an init-system :laughing: )

[strk]

I'd still find it useful for Gogs to be able to _change_
something about repositories when switching them between
public and private, beside the initial creation.

That's not something umask alone can do.

[AgentOak]

As your service-file (init.d, sysvinit, systemd, etc) should do if you want that

The Interwebs states daemons should explicitly ignore umasks set from the outside.

The purpose of the umask is to allow users to influence the permissions given to newly created files and directories. Daemons should not allow themselves to be affected by this setting, because what was appropriate for the user will not necessarily be suitable for the daemon.

In some cases it may be more convenient for the umask to be set to a non-zero value. This is equally acceptable: the important point is that the daemon has taken control of the value, as opposed to merely accepting what it was given.

Taken from http://www.microhowto.info/howto/cause_a_process_to_become_a_daemon.html#idp34384

[bkcsoft]

So, the only proper source for how a well known/used service (apache, php-fpm, nginx, mysql, postgres) sets umask is MySQL which does it with bitwise operations instead of using umask().
_All_ the others recommend using the init-system for umask settings :unamused:

[AgentOak]

Well yes, MySQL reads the environment variable UMASK (Notice the fallback to a secure filemode of 640 if it is not present!), then passes it to the open() call instead of using umask() beforehand (open will create a file when O_CREAT parameter is given).

nginx handles file permissions itself as well, without relying on an external variable at all.
At startup it even overwrites the umask set by the user (or by an init script, possibly).
nginx only writes temporary files and logs, both are handled well: See this and this and this.

php-fpm is not managing user data itself (that would be left to an actual php application), except logs again and session files, which use this function. Both of them state the filemode explicitly.

I haven't checked postgres and apache.

Since we are talking about a subprocess here we cannot directly specify what gets passed to the open() call. But by using umask we can mask the mode passed to open(), so this is exactly what we need.

Setting a umask in the init file might be okay for users trying to achieve something not otherwise possible, but it isn't something daemons should rely on for proper file permission handling.
I'm not totally opposed to specifying the umask in the init files but there are several drawbacks:

• What if a user or distribution rolls their own init script? (as packager.io does)
• What about existing gogs installations? (chmod is needed as well)
• It will not satisfy OPs request (having different permissions for public/private repos)
• By using a umask in the init file, we are not only umasking the git subprocesses, but the whole gogs process. What if gogs at some point should create certain files as world-readable? It would have to use umask() then, only the other way around (widening permissions this time)

[bkcsoft]

Okey, I'm convinced. umask(0027) is fine? or should it be a setting? setting different on public/private would end up in a performance hit (barely noticable but still...), and is a bitch to manage, so I'm more or less against that unless someone has a valid argument for why it's needed :wink:

[edacval]

You really want to dublicate filesystem permission management in gogs side? Good luck with that, especialy on Windows NTFS :) (Owner, permission inheritance... ). Many times said - is not gogs job to duplicate administrator role. if your company sysop can not deal with it, fire or shot him, depending on local laws :)

[AgentOak]

umask(0027) is fine? or should it be a setting?

I believe 0027 will ultimately be the standard choice anyway.
For example when gogs is run as user "git":

• gogs will be able to read+write files
• if you want another service/user on the system to be able to read and list (but not write) the repositories you just add them into the group "git"
• all other users can neither read, write, nor list the repositories managed by gogs

Good luck with that, especialy on Windows NTFS :) (Owner, permission inheritance... )

This issue was primarily about file permissions on unix-compatible systems and I honestly don't know anything about permissions on windows. Windows is very different anyway.

Many times said - is not gogs job to duplicate administrator role.

Daemons handling their file's permissions is established standard. Just to make sure I also checked postgres (uses umask 077 when creating new databases -> filemode 600). All well known services do it, so please back up your contrary claims.

[edacval]

Any well known OS already has everything to manage user access to files and directories.
On Unix:
chown gogs_user:trusted_gogs_users /srv/gogs; chmod 0700|0750|0770|0755 /srv/gogs/repos; umask 00xx; su -c "gogs web -c /etc/gogs/app.ini" gogs
and you are done.
Any daemon side filesystem permission management will be:

1. Non portable.
2. Slow.
3. Error prone.
4. Silly duplicate the functionality of the OS
5. Does not deal with SELinux, Apparrmor,GRsecurity, NTFS ...

[strk]

About portability:
umask system call conforms to SVr4, 4.3BSD, POSIX.1-2001.

[AgentOak]

umask 00xx; su -c "gogs web -c /etc/gogs/app.ini" gogs

As explained in the howto I linked a bit above, daemons should explicitly reject a previously set umask and deal with that themselves.

1 Non portable.

Only applies to windows. Yet, all well-known services implement file permission handling even if they also target windows.

2 Slow.
3 Error prone.

???

4 Silly duplicate the functionality of the OS

Actually, the commands provided by the OS are just a convenient way to call these syscalls from a shell.

5 Does not deal with SELinux, Apparrmor,GRsecurity, NTFS ...

The latter is just number 1 again. What do the other three have to do with anything? We don't need to use them to have proper file permissions and having file permissions does not prevent anyone from using these. They are a completely different topic.

Again, please back up your claim that a daemon shouldn't deal with file permissions appropriately by providing material to read up on.

[edacval]

Please, read all your own reference, and not just parts that you like

[mjianr]

There's a one simple reason to stay way from daemon setting it's own permissions - unnecessary hassle.

1. Does not deal with SELinux, Apparrmor,GRsecurity, NTFS ...

It could cause a lot of complications within code to include exceptions for mentioned instances and I believe most of people would agree to keep it simple.

Secondly, I think in most Gogs uses local system users shouldn't access repos so simple permission change for parent dir is more than enough to solve such security issue. If for reasons you would want to allow local user to access certain repo, solutions like using symlink could be applied, but as @edacval pointed out, up to admin.

[strk]

A symlink cannot circumvent directory permissions

[mjianr]

@strk true, my bad. Probably ACL or bind mount is a more appropriate example.

[unknwon]

I'm not pro on file permissions, but I assume any child directory permission follows their parent permission?

[strk]

Wrong assumption.
Permission on child directories is independent of permission of parent
ones, with the exception of the "execute" bit, which prevents
descending into the childs of the parent, so that you cannot reach any
child of a directory which you don't have execute permission on.

Note you can still reach files which are child of a directory you
don't have execute permission on IFF these files are reachable from
other (executable) paths via hard links.

[unknwon]

Wrong assumption.
Permission on child directories is independent of permission of parent
ones, with the exception of the "execute" bit, which prevents
descending into the childs of the parent, so that you cannot reach any
child of a directory which you don't have execute permission on.

If that's the case, I still do not think it's Gogs's job to set every bit.

umask sounds good but it is not very compatible.

[AgentOak]

umask is as compatible as it gets, since it is standardized in the POSIX specification (Linux, OpenBSD, FreeBSD and Darwin/OS X implement it). umask is supported on 100% of the systems that use unix filemodes.

[waldiTM]

It is Gogs's job to protect the data it gets. So setting minimal permissions by default is the way to go.