#51235 rfc822localpart schema update
Closed: wontfix 3 years ago by firstyear. Opened 3 years ago by firstyear.

Issue Description

Looking at https://tools.ietf.org/html/draft-zeilenga-ldap-cosine-02 which openldap implements our rfc822localpart schema isn't complete

openldap may ('commonName', 'surname', 'description', 'seeAlso', 'telephoneNumber', 'physicalDeliveryOfficeName', 'postalAddress', 'postalCode', 'postOfficeBox', 'streetAddress', 'facsimileTelephoneNumber', 'internationalISDNNumber', 'telephoneNumber', 'teletexTerminalIdentifier', 'telexNumber', 'preferredDeliveryMethod', 'destinationIndicator', 'registeredAddress', 'x121Address') 
389 may ('cn', 'sn')

it's very likely that this is a low use and not relevant schema, but my schema migration tool flags it as inconsistent, so it's low effort/risk for us to just update our copy to match since these are all may attributes.


Metadata Update from @firstyear:
- Custom field origin adjusted to None
- Custom field reviewstatus adjusted to None
- Issue marked as blocking: #50544

3 years ago

Actually, this looks like it's not correct. Our schema is consistent to openldap's. The issue appears to be at run time, the cn=schema shows:

objectClasses: ( 0.9.2342.19200300.100.4.14 NAME 'rFC822localPart' SUP domain
 STRUCTURAL MAY ( cn $ sn ) X-ORIGIN 'RFC 4524' )

However, our 05rfc4524.ldif shows:

objectClasses: ( 0.9.2342.19200300.100.4.14 NAME 'rFC822localPart'
  SUP domain STRUCTURAL
  MAY ( cn $ description $ destinationIndicator $
    facsimileTelephoneNumber $ internationaliSDNNumber $
    physicalDeliveryOfficeName $ postalAddress $ postalCode $
    postOfficeBox $ preferredDeliveryMethod $ registeredAddress $
    seeAlso $ sn $ street $ telephoneNumber $
    teletexTerminalIdentifier $ telexNumber $ x121Address )
  X-ORIGIN 'RFC 4524' )

the name and oid are unique:

grep -i -r -n -e '0.9.2342.19200300.100.4.14' .
./05rfc4524.ldif:270:objectClasses: ( 0.9.2342.19200300.100.4.14 NAME 'rFC822localPart'
grep -i -r -n -e 'rfc822localpart' .
./05rfc4524.ldif:270:objectClasses: ( 0.9.2342.19200300.100.4.14 NAME 'rFC822localPart'

@tbordaz do you have any hints or tips on how to debug schema parsing at startup? Or is this a case where I'll need to add more logging to help out?

Okay I think I understand what's occuring.

Looking at this in a debugging I can see during the parsing:

(lldb) print *pnew_oc
(objclass) $86 = {
  oc_name = 0x00006020000c46f0 "rFC822localPart"
  oc_desc = 0x0000000000000000
  oc_oid = 0x0000603000269dd0 "0.9.2342.19200300.100.4.14"
  oc_superior = 0x00006020000c4710 "domain"
  oc_kind = '\x01'
  oc_flags = '\x01'
  oc_required = 0x0000000000000000
  oc_allowed = 0x000060e000082fe0
  oc_orig_required = 0x0000000000000000
  oc_orig_allowed = 0x0000603000269da0
  oc_extensions = 0x0000603000269d70
  oc_next = 0x0000000000000000
}

(lldb) print pnew_oc->oc_allowed
(char **) $87 = 0x000060e000082fe0
(lldb) print pnew_oc->oc_allowed[0]
(char *) $88 = 0x00006020000c4730 "cn"
(lldb) print pnew_oc->oc_allowed[1]
(char *) $89 = 0x00006020000c4750 "description"
(lldb) print pnew_oc->oc_allowed[2]
(char *) $90 = 0x0000603000269e00 "destinationIndicator"
(lldb) print pnew_oc->oc_allowed[3]
(char *) $91 = 0x0000603000269e30 "facsimileTelephoneNumber"
....

(lldb) print pnew_oc->oc_orig_allowed[0]
(char *) $92 = 0x00006020000c4690 "cn"
(lldb) print pnew_oc->oc_orig_allowed[1]
(char *) $93 = 0x00006020000c46d0 "sn"
(lldb) print pnew_oc->oc_orig_allowed[2]
(char *) $94 = 0x0000000000000000

Now we can note that oc_orig_allowed is only two elements and oc_allowed is the full set. This is due to the following code in parse_objclass_str:

3742         if (psup_oc->oc_allowed && objClass->oc_at_oids_may) {
3743             for (i = 0; objClass->oc_at_oids_may[i]; i++) {
3744                 found_it = 0;
3745                 for (j = 0; psup_oc->oc_allowed[j]; j++) {
3746                     if (strcasecmp(psup_oc->oc_allowed[j], objClass->oc_at_oids_may[i]) == 0) {
3747                         found_it = 1;
3748                         break;
3749                     }
3750                 }
3751                 if (!found_it) {
3752                     charray_add(&OrigAllowedAttrsArray, slapi_ch_strdup(objClass->oc_at_oids_may[i]));
3753                 }
3754             }
3755         } else {
3756             /* we still need to set the originals */
3757             charray_free(OrigAllowedAttrsArray);
3758             OrigAllowedAttrsArray = charray_dup(objClass->oc_at_oids_may);
3759         }

It appears here that we build the orig_allowed set from the set of attributes that are MAY but unique to this class - IE we don't duplicate MAY from our superiors MAY set. I would hazard a guess this is to reduce cycles in the checks for may/must set validity but I haven't followed that too far.

Then to present these during an ldap search we use the function read_schema_dse, the value schema_ds4x_compat is set to (int) $208 = 0.

This then leads to:

Process 89678 stopped
* thread #11, name = 'ns-slapd', stop reason = breakpoint 3.1
    frame #0: 0x00007ffff73a85b3 libslapd.so.0`read_schema_dse(pb=0x0000608000484020, pschema_info_e=0x0000610000478040, entryAfter=0x0000000000000000, returncode=0x00007fffd5a57fc0, returntext="", arg=0x0000000000000000) at schema.c:1742:55
   1739             for (i = 0; required[i]; i++)
   1740                 size += 16 + strlen(required[i]);
   1741         }
-> 1742         allowed = schema_ds4x_compat ? oc->oc_allowed : oc->oc_orig_allowed;
   1743         if (allowed && allowed[0]) {
   1744             for (i = 0; allowed[i]; i++)
   1745                 size += 16 + strlen(allowed[i]);
(lldb) print allowed
(char **) $296 = 0x0000603000269da0
(lldb) print oc->oc_allowed
(char **) $297 = 0x0000613000006e80
(lldb) print oc->oc_orig_allowed
(char **) $298 = 0x0000603000269da0

As we can see here, as schema_ds4x_compat is 0, this means the attributes we display write into the entry is the subset of the allowed, which is the subset unique to this objectclass, excluding the content of the superior.

This is what causes the content of cn=schema to be the subset of attributes we see (may cn,sn) rather than the full set.

So I think the question is should cn=schema display the "schema as the server logically interprets it" or "the schema as defined in the ldifs of the config"?

IMO the logic for schema_ds4x_compat should be removed and we display the complete content of oc_allowed, rather than just oc_orig_allowed. But I'd really like some input from @mreynolds and @tbordaz about this, as I'm sure this is not as simple as it seems.

Thanks!

@firstyear thanks for this great investigations/explanations.
schema_ds4x_compat is OFF by default since the origin (Netscape). It allows compatibility with DS4 that more than 20 years old. I do not know if this compatibility is still working fine but I am sure nobody is now using DS4. So I agree this option should be removed.

Regarding how 389-ds should behave if we remove this option. I think we should stick to the current behaviour, so display oc_orig_allowed/required rather then oc_allowed/oc_required, unless we have good reason to change. One concern, but I guess there should be others, is with schema learning. At the moment we compare oc_orig_allowed/required to learn/check remote schema. If we change to oc_allowed/required then all already deployed instance will likely learn those extended definition, filling 99user.ldif with standard definitions.

The code looks messy and the logic is not clear when oc_allowed or oc_orig_allowed are used. I do not know if there is benefit oc_orig_allowed rather oc_allowed but it saves memory (for useless def) and possibly oc_orig_* are processed faster as there a fewer definition.

Finally I think oc_orig are better than oc_. RFC for rFC822localPart is buggy as it redefines attributes that are inherited. I think oc_orig* was done to prevent/show those useless definitions in ldif files.

In conclusion, I agree to remove schema_ds4x_compat option that looks like cleanup. I prefer to keep the use of oc_orig_* like it is now and possibly to extend its use in the server. I think we should stick to the use of oc_allowed/required when dealing with LDIF file because oc_allowed/required represent their contents.

Regarding how 389-ds should behave if we remove this option. I think we should stick to the current behaviour, so display oc_orig_allowed/required rather then oc_allowed/oc_required, unless we have good reason to change. One concern, but I guess there should be others, is with schema learning. At the moment we compare oc_orig_allowed/required to learn/check remote schema. If we change to oc_allowed/required then all already deployed instance will likely learn those extended definition, filling 99user.ldif with standard definitions.

I think the schema learning reads from internal schema structures, rather than the ones presented by cn=schema, so changing cn=schema wouldn't affect that process IMO.

Saying this I think there is wisdom in not changing the current behaviour. It's hard to know how external applications are using our cn=schema interface. While I suspect it's probably rare that applications actually read from it, it will be the unknown side effects that will come back to bite us.

So I agree we should clean up this ds4 compat switch here.

I think in this case this is due to poor schema design in this rfc as you pointed out. It is buggy. I thought about it for a while and I think that cn=schema showing schema "as it is interpreted" makes sense. In this regard our server is highlighting that the rfc822localpart schema is poorly thought out, and that the entire nature of the superior inheritance system is confusing. I have a lot to say about ldap schema but I can not change everything :)

Thankfully there is a switch in my schema migration tool to allow skipping oids, so I think I'll add this schema to that since our server is doing the right thing here, so I think I might close this issue and open a new one for the removal of this compat option. Which @mreynolds will probably like, he's been on a code removal mission I think :)

Thanks for your input @tbordaz I appreciate it!

Metadata Update from @firstyear:
- Issue close_status updated to: wontfix
- Issue status updated to: Closed (was: Open)

3 years ago

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 issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/4288

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Login to comment on this ticket.

Metadata