Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attributes retrieve are case sensitive #129

Closed
memorais opened this issue Aug 31, 2017 · 20 comments
Closed

Attributes retrieve are case sensitive #129

memorais opened this issue Aug 31, 2017 · 20 comments

Comments

@memorais
Copy link

If we use GetAttributeValue function in a search to retrieve an specific attribute, the attribute name is currently case sensitive in this API.

According to LDAP specifications attribute retrieval must be case insensitive. For exemple, the attribute givenName and givenname are the same attribute, therefore if a client ask for "givenname" and LDAP definitions is set to "givenName", the API must be case insensitive returning the attribute and its value to the client, if present of course.

@liggitt
Copy link
Contributor

liggitt commented Aug 31, 2017

According to LDAP specifications attribute retrieval must be case insensitive.

Can you provide a reference for that?

@memorais
Copy link
Author

https://tools.ietf.org/html/rfc4511#section-4.5.1.8
"LDAPString values of this field are constrained to the following Augmented Backus-Naur Form (ABNF) [RFC4234]: ..."

https://tools.ietf.org/html/rfc4234#section-2.1
"Rule names are case-insensitive"

This behavior could be found in other wide used ldap clients like "ldapsearch" from OpenLDAP for example.

@johnweldon
Copy link
Member

The relevant link is https://tools.ietf.org/html/rfc4512#section-2.5 I think.

@johnweldon
Copy link
Member

johnweldon commented Aug 31, 2017

Actually, none of these links seem conclusive. When using the attribute name we're using an alias for the OID (which doesn't have case), and rfc 4512 doesn't seem to clarify whether matching on the alias should be case sensitive. It's clear that the attribute name (alias) itself is just a utf-8 string.

@memorais
Copy link
Author

IMHO it does not make sense not returning attributes based on their sensitiveness. If searches like '(givenname=*)' will work on server, no matter if LDAP definition is 'givenName' or 'givenname', this should work at the same manner for returned attributes.

@liggitt
Copy link
Contributor

liggitt commented Aug 31, 2017

does that mean the server can return multiple attributes?

Givenname=foo
givenName=bar
GIVENNAME=baz

what should the behavior of getAttribute("givenname") be in that circumstance?

@memorais
Copy link
Author

The server will return based on the schema definition, in this case it will return:

givenName=foo
givenName=bar
givenName=baz

Since this attribute is multi-valued, getAttribute("givenname") should be an array of all 3 givenName values.

@liggitt
Copy link
Contributor

liggitt commented Aug 31, 2017

can a schema define multiple attributes that differ only by case?

if it is actually impossible to get a response like #129 (comment) from a server, I'm not necessarily opposed to something like #111, but I'm very wary of compressing a value space when it has been unclear to me whether the case insensitivity applies to specific uses of attributes (like DN formation and the search operation) or globally

@memorais
Copy link
Author

No, it can't.

From LDAP point of view it applies globally. What we should take care is the value of attributes because of matching rules. Some attributes may have matching rules defined to be case sensitive. For example, java.schema from OpenLDAP have the attribute 'javaClassName' which have an equality rule 'caseExactMatch'. This have implications on how clients handle searches and values retrieval.

@vetinari
Copy link
Contributor

@liggitt can we then merge #111? With the merge of master which fixes the race condition it passes... and what about #54 (sorry for the hijack ;-))

@johnweldon
Copy link
Member

I still haven't found information in any spec to support this statement:

According to LDAP specifications attribute retrieval must be case insensitive.

However, going with the principle of least surprise, if we can document at least a few mainstream tools or libraries that default to case-insensitive attribute matching and searching, then I'd be open to having an exported package variable to toggle this feature on or off, and we can even default to having it enabled, as long as consumers can revert to case-sensitive behaviour as needed.

@liggitt
Copy link
Contributor

liggitt commented Sep 28, 2017

a parallel set of case insensitive functions could make sense as well

@memorais
Copy link
Author

@johnweldon And you found any information in spec which states that attribute names should be case sensitive?

One of the most used clients (ldapsearch from OpenLDAP as I said before) don't rely on case sensitiveness of the attribute names. In fact I never used an LDAP client which behaves like this API in this particular subject. You can also check tools like Apache Directory Studio [1] which is also widely used.

[1] - http://directory.apache.org/studio/

@liggitt
Copy link
Contributor

liggitt commented Sep 29, 2017

And you found any information in spec which states that attribute names should be case sensitive?

For a low-level library, preserving fidelity of returned information is extremely important. Since it is possible for an LDAP server to return a set of attributes like the following, this library should not silently compress those differences:

Givenname=foo
givenName=bar
GIVENNAME=baz

I'd be amenable to adding GetAttributeValueIgnoreCase, etc, with clear documentation of behavior given an example like that

@vetinari
Copy link
Contributor

vetinari commented Oct 2, 2017

@johnweldon, @liggitt IMO RFC 4512 / Section 2.5 does explicitly state that attribute names are case insensitive:

An attribute description is represented by the ABNF:

 attributedescription = attributetype options
 attributetype = oid
 options = *( SEMI option )
 option = 1*keychar

where <attributetype> identifies the attribute type and each <option>
identifies an attribute option. Both <attributetype> and <option>
productions are case insensitive.

The oid part is defined at end of section 1.4 (Common ABNF Productions):

Where either an object identifier or a short name may be specified,
the following production is used:

 oid = descr / numericoid

@johnweldon
Copy link
Member

@vetinari I think you're right; some quotes from 1.4 of RFC 4512: (emphasis mine)

Short names, also known as descriptors, are used as more readable
aliases for object identifiers. Short names are case insensitive and
conform to the ABNF:

  descr = keystring

Where either an object identifier or a short name may be specified,
the following production is used:

  oid = descr / numericoid

@memorais
Copy link
Author

memorais commented Oct 3, 2017

Right, this should be the default behavior of the API.

@johnweldon
Copy link
Member

I'm inclined to still approach this as @liggitt suggests - with parallel functions that are case insensitive. We shouldn't be changing the behaviour of the established API IMO.

@vetinari
Copy link
Contributor

I've added now GetEqualFoldAttribute* in #111, but with switching to v3 we could use those as defaults and the old ones as e.g. GetAttributeValueCaseSensitive.

@liggitt
Copy link
Contributor

liggitt commented May 28, 2018

with switching to v3 we could use those as defaults and the old ones as e.g. GetAttributeValueCaseSensitive.

I'd prefer to keep any value transformation done by this library explicit. If the comparison is ignoring case, the method name should make that clear

vetinari added a commit to vetinari/ldap that referenced this issue Jun 7, 2018
these are the case insensitive counter parts, e.g.
  GetAttributeValue <=> GetEqualFoldAttributeValue
using strings.EqualFold to compare the attrribute names

fixes go-ldap#109 go-ldap#129
johnweldon added a commit that referenced this issue Jun 11, 2020
these are the case insensitive counter parts, e.g.
  GetAttributeValue <=> GetEqualFoldAttributeValue
using strings.EqualFold to compare the attrribute names

fixes #109 #129

Co-authored-by: John Weldon <johnweldon4@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants