Caddy log file permissions

[pascalgn]

Caddy uses https://github.com/natefinch/lumberjack and some time ago they switched the log file permissions from 0600 to 0644: https://github.com/natefinch/lumberjack/pull/83/files

Now, some users (me included) have issues due to these too-restrictive file permissions: https://caddy.community/t/change-file-mask-for-caddy-log-files/22519

There is an issue in the upstream library, but it currently doesn't look like it will be fixed (although to be honest, the proposed PRs don't look very good to me) natefinch/lumberjack#164

However, what lumberjack does is, it will take the permissions of the file if it exists. Now, in Caddy's filewriter.go we already have

// OpenWriter opens a new file writer.
func (fw FileWriter) OpenWriter() (io.WriteCloser, error) {
	// roll log files by default
	if fw.Roll == nil || *fw.Roll {
		if fw.RollSizeMB == 0 {
			fw.RollSizeMB = 100
		}
		if fw.RollCompress == nil {
			compress := true
			fw.RollCompress = &compress
		}
		if fw.RollKeep == 0 {
			fw.RollKeep = 10
		}
		if fw.RollKeepDays == 0 {
			fw.RollKeepDays = 90
		}

		return &lumberjack.Logger{
			Filename:   fw.Filename,
			MaxSize:    fw.RollSizeMB,
			MaxAge:     fw.RollKeepDays,
			MaxBackups: fw.RollKeep,
			LocalTime:  fw.RollLocalTime,
			Compress:   *fw.RollCompress,
		}, nil
	}

	// otherwise just open a regular file
	return os.OpenFile(fw.Filename, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0o666)
}

So I'm proposing to change this so we always create the file (even if no rolling is configured), so that lumberjack will pick up these permissions. This would also help to prevent the issue that log files are created with mode 0666, but when users configure log rotation, the log files suddenly get created with mode 0600.

What do you think?

[francislavoie]

I think someone should fork Lumberjack and rewrite it in such a way that it works better for Caddy 😅 see the discussion in natefinch/lumberjack#160 We would do it ourselves if we had time, but it's too big an ask to maintain for us in addition to the rest of Caddy at the moment.

[ririsoft]

Hello,

My first post here.
I have just switched from Apache to Caddy and would like to congratulate every one for the amazing job.

I am reaching to this issue with the following user story:

I use netdata to monitor my infrastructure. I have activated caddy's metrics for prometheus monitoring. I would like per host metrics. I understand this is being worked on through #6279 and related issues. But I also would like to run other monitoring tools on the access logs: fail2ban and netdata web_log analysis plugin. Those tools need read permissions to the access logs.

I understand that there is no bandwidth to maintain natefinch/lumberjack.
@pascalgn suggested a work around which is to create the file with the right permissions so that lumberjack reuse it.

What do you think about this workaround ? Would you be open to implement it ? It looks like a one-liner maybe ?

[mholt]

@ririsoft Welcome to the project!

I don't think we know the filename Lumberjack will use, unfortunately:

https://github.com/natefinch/lumberjack/blob/305d3e2979e73ec269fa12dab3d82e2f829c1c84/lumberjack.go#L331-L338

Maybe I am misreading (it's very early here)

[ririsoft]

I do not get your concern @mholt , sorry.
The filename is known : see https://github.com/caddyserver/caddy/blob/master/modules/logging/filewriter.go#L102.

As @pascalgn mentioned above Lumberjack reuses the same permissions if the file already exists: https://github.com/natefinch/lumberjack/blob/305d3e2979e73ec269fa12dab3d82e2f829c1c84/lumberjack.go#L304

In such case we could have Caddy create the log file first with the right permissions before calling Lumberjack.
Something like:

// OpenWriter opens a new file writer.
func (fw FileWriter) OpenWriter() (io.WriteCloser, error) {
	// roll log files by default
	if fw.Roll == nil || *fw.Roll {
		if fw.RollSizeMB == 0 {
			fw.RollSizeMB = 100
		}
		if fw.RollCompress == nil {
			compress := true
			fw.RollCompress = &compress
		}
		if fw.RollKeep == 0 {
			fw.RollKeep = 10
		}
		if fw.RollKeepDays == 0 {
			fw.RollKeepDays = 90
		}

               // create the log file with read permissions at group level
               f_tmp := os.OpenFile(fw.Filename, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0640)
               f_tmp.Close()

		return &lumberjack.Logger{
			Filename:   fw.Filename,
			MaxSize:    fw.RollSizeMB,
			MaxAge:     fw.RollKeepDays,
			MaxBackups: fw.RollKeep,
			LocalTime:  fw.RollLocalTime,
			Compress:   *fw.RollCompress,
		}, nil
	}

	// otherwise just open a regular file
	return os.OpenFile(fw.Filename, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0o666)
}

Please excuse my Go, I am a rust developer and it has been a long time since I have practiced the language ;-)

That would be a first step and easy workaround, allowing administrators to add tools to the "caddy" group if they want to grant log read access. And of course this need to be tested, because we are just assuming this should work reading Lumberjack code.

This would allow most users to drop logrotate with "copytruncate" option which has a race condition that may lose some few logs.

A second step would be to make the permissions (and even group) configurable, but that is definitely more work.

What do you think ?

[mholt]

@ririsoft Could you test to make sure that works (including through rotation)? If so we could accept a patch along those lines I think.

[ririsoft]

Sure, I will try to test that when time permit

[ririsoft]

Ok so I just tested it (it was so easy to onboard with caddy development I could not resist) and it works as expected. I will submit a PR soon.

[mholt]

Awesome! Thanks