Probably worth a comment here to explain what this line does, it took me a second to parse it to realise it was a bit mask check :)

Besides that comment, looks fine to me :) ack

If I am correct, currently dscreate sets perms to 600 (same of pwdfile)
On deployments without freeipa it will remain 600. I expects checking against 400 will report an error, correct ?

Probably worth a comment here to explain what this line does, it took me a second to parse it to realise it was a bit mask check :)

Maybe this would make it clearer as well (untested):

diff --git a/src/lib389/lib389/dseldif.py b/src/lib389/lib389/dseldif.py
index f5a44c417..1077f3b3a 100644
--- a/src/lib389/lib389/dseldif.py
+++ b/src/lib389/lib389/dseldif.py
@@ -346,10 +346,9 @@ class FSChecks(object):
     def _lint_file_perms(self):
         # Check file permissions are correct
         for ds_file in self.ds_files:
-            perms = str(oct(os.stat(ds_file[0])[ST_MODE])[-3:])
-            bad = False
-            for i in range(0, 3):
-                if int(perms[i]) != ds_file[1][i] and not int(perms[i]) & ds_file[1][i]:
+            perms = map(int, str(oct(os.stat(ds_file[0])[ST_MODE])[-3:]))
+            for p, e in zip(perms, ds_file[1]):
+                if p != e and not p & e:
                     # Bad permissions, report it...
                     report = copy.deepcopy(ds_file[2])
                     report['items'].append(ds_file[0])

If I am correct, currently dscreate sets perms to 600 (same of pwdfile)
On deployments without freeipa it will remain 600. I expects checking against 400 will report an error, correct ?

In a IPA deployment the pin.txt file was set to 400, which is valid. I use a bitmask check to see if 4 is set. As long as the "user" permission contains READ privilege (4) we don't complain, so these permissions are all valid for the pin.txt file: 700, 600, 500, 400

Applied @mhonek code suggestion (with a small tweak - thanks!). This should also clarify the how the bitmask is being used (so I did not add a comment about it @firstyear).

Please review...

This PR is flawed, I am reworking the entire patch (including tests). I'm going to close this PR....

This PR is flawed, I am reworking the entire patch (including tests). I'm going to close this PR....

Curious how it was flawed?

This PR is flawed, I am reworking the entire patch (including tests). I'm going to close this PR....

Curious how it was flawed?

The bit mask approach didn't work as I had hoped. So for resolve.conf we expect 644, but with the bit mask check 544 was seen as okay. So I need to hard-code the allowed/expected permissions :-/ Not as elegant as I had hoped, but perfectly sufficient...

Oh, I was suspicious about that... but wouldn't doing 0 != (~p & e) instead of not (p & e) just do it?

Oh, I was suspicious about that... but wouldn't doing 0 != (~p & e) instead of not (p & e) just do it?

It still sees it as a match, but it's not what we want to test:

(~6 & 5)
1
(~5 & 6)
2

This just isn't the correct way to do the checks we need to make. For some files we don't want the permission too open, in other files we need to the permission to be more open. Every file has special conditions, so a global approach like this is not correct.

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3861

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.