On Fri, 2017-04-21 at 07:55 +0200, Simon Pichugin wrote:
> On Fri, Apr 21, 2017 at 09:25:23AM +1000, William Brown wrote:
> > On Thu, 2017-04-20 at 22:54 +0200, Simon Pichugin wrote:
> > > Hello William,
> > > I think my question is for you in the first place.
> > > It regards the default attributes for cn=config feature.
> > > 
> > > Version tested: 389-ds-base-1.3.6.1-9.el7.x86_64
> > > 
> > > During TET troubleshooting I've faced two issues:
> > > 1. By default, we have:
> > > [root@qeos-126 dirsrv-tet-install]# ldapsearch -h localhost -p 389 -D 
> > > "cn=Directory manager"
> > >     -w Secret123 -b "cn=config" "objectclass=*" | grep 
> > > nsslapd-allowed-sasl-mechanisms
> > > nsslapd-allowed-sasl-mechanisms:
> > > 
> > > Empty value.
> > > 
> > > We can modify it and set something. (I'll skip the output, it works as 
> > > expected.
> > > And after this, the server allows to do like this:
> > > [root@qeos-126 dirsrv-tet-install]# ldapmodify -h localhost -p 389 -D 
> > > "cn=Directory manager" -w Secret123
> > > dn: cn=config
> > > changetype: modify
> > > delete: nsslapd-allowed-sasl-mechanisms
> > > 
> > > modifying entry "cn=config"
> > > 
> > > [root@qeos-126 dirsrv-tet-install]# ldapsearch -h localhost -p 389 -D 
> > > "cn=Directory manager"
> > >     -w Secret123 -b "cn=config" "objectclass=*" | grep 
> > > nsslapd-allowed-sasl-mechanisms
> > > nsslapd-allowed-sasl-mechanisms:
> > > 
> > 
> > > 
> > 
> > Pretty sure the SASL mechs can't be written to: they are returned from
> > https://pagure.io/389-ds-base/blob/master/f/ldap/servers/slapd/saslbind.c#_754
> >  
> > 
> > And that only allows setting from mechs discovered by cyrus-sasl, and
> > mechs that are from plugins that register to
> > slapi_register_supported_saslmechanism() . The allowed mechs parameter
> > is the list that is checked to see if we'll allow using it, and that's
> > called from ids_sasl_getopt. During the set in libglobs, it looks like
> > we check that the mech is a real name supported by SASL, so perhaps test
> > with values like PLAIN, GSSAPI instead? 
> > 
> > > 
> > > 2. Second one is a known issue, but still I'd like to clarify the 
> > > expected behaviour:
> > > [root@qeos-126 dirsrv-tet-install]# ldapsearch -h localhost -p 389 -D 
> > > "cn=Directory manager"
> > >     -w Secret123 -b "cn=config" "objectclass=*" | grep 
> > > nsslapd-allowed-sasl-mechanisms
> > > nsslapd-allowed-sasl-mechanisms: A
> > > [root@qeos-126 dirsrv-tet-install]# ldapmodify -h localhost -p 389 -D 
> > > "cn=Directory manager" -w Secret123
> > > dn: cn=config
> > > changetype: modify
> > > add: nsslapd-allowed-sasl-mechanisms
> > > nsslapd-allowed-sasl-mechanisms: B
> > > [root@qeos-126 dirsrv-tet-install]# ldapsearch -h localhost -p 389 -D 
> > > "cn=Directory manager"
> > >     -w Secret123 -b "cn=config" "objectclass=*" | grep 
> > > nsslapd-allowed-sasl-mechanisms
> > > nsslapd-allowed-sasl-mechanisms: B
> > > 
> > > So it wouldn't be a multivalued attribute? if we'll do the 'add' 
> > > operation, it would replace the existing value with a new.
> > > 
> > > Please, comment of a both cases. First looks more like a bug to me 
> > > though, and I will file it if you'll confirm it.
> > 
> > Anyway, it looks like in libglobs.c, that it expects a comma seperated
> > list, it's not multivalued. The reason is that this single attribute is
> > returned in  config_get_allowed_sasl_mechs(); during the ids_sasl_getopt
> > call, which SASL_CB_GETOPT expects a specific format.
> > 
> > I imagine this is because it's easier to store it as one line in the
> > config, and requires less parsing each SASL request to go from
> > multivalue to one line, so it's an efficiency thing. 
> > 
> > 
> > Does that help explain the answer to your problems? 
> 
> Partly, yes. My question was about possible regression.
> 
> We have a lot of tests for nsslapd-allowed-sasl-mechanisms in TET.
> It has raised two concerns I have now.
> 
> First, if we are trying to set 'empty value' with a replace operation, it 
> fails (and it is okay).
> 
> But now with your new cn=config feature we have 
> 'nsslapd-allowed-sasl-mechanisms' empty by default.
> It seems to be wrong as long as the servers forbids to set 'empty value' 
> manually.

Well, the bug there is we can't set an empty value. You should be able
to delete the attribute though, and that should reset it.

Additionally, it may be worth checking that if the value is empty sasl
treats that as *all* mechs are allowed. This would be one of the few
config items where I think it should be a blacklist, rathar than
whitelist. 

So certainly, this could be a bug. 

> 
> Second test that fails now asserts that we can't add more values to the
> single-value attribute (nsslapd-allowed-sasl-mechanisms is the one).
> The test excpects an error. But the server just replaces the value with
> a new one. No error happens. Is it by design or we should fix it?

I did fix some code related to add/replace. I think that if you add
another value, and it replaces, that's a bug. It contradicts the ldap
standard I think. 

> 
> > 
> > 
> > -- 
> > Sincerely,
> > 
> > William Brown
> > Software Engineer
> > Red Hat, Australia/Brisbane
> > 
> 
> 

-- 
Sincerely,

William Brown
Software Engineer
Red Hat, Australia/Brisbane

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
389-devel mailing list -- 389-devel@lists.fedoraproject.org
To unsubscribe send an email to 389-devel-le...@lists.fedoraproject.org

Reply via email to