Set permission for network logging files and dirs

[joschrew]

All logfiles and folder used by ocrd-network are created with 777 (dirs) and 666 (files) so that logging should be possible regardlessly of permission.

This is a suggestion for fixing OCR-D/ocrd_all#426. I'm not sure if we want this but imo this PR is a possible solution.

[MehmedGIT]

It looks good, great job!

[stweil]

Files and directories which are writable by everybody? I don't think that is a good idea.

[kba]

Files and directories which are writable by everybody? I don't think that is a good idea.

Are permissions really relevant for the use case security-wise?

How about 774 and 664, so user-group writeable/enterable?

[stweil]

The file and directory permissions are okay now, thank you.

I did not review the rest of the code.

[joschrew]

I don't think 774 and 664 solve the problem. Idea is the following: The log-files are created by root, e.g. when in the docker-image creation ocrd-resources are downloaded or any other ocrd command is executed which involves logging. Later the image is used by another user with different permissions. Then ocrd is used within this container, the logfiles exist and the ocrd-process with the user's permissions tries to write to them which will in this case not work with 664 for example.

The permissions are only set for the logfiles and its directories which are created by ocrd and only if they do not exist previously. As far as I see it, in the worst case the logfiles could be lost. But maybe I there are other problem with the permissions I am not aware of? What are the possible problems with these permissions?

[MehmedGIT]

The permissions are only set for the logfiles and its directories which are created by ocrd and only if they do not exist previously. As far as I see it, in the worst case the logfiles could be lost. But maybe I there are other problem with the permissions I am not aware of? What are the possible problems with these permissions?

That is why I did not see any issues with the previous permission settings although they are considered problematic in a general sense.

@stweil, do you maybe have a better idea than going back to 777 and 666?

[kba]

I really don't see the problem with 777/666, so I'll revert and merge, since this fixes a preventable issue and I don't see the security problem in this case.

[stweil]

Would it be possible to have different permissions for Docker and non-Docker installations?

[kba]

We could make this configurable via an ocrd_utils.config variable, e.g. OCRD_WORLD_WRITEABLE_LOGS?

[stweil]

This might be a solution. Ideally the right configuration should be used for different use cases without the need for manual user settings.

[kba]

Ideally the right configuration should be used for different use cases without the need for manual user settings

@joschrew any idea how we might best determine this automatically?

[joschrew]

The problem is this: When the images are created the logfiles might be created as well. For example when a processor's image extends core and then calls ocrd core for loading resources. Then the logfiles are created. Then a user creates a container of this processor and the logs are root-owned which cause problems when the user of the container is not root. So I don't think a variable can really help here.
This problem is avoidable though, with setting a separate logging-conf or with pre deleting the logfiles in the image before starting the container.

This PR is like a quick fix for this problem. But other fixes are possible as well. So if we don't want this world permission logfiles it is not that big problem.

Therefore my suggestion is to close and not merge this PR and go with something like OCR-D/ocrd_all#429 and do the same in the processor-images which also have those logfiles created. We have to touch the images anyway when setting the core version.

Sry for being late with this suggestion, I forgot to mention this issue in our last meetings.

[kba]

OK, then let's not include this PR in the next release and focus on fixing the permission error during image build.