https://bugs.openldap.org/show_bug.cgi?id=9256

--- Comment #2 from Karl O. Pinc kop@karlpinc.com --- (In reply to Howard Chu from comment #1)

(In reply to Karl O. Pinc from comment #0)

...

E.g. knowing that (objectClass=*) is the default filter, and that there's _always_ _some_ filter,

This is fundamental to LDAP. Everyone administering slapd should already know this.

That's as may be, but someone doing their first installation may not have it in their mind or be immediately aware of all the implications. It is easy to forget; ldapsearch does not require a filter be specified.

Regardless, the authorization required for SASL binding is seemingly unrelated to that required for simple binding. Simple binding does not require authorization to the entry pseudo-attribute or the objectClass attribute, even though some sort of search/lookup must be done internally. Anyone trying to configure authorization for SASL binding based on their experience with simple binding will be mislead, even if only doing direct DN mapping.

Being explicit about SASL authorization requirements goes a long way toward reducing the effort involved in setting up SASL.

https://bugs.openldap.org/show_bug.cgi?id=9256

--- Comment #4 from Karl O. Pinc kop@karlpinc.com --- Hi Ondřej,

Thanks for looking over my suggestions.

"depending on the SASL mechanism in use." why not say something like "if authz-regexp remapping is in place".

I really like this idea. Please see the text of the new patch. (I was able to re-write the sentence and eliminate many words.)

Maybe keep the slapd.conf->cn=config changes to a separate commit.

It didn't seem worth having a separate commit which changes only a single word. (Or two words, as "directive" becomes "attribute".) My thought was that a baby-step forward was better than none. On your recommendation I've reverted to using the the slapd.conf terminology. After settling on on some final text with you, I can add a separate commit, if you would like, to move to the cn=config terminology.

In the paragraph "Some internal operations..." not sure such sweeping changes are really needed,

<snip>

I have a lot of thoughts on the specifics of the changes I propose, and would be happy to go through the details if you want to hear. A lot of the details depend on exactly what it is that you (openldap) want to say. I hope that if you want to say something different than what I think you want to say then that will become clear as we write back and forth and I can make changes.

Until then, I've 2 responses, to go with the 2 parts of your comment:

The paragraph is about access requirements for authentication and authorization. As such, it should start with a topic sentence that says so. That right there (seemingly) requires that the existing first and last sentences, 2 out of the 4 in the paragraph, be moved around and re-worded. With that I'm well on my way to making sweeping changes.

<snip>

maybe just saying the default filter equals to objectclass=* if not specified would simplify and clarify that part?

The purpose of the section is to document what privileges on what entries/attributes are required for which operations. To be comprehensive, and concise, attributes (and so forth) must be paired with specific privileges, in the context of an operation. If you don't specify every attribute needing privilege paired with that attribute's required privilege you are leaving things out.

Adding rational and context and background and other things to build a narrative can make the documentation easier to read. But it does not make sense to mention that objectClass is searched on and make the reader reason through on their own what sort of access permissions are needed under what conditions. Requiring the reader to reach their own conclusions, when the correct answer will be the same for every single reader, has little purpose and makes life hard for the reader. At least that's how I see it. SASL is complicated enough as it is.

One of my goals was to keep things brief. If you think it helps to add background about the existence of the default search filter I can do that. It might help if you suggested an edit to work from, if you want to go forward with this.

Another goal was to try to retain the narrative style and as much of the original text as possible. If your concern is that my rewrite adds additional verbiage and is longer than the original paragraph you might, instead, like the following more aggressive edit:

-------<snip>------- Authentication which uses authzID mapping (typically needed for SASL) and authorization using the proxyAuthz control require auth (=x) privileges on: all the attributes present in the search filter of the URI regexp maps (the right-hand side of the authz-regexp directive), or on the objectClass attribute when either direct DN mapping or when the URI contains no explicit search filter; on the entry pseudo-attribute of all the database entries searched; and on the authzTo attribute of the authorizing identity and/or on the authzFrom attribute of the authorized identity. -------<snip>-------

This is shorter than the original and, I believe, completely documents what attributes require what permissions under what conditions.

I kind of like the short version. But I also like the explanation in the original about how the idea is to relax, what would otherwise be more restrictive permissions, to "auth" when authenticating. That's the kind of thing that a narrative helps with. There's an "ah-ha" moment for the reader to give them a bit of satisfaction and an expectation that since they understand this point they will be able to understand the boring more complicated stuff that's coming later in the paragraph.

If you want the short version I'll have to upload another patch; I've left the long version in the patch I'm uploading with this message.

Thanks again.

https://bugs.openldap.org/show_bug.cgi?id=9256

Karl O. Pinc kop@karlpinc.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Attachment #728 is|0 |1 obsolete| |

--- Comment #6 from Karl O. Pinc kop@karlpinc.com --- Created attachment 729 --> https://bugs.openldap.org/attachment.cgi?id=729&action=edit Version 3, using non-narrative, "minimal" text

Hi Ondřej,

It occurs to me that my patch has left out explicit documentation of the permissions required when authzFrom/authzTo contains a URI.

Submitting a new patch which fixes this. I believe the documentation is now explicit and complete.

I have also entirely removed the "look down there for SASL binding" sentence in the bind paragraph. This was possible because I moved the authz binding paragraph to just after the first bind paragraph to put all the bind related information together. This has the additional benefit of putting the paragraph on front-end search-related behavior next to the paragraph on back-end search.

I did add words, to be explicit, and say that binding requires "anonymous" to have the documented auth privileges, even though "anonymous" is explained above. This added only 4 words. Four words seems a small price to pay for having everything you need to know about binding in one place -- and the end result is still a smaller paragraph than that in the current docs.

This last patch is based on the the "minimal text" version I put forward in the comment in response to the initial patch review. If you want a longer, more narrative, text or if you think something important has been omitted or no longer being said please let me know what else you'd like included.

Thanks for the help.

https://bugs.openldap.org/show_bug.cgi?id=9256

--- Comment #8 from Ondřej Kuzník ondra@mistotebe.net --- Hi Karl, thanks for continuing to work on this. I've had a look at your latest patch. It looks better, now we need to make sure we nail down the explanation in all cases mentioned.

Running a quick check with authorization (idassert etc.) it seems a bit more complicated than described. Access looks to be checked with the credentials of the authenticated account, not anonymous. Have a look at the code or slapd (level acl) logs in scenarios like test014/028 to see what actually happens.

Thanks,

https://bugs.openldap.org/show_bug.cgi?id=9256

Quanah Gibson-Mount quanah@openldap.org changed:

What |Removed |Added ---------------------------------------------------------------------------- Assignee|bugs@openldap.org |ondra@mistotebe.net

https://bugs.openldap.org/show_bug.cgi?id=9256

--- Comment #11 from Karl O. Pinc kop@karlpinc.com --- On Thu, 18 Feb 2021 14:16:29 +0000 openldap-its@openldap.org wrote:

https://bugs.openldap.org/show_bug.cgi?id=9256

--- Comment #10 from Ondřej Kuzník ondra@mistotebe.net --- On Tue, May 19, 2020 at 02:31:12PM +0000, openldap-its@openldap.org wrote:

...

...

thanks for continuing to work on this. I've had a look at your latest patch. It looks better, now we need to make sure we nail down the explanation in all cases mentioned.

Running a quick check with authorization (idassert etc.) it seems a bit more complicated than described. Access looks to be checked with the credentials of the authenticated account, not anonymous. Have a look at the code or slapd (level acl) logs in scenarios like test014/028 to see what actually happens.

I'll take a look when I get a chance. Thanks for the help.

Hi Karl, just checking if you had an updated version of the docs?

Hi,

No. I'll see if I can look at what you bring up today or tomorrow. I did test everything I documented. Of course I could have made a mistake.

I'm thinking of putting a patch together to add a regression test to demonstrate. I'm hoping that will either satisfy you or prove me wrong. (I would not be submitting such a patch for inclusion, doing that would be up to you.)

If you think this is the wrong approach please let me know.

Regards,

Karl kop@karlpinc.com Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein

https://bugs.openldap.org/show_bug.cgi?id=9256

Quanah Gibson-Mount quanah@openldap.org changed:

What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |2.5.3

https://bugs.openldap.org/show_bug.cgi?id=9256

--- Comment #14 from Karl O. Pinc kop@karlpinc.com --- Hello,

The attached patch contains a test script which, I believe, validates the (newly but not yet fully documented in the patch) access rules which are required to bind.

However, I believe I have uncovered another bug. It appears that, even when every access rule applies _only_ to a single DIT entry, one attribute of that entry, and grants access to _only_ a single DN, the access rules are sensitive to ordering. Some access rules are required, given a particular ordering of the access rules, that are not required when the rules are re-ordered.

Search for "ORDERING BUG" in the test script for an example. (The code there is run twice, once with an authz-regexp with a URI and again with an authz-regexp that does direct DN mapping with a regexp.)

The supplied test case does not test every permutation of the given access rules, which leaves open the possibility that it is not discovering all the access rules that are required; and the supplied documentation patch does not mention any ordering requirements for access rules and so is likely incomplete.

I don't believe that access rules should be order dependent unless, obviously, there exist some rules that apply to the same portion of the DIT and also grant access to more than one authenticating entity.

If you (openldap) agree with the assessments above I leave it to you to continue to develop the test case. Say, by having the script test all permutations of rule ordering.

Further, if there is indeed a bug to fix, code alterations could well affect the permissions required to bind. For these reasons, and because I've spent far too much time on this to-date, I would like to back-away from further development of this patch. I am willing to continue to assist with the wording of the documentation when you have determined that the time is right.

There remains work on the documentation portion of the patch. I don't see a lot of point in proceeding until it is known if the test script is comprehensive.

I have not fully annotated all the access rules in the test script, referencing them back to the portion of the documentation that says they are required. These annotations should shed light on any further changes, as below, that the documentation needs.

I believe that text must be added to the documentation which says that access to objectClass is not required (for sasl binds) in the case of proxy authorization.

I believe that text must be added to the documentation which says that sasl binds need =x by the authorizing identity on "entry" of the search base of authzFrom URIs (but not when dn mapping). (In addition to =x access by "anonymous", which is documented.)

I believe that text must be added to the documentation which says that sasl binds need =x by the authorizing identity on the authFrom attribute of the authorized identity when authFrom contains a URI (but not when dn mapping). (In addition to =x access by "anonymous", which is documented.)

I believe that text must be added to the documentation which says that sasl binds via an authorizing group need anonymous to have access to "entry" of the authorizing entity. (Attention needs to be paid here to the relationship between this requirement and the authz-regexp.)

I believe that text must be added to the documentation which says that simple binding with dn.onelevel does not need "cn" access by authorizing entity. But my notes don't say which of the dn.onelevel tests this applies to.

Regards,

Karl kop@karlpinc.com Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein

P.S. Work on the test case is what uncovered bug# 9495. I wound up using a ":" character in the authzid to work-around the problem, and made corresponding adjustments to the authz-regexp(s). When 9495 is fixed you may want to fiddle with the test script and do "something different"; or not.

https://bugs.openldap.org/show_bug.cgi?id=9256

--- Comment #16 from Karl O. Pinc kop@karlpinc.com --- Hi Ondřej,

On Wed, 28 Apr 2021 10:16:08 +0000 openldap-its@openldap.org wrote:

https://bugs.openldap.org/show_bug.cgi?id=9256

--- Comment #15 from Ondřej Kuzník ondra@mistotebe.net --- On Thu, Apr 08, 2021 at 08:12:18PM +0000, openldap-its@openldap.org wrote:

Thanks for the reply.

...

The attached patch contains a test script which, I believe, validates the (newly but not yet fully documented in the patch) access rules which are required to bind.

thanks for the patch. I don't understand many of the assertions below and don't understand what the test script is supposed to test, if you're saying it uncovers bugs, does it mean if fails for you? I doesn't fail for me even if I uncomment/enable parts that look like they're (temporarily) disabled.

The script succeeds, as written.

I believe it does uncover a bug. Or at least can be made to do so.

What it tests is for the exact access requirements needed to perform each "kind" of bind. It does this by proving that a particular set of access rights is both minimal and necessary.

It does this by using only access rules the are maximally specific. Each access rule grants only one permission to one authenticating DN on one attribute attribute of one entry. When access rules are written this way, removing a single rule will prevent the authenticating DN from binding.

Each bind test has a set of these access rules. The script checks that the bind succeeds with the rules, proving that the access rules permit binding. Then it checks that the bind fails when any one of the access rules is removed, proving that each permission is required.

This method is the only way I can see to demonstrate what ACLs are required to bind, and so prove that the documentation is correct and that the requirements do not change from release to release.

As an FYI unrelated to the topic at hand, the script loops through the "set of binds" twice. First with an olcAuthzRegexp that does ldap searching, with a replace name beginning with "ldap:", and again using direct dn mapping, with a replace name beginning with "dn:". The access rules vary between these two cases so there are functions called to create the rules. The idea was to show what's common. The functions do obscure what's different, so I would up printing the set of access rules in order to have something to look at.

...

However, I believe I have uncovered another bug. It appears that, even when every access rule applies _only_ to a single DIT entry, one attribute of that entry, and grants access to _only_ a single DN, the access rules are sensitive to ordering. Some access rules are required, given a particular ordering of the access rules, that are not required when the rules are re-ordered.

ACLs are order-sensitive, could you give an example?

The below two ALCs should not be order sensitive, because they are "maximally specific" as defined above. (Unless, of course, there are other interspersed ACLS that are not maximally specific, do not end with "by * break", etc.)

olcAccess: to dn.exact="cn=Barbara Jensen,ou=Information Technology Division,ou=People,dc=exam ple,dc=com" attrs=userpassword by anonymous =x by * break

olcAccess: to dn.exact="cn=Barbara Jensen,ou=Information Technology Division,ou=People,dc=exam ple,dc=com" attrs=entry by anonymous =x by * break

...

Search for "ORDERING BUG" in the test script for an example. (The code there is run twice, once with an authz-regexp with a URI and again with an authz-regexp that does direct DN mapping with a regexp.)

Again, this doesn't fail for me and nothing there suggests what the alternative set up should be to make it expose a bug.

But this series of tests is not written to expose that bug, it's written to affirm that the documented minimal ACLs necessary to bind are correct. (See below for more.)

...

The supplied test case does not test every permutation of the given access rules, which leaves open the possibility that it is not discovering all the access rules that are required; and the supplied documentation patch does not mention any ordering requirements for access rules and so is likely incomplete.

I don't believe that access rules should be order dependent unless, obviously, there exist some rules that apply to the same portion of the DIT and also grant access to more than one authenticating entity.

Could you give an explicit example?

This is what "ORDERING BUG" uncovers, if you do as the comment says. The command is:

../clients/tools/ldapwhoami -Q -H ldap://localhost:9011/ -Y DIGEST-MD5 -U "Barbara Jensen:Information Technology Division" -w bjensen

The mapping is:

olcAuthzRegexp: "^uid=([^:]+):([^,]+),.*" "dn:cn=$1,ou=$2,ou=people,dc=example,dc=com"

The following access rules are proved, as the script is written which delivers the rules in the order given below, to be the minimal needed to bind:

olcAccess: to dn.exact="cn=Barbara Jensen,ou=Information Technology Division,ou=People,dc=example,dc=com" attrs=userpassword by anonymous =x by * break

olcAccess: to dn.exact="cn=Barbara Jensen,ou=Information Technology Division,ou=People,dc=example,dc=com" attrs=entry by anonymous =x by * break

olcAccess: to dn.exact="cn=Barbara Jensen,ou=Information Technology Division,ou=People,dc=example,dc=com" attrs=objectClass by anonymous =x by * break

olcAccess: to dn.exact="dc=example,dc=com" attrs=entry by anonymous =x by * break

olcAccess: to * by * none stop

However, if you follow the instructions in the "ORDERING BUG" section and re-order the access rules to the following:

olcAccess: to dn.exact="cn=Barbara Jensen,ou=Information Technology Division,ou=People,dc=example,dc=com" attrs=userpassword by anonymous =x by * break

olcAccess: to dn.exact="cn=Barbara Jensen,ou=Information Technology Division,ou=People,dc=example,dc=com" attrs=entry by anonymous =x by * break

olcAccess: to dn.exact="dc=example,dc=com" attrs=entry by anonymous =x by * break

olcAccess: to dn.exact="cn=Barbara Jensen,ou=Information Technology Division,ou=People,dc=example,dc=com" attrs=objectClass by anonymous =x by * break

olcAccess: to * by * none stop

Then what you find is that the rule:

olcAccess: to dn.exact="dc=example,dc=com" attrs=entry by anonymous =x by * break

is unnecessary. The bind can be performed without it and so the script fails.

At this point I gave up. Anything, especially previous binds if there's some sort of caching involved, could be causing this and I didn't try to come up with a minimal example. I can tell you that you can comment out line #2467, which removes all the mappings to "ldap:" urls and leaves only the olcAuthzRegexp with the "dn:" mappings, and the problem still exhibits.

My problem is that if you can't test for minimal access rights required to bind by removing ACLs until you get a minimal set, then there's no way to know or document what the minimal access rights are.

In theory, the test script could do more than remove access rights to come up with a minimal set. It could also re-order all the access rights and test all permutations to prove that when all access rights are "maximally specific", ACLs are not sensitive to order. (As I believe should be the case.) This is probably prudent, to prove that the bug I believe I've discovered is fixed. At least with respect to binding. But all of this is out-of-scope with respect to documenting access rights, which is what I want to do.

...

P.S. Work on the test case is what uncovered bug# 9495. I wound up using a ":" character in the authzid to work-around the problem, and made corresponding adjustments to the authz-regexp(s). When 9495 is fixed you may want to fiddle with the test script and do "something different"; or not.

If you consider the authzid provided by the client has to go through normalisation as per RFC4513 Section 5.2.1.8, would you comment on ITS#9495 whether/or to what extent it might explain your findings?

I mention ITS#9495 here because this script discovered the problem. I don't believe that it really has to do with this script, other than when ITS#9495 is fixed this script might be changed so that the mappings happen in a more natural way. ITS#9495 explains why this script's mappings are "different".

Regards,

Karl kop@karlpinc.com Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein

https://bugs.openldap.org/show_bug.cgi?id=9256

Quanah Gibson-Mount quanah@openldap.org changed:

What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|2.5.5 |2.5.6

https://bugs.openldap.org/show_bug.cgi?id=9256

Quanah Gibson-Mount quanah@openldap.org changed:

What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|2.5.7 |2.6.0

https://bugs.openldap.org/show_bug.cgi?id=9256

--- Comment #18 from Ondřej Kuzník ondra@mistotebe.net --- On Tue, Aug 31, 2021 at 10:32:52AM +0000, openldap-its@openldap.org wrote:

In the patch, the line: +attribute of the authorizing identity and/or on the ends with a space.

Moreover, https://www.openldap.org/doc/admin25/access-control.html#Basic%20ACLs states:

Thanks for taking time to review this, would you be also adapt the proposed patch with your suggestions and submit a MR?

Generally one should start with some basic ACLs such as:

access to attrs=userPassword
    by self =xw
    by anonymous auth
    by * none


  access to *
    by self write
    by users read
    by * none

Per https://bugs.openldap.org/show_bug.cgi?id=9657, for SIMPLE bind, anonymous needs auth access only to the userPassword attribute, but for SASL bind, anonymous needs access to the whole entry.

I propose removing "by * none", as it is implicit.

I my view, documentation should be explicit about defaults like this and suggest their use where appropriate.

I propose extending the patch, to state for this particular example, that the example is suitable for SIMPLE bind, and unsuitable for SASL bind. (well “access to attrs=userPassword by self =xz” shall still be preserved). Provide example that works with SASL bind, e.g.

access to attrs=userPassword
    by self =xw

  access to *
    by anonymous auth
    by self write
    by users read

(without by * none, since it is explicit).

Thanks,

https://bugs.openldap.org/show_bug.cgi?id=9256

Quanah Gibson-Mount quanah@openldap.org changed:

What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|2.6.1 |2.6.2