OpenIdentityPlatform / OpenDJ

OpenDJ is an LDAPv3 compliant directory service, which has been developed for the Java platform, providing a high performance, highly available, and secure store for the identities managed by your organization. Its easy installation process, combined with the power of the Java platform makes OpenDJ the simplest, fastest directory to deploy and manage.
https://www.openidentityplatform.org/opendj
Other
369 stars 103 forks source link

Schema parser for DITStructureRules violates RFC 4512 Section 4.1.7.1 #393

Closed JesseCoretta closed 1 month ago

JesseCoretta commented 1 month ago

Describe the bug

Consider the following ABNF production, per RFC 4512 Section 4.1.7.1 ("DIT StructureRules") with regards to the "superior rules" element:

     DITStructureRuleDescription = LPAREN WSP
         ruleid                     ; rule identifier
         [ SP "NAME" SP qdescrs ]   ; short names (descriptors)
         [ SP "DESC" SP qdstring ]  ; description
         [ SP "OBSOLETE" ]          ; not active
         SP "FORM" SP oid           ; NameForm
         [ SP "SUP" ruleids ]       ; superior rules
         extensions WSP RPAREN      ; extensions

Unlike other definition types which utilize the "oidlist" ABNF production with an ASCII char 44 $ delimiter (and optional WSP padding) ...

      oids = oid / ( LPAREN WSP oidlist WSP RPAREN )
      oidlist = oid *( WSP DOLLAR WSP oid )

... DIT Structure Rules are different in a subtle way when dealing with multiple SUP(erior) rules. Specifically:

     ruleids = ruleid / ( LPAREN WSP ruleidlist WSP RPAREN )
     ruleidlist = ruleid *( SP ruleid )
     ruleid = number

Here, WSP is only used as optional parenthetical encapsulation padding, however the delimiter is not $, but is in fact (ASCII char 32 token "SP").

Now, I'd be fine with this if it were "OpenDJ syntactical schema sugar" in that both ( 1 2 3 ) and ( 1 $ 2 $ 3 ) would be accepted, and considered identical. Unfortunately that is not the case. While the technically-incorrect form ( 1 $ 2 $ 3 ) does accomplish what is expected of it, the correct form causes the following to happen upon launch of OpenDJ:

msg=The config schema file '99-user.ldif' generated warning when trying to
update schema with its content: [Validation of DIT structure rule definition 
( 161 NAME 'subentryStructure' FORM subentryForm SUP ( 150 51 ) ) failed
and will be removed from the schema: The provided value "subentryStructure"
could not be parsed as a DIT structure rule description because it referenced
an unknown rule ID 51 for a superior DIT structure rule]

Wait ... 51?

At first I just assumed I made a typo. But when I went into the schema file, I found just the opposite:

dITStructureRules: ( 161
          NAME 'subentryStructure'
          FORM subentryForm
          SUP ( 150 151 ) )

This is exactly how I wrote it .... ?!?

Now, this is kind of a funny situation. I actually am the maintainer of a general purpose Go-based Directory Schema SDK. Among its many features is a pretty strict interpretation of RFC 4512 Section 4.

Back during the early days when I was first designing this app, I made the same mistake I believe I have exposed now, in that I wrongly used $ for the DIT Structure Rule SUP delimiter in my parser. To prove my theory, (wrongly) changing the above to:

dITStructureRules: ( 161
          NAME 'subentryStructure'
          FORM subentryForm
          SUP ( 150 $ 151 ) )

... "solves" the issue.

I'm going to assume that the best solution is to allow both conventions, so as to be in compliance with the aforementioned standards but also to avoid introducing breaking changes for some customers.

Thank you! 😃

Jesse Coretta

PS - Still somewhat on-topic, please note this errata I filed a while back for RFC 4512. Although not yet approved, I thought it might be relevant if you're going to be looking at this exact part of your parser anyway. I'm 99.999999% certain my assertion is correct in that errata. Just figured you'd want to know. 😈

vharseko commented 1 month ago

Please check PR https://github.com/OpenIdentityPlatform/OpenDJ/pull/395 distribution can be downloaded from https://github.com/OpenIdentityPlatform/OpenDJ/actions/runs/10793608300

JesseCoretta commented 1 month ago

@vharseko -- works as expected! Thank you 😃

Jesse

JesseCoretta commented 1 month ago

BTW, I am honored that you used my example(999) allocation in your tests; it is officially registered exactly for this reason. Enjoy 😃