Page MenuHomeFreeBSD

Revision of LDAP section of the FreeBSD Handbook
ClosedPublic

Authored by rockyhotas_post.com on May 4 2017, 5:48 PM.

Details

Summary

Draft for the chapter `28.5. Lightweight Directory Access Protocol (LDAP)'.
Written by a newbie, with all his limitations. All the received suggestions should now appear on the text. Is there any other advice?

Diff Detail

Repository
rD FreeBSD doc repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
rockyhotas_post.com marked an inline comment as done.May 5 2017, 5:45 PM
rockyhotas_post.com edited the test plan for this revision. (Show Details)
This revision now requires review to proceed.May 5 2017, 5:45 PM
rockyhotas_post.com marked 3 inline comments as done.
rockyhotas_post.com added inline comments.
chapter.xml
2354 ↗(On Diff #28032)

As specified in the summary, it is "28.5. Lightweight Directory Access Protocol (LDAP)" chapter of the Handbook. This is the source file: https://svnweb.freebsd.org/doc/head/en_US.ISO8859-1/books/handbook/network-servers/chapter.xml?view=log

This revision is now accepted and ready to land.May 5 2017, 5:50 PM
sevan edited reviewers, added: Doc Committers; removed: bjk, brd, bcr.Nov 17 2017, 7:08 PM
remko added a subscriber: remko.Nov 17 2017, 7:15 PM
remko added inline comments.
chapter.xml
2703 ↗(On Diff #28076)

won' t -> will not

2749 ↗(On Diff #28076)

doesn' t -> does not

2843 ↗(On Diff #28076)

Please do not use contractions. "It will not have side effects".

remko added a comment.Nov 17 2017, 8:42 PM

More updates :) please look into them and poke me whenever I can assist you :)

chapter.xml
2397 ↗(On Diff #28076)

There is a strange space between the and OpenLDAP...

2432 ↗(On Diff #28076)

to create even the -> to create the

2467 ↗(On Diff #28076)

These two lines can be wrapped to the previous lines and redone.

2502 ↗(On Diff #28076)

Please wrap the previous line with this one and try to make sure that the line does not exceed the max..

2599 ↗(On Diff #28076)

are here specified -> are specified here.

2608 ↗(On Diff #28076)

&man.slappaswd.8C; does not exist. &man.slappasswd.8; should be the right one..

2653 ↗(On Diff #28076)

Very long line, please wrap it.

rockyhotas_post.com marked 13 inline comments as done.
rockyhotas_post.com edited the summary of this revision. (Show Details)

@remko Thank you for your suggestions.
This is a new diff, which is now based on the last revision of the chapter (50922). This even includes some final modifications I had made before uploading the diff in the bug report, where I had already fixed some typos/errors/bad phrases.
I followed all your advices, except two:

  1. After reading all the guidelines, there seem to be no way to split such long lines:
<para>In<uri
         xlink:href="http://www.openldap.org/devel/gitweb.cgi?p=openldap.git;a=tree;f=tests/data/regressions/its8444;h=8a5e808e63b0de3d2bdaf2cf34fecca8577ca7fd;hb=HEAD">

I can split before the tag, before the attribute, but not the entire string

"http://www.openldap.org/devel/gitweb.cgi?p=openldap.git;a=tree;f=tests/data/regressions/its8444;h=8a5e808e63b0de3d2bdaf2cf34fecca8577ca7fd;hb=HEAD"

If you know a way, please let me know.

  1. `&man.slappasswd.8;' does not compile:
sbase/doc/en_US.ISO8859-1/books/handbook/network-servers/chapter.xml:2714: parser error : Entity 'man.slappasswd.8' not defined

I guess this happens also in other cases, such as c_rehash(1). What about including a link to the web manpage instead?
I generated this diff with

svn diff --diff-cmd=diff -x -U999999 chapter.xml > diff_file

as suggested here.

This revision now requires review to proceed.Nov 18 2017, 12:44 PM

In general, we are getting better and better. Please keep up with that, it is much appreciated!

I do think we need to make the writing a bit more formal. Previous writing was always more formal (not restricted/limited to the ldap section) which makes the handbook more coherent. I'll graw the raw diff and look into that a bit more because that might be easier to read(for me). Phabricator mixes up a bit of the comments and rewrites and places them on the wrong lines for me. I'll get back to you as soon as time permits (hopefully this weekend or early next week).

chapter.xml
2608 ↗(On Diff #28076)

if we need &man.something then we should add it to the entity's for that. There is $base/share/xml/man-refs.ent which has a lot of those entries, also some for external ones like samba-tool. The build system obviously complaints because the reference is missing :)

In general, we are getting better and better. Please keep up with that, it is much appreciated!
I do think we need to make the writing a bit more formal. Previous writing was always more formal (not restricted/limited to the ldap section) which makes the handbook more coherent. I'll graw the raw diff and look into that a bit more because that might be easier to read(for me). Phabricator mixes up a bit of the comments and rewrites and places them on the wrong lines for me. I'll get back to you as soon as time permits (hopefully this weekend or early next week).

Thanks!
After reading the guidelines, I tried to do my best to make the writing as formal as possible. Anyway, you can read it from another perspective and this is important.
If it can help, I attach here the whole `chapter.xml' itself, so you will be eased to generate the diff you prefer.

  • The old Berkeley DB is deprecated and the file
DB_CONFIG.example

is no more present in the package. The related lines in the guide have been removed. The new example uses an MDB database.
(The error I was having during configuration was about another issue: a clean installation of FreeBSD, openldap-server and MDB perfectly works without `DB_CONFIG.example')

  • Links have been updated about packages and ports. A link to this article is used, which seems very straightforward with respect to the corresponding handbook page.
  • Parts about Certificates have all been revisited, being now more similar to the old Tom Rhodes text; with respect to the previous diff, they have been shrunk as much as possible, according to @remko suggestions. A separated space, also as much concise as possible, has been placed to provide basic information to the user as regards the Certificates creation.

The whole chapter is attached, too, in order to facilitate the creation of custom diffs.

joel_lopes-da-silva.com added inline comments.
chapter.xml
2745 ↗(On Diff #28032)

I think there is a small mistake here. It should probably be: slapd_enable="YES"

rockyhotas_post.com marked an inline comment as done.Dec 18 2017, 2:00 PM
rockyhotas_post.com added inline comments.
chapter.xml
2745 ↗(On Diff #28032)

Sure, it is like you wrote: slapd_enable="YES". Thank you

remko removed a subscriber: remko.Jan 2 2018, 6:16 PM
wollman added a subscriber: wollman.May 9 2018, 2:08 AM

Is it planned to address the client side (e.g., nss-pam-ldapd) or just the server?

chapter.xml
2545 ↗(On Diff #36598)

Grammar is a bit fractured here. I don't understand what this is saying. (But as a general rule, ciphers that aren't in the "HIGH" set should *never* be used unless absolutely required for compatibility with ancient clients, and even that contains a bunch of ciphers that should not be enabled.)

rockyhotas_post.com updated this revision to Diff 42361.EditedMay 10 2018, 1:40 PM
rockyhotas_post.com marked an inline comment as done.
rockyhotas_post.com edited the summary of this revision. (Show Details)
rockyhotas_post.com edited the test plan for this revision. (Show Details)

Correction of the first part. The original text has been restored till the slapd.ldif example. The first section of slapd.ldif configuration has been significantly shortened, and many not necessary flaws have been deleted. If this is ok, the remainder can be modified following the same method.
The whole chapter


is also attached.

A link to an already existing part of the Handbook about certificates is provided.
The text should not refer to a specific version of the OS. However, the following compiling error occurs when a manpage of the port is referred to:

sbase/doc/en_US.ISO8859-1/books/handbook/network-servers/chapter.xml:2442: parser error : Entity 'man.slapd-config.5' not defined

The temporary solution has been so far to use a weblink to the manpage:

<link xlink:href="https://www.freebsd.org/cgi/man.cgi?query=slapd-config&amp;manpath=FreeBSD+11.0-RELEASE+and+Ports">slapd-config(5)</link>

but this is about a specific version of the OS. Any suggestions to fix this?

Is it planned to address the client side (e.g., nss-pam-ldapd) or just the server?

Not at the moment. As written in the conclusion:

This is the configuration of the server only. The client, which can be the server itself, and/or another machine, relies upon other configuration files: a dedicated guide must be followed for them.

However, I have a working example of the client side configuration too, so it can be addressed as well after this text (just not planned so far).

rockyhotas_post.com marked an inline comment as done.May 10 2018, 1:55 PM

A new version with a more concise text in the first part is available.

chapter.xml
2545 ↗(On Diff #36598)

Yes, I understand. This part needed a complete rephrasing. As you can see in the new diff, it has been shrunk.
It contained some information about the "HIGH:MEDIUM" keywords and the fact that "SSLv3" has been deprecated. The original meaning was:

The line "#olcTLSCipherSuite: HIGH:MEDIUM:+SSLv3" appears in some examples, so it is mentioned here. It forces the client (from the server) to choose the security cipher. It uses the openssl TLC client syntax. When the client does not use openssl as TLS client, this line generates an error.

Please, refer now to the new version of this text. If you think that your suggestion about ciphers and "HIGH" must still be specified, I can add it.

wollman added inline comments.May 11 2018, 1:40 AM
chapter.xml
2545 ↗(On Diff #36598)

New version looks OK to me. While I'd still prefer to see policy enforced on the server side, but if there is a significant population of clients that still can't do modern TLS, then I suppose that suggestion would be counterproductive.

wollman removed a subscriber: wollman.May 11 2018, 1:41 AM
eadler resigned from this revision.Jun 18 2018, 2:26 AM
bcr added a comment.Jun 18 2018, 5:20 PM

A few suggestions, mostly about line breaks with a few textual corrections.

chapter.xml
2443 ↗(On Diff #42361)

There must not be a space between the closing > and slapd-config(5)</link>. Even though the link is long and the page can not be broken earlier, it must be there or there is a visual extra space in the output.

2557 ↗(On Diff #42361)

Same as above: no space between <filename> and slappasswd</filename>

2567 ↗(On Diff #42361)

s/showed/shown/

2574 ↗(On Diff #42361)

And another superfulous space: <filename>slapd.ldif</filename>

2575 ↗(On Diff #42361)

Here as well.

2603 ↗(On Diff #42361)

you can break this one like the following:
...longurl>this
repository</link>

2604 ↗(On Diff #42361)

Let <filename> and slapd.ldif</filename> come closer together here, too.

2608 ↗(On Diff #42361)

And another space that needs to go. You can break the line between "this" and "page".

2622 ↗(On Diff #42361)

I think we don't need "anyway" here.

2636 ↗(On Diff #42361)

And one more space to remove.

2659 ↗(On Diff #42361)

s/will not still be/will still not be/

2661 ↗(On Diff #42361)

This line needs to be on the one above. Continuing after the "anyway."

2667 ↗(On Diff #42361)

s/FreeBSD/&os;/
s/a/the/

2697 ↗(On Diff #42361)

Move this sentence up one line.

2701 ↗(On Diff #42361)

Same here, move the sentence up to start after "boot."

2715 ↗(On Diff #42361)

no space here between <filename> and /var/log/messages</filename>

2723 ↗(On Diff #42361)

s/here provided/provided here/

2724 ↗(On Diff #42361)

No space here.

2766 ↗(On Diff #42361)

No line break here.

2786 ↗(On Diff #42361)

Move this one up to continue the line.

2806 ↗(On Diff #42361)

Move this one up, too.

2808 ↗(On Diff #42361)

This can also go on on the previous line.

2822 ↗(On Diff #42361)

no break here after the colon (:). The "it" will certainly fit into this line and then wrap like normal.

rockyhotas_post.com updated this revision to Diff 44148.EditedJun 20 2018, 2:49 PM
rockyhotas_post.com marked an inline comment as done.
rockyhotas_post.com edited the summary of this revision. (Show Details)

Applied all the modifications suggested by @bcr . Thank you!
The line-breaking were a first attempt to handle too-long lines, your solution is far better. A couple of comments:

  • Your correction was useful, because it let me learn how to handle long lines. However, this text may not be in its definitive form, because the actual steps of the configuration have never been tested by someone else.
  • Some of the links that generated the long lines were a temporary solution, too. If you want, for more information, please refer to this previous comment.

This is the whole

xml file.

rockyhotas_post.com marked 23 inline comments as done.Jun 20 2018, 2:54 PM
bcr added a comment.Jun 21 2018, 4:58 PM

So, you're saying the we need testers for this configuration before we put it in the handbook as official instructions?

Overlong lines are sometimes unavoidable, especially when dealing with long links. The only way to break the line there is like this:
<link
xlink:href="http://www.openldap.org/doc/admin24/slapdconf2.html">here</link>

and not like this:
<link xlink:href=

	  "http://www.openldap.org/doc/admin24/slapdconf2.html">here</link>

Out of curiosity, did you run textproc/igor on your changes to see what it says?
But like you said, the text is more important to have and we don't expect perfection from submitters. We doc people can clean up some of the style and whitespace nits.
I like your approach though, that learning by doing will bring you closer to the desired result.

bcr removed a reviewer: manpages.Jun 21 2018, 4:59 PM
In D10600#337622, @bcr wrote:

So, you're saying the we need testers for this configuration before we put it in the handbook as official instructions?

Uhm, not exactly. Sorry, I never wrote a documentation patch and I made some guesses that maybe are wrong.
For example, I thought that the configuration (as well as the syntax) must be tested by someone else. But this configuration works for me: if this is enough, it's ok!

Overlong lines are sometimes unavoidable, especially when dealing with long links. The only way to break the line there is like this:
<link
xlink:href="http://www.openldap.org/doc/admin24/slapdconf2.html">here</link>
and not like this:
<link xlink:href=

	  "http://www.openldap.org/doc/admin24/slapdconf2.html">here</link>

Ok!

Out of curiosity, did you run textproc/igor on your changes to see what it says?

Yes, and IIRC it didn't give error about my wrong way to break links. But during the last corrections, new long lines were generated, and igor always suggested your methods to break them.

We doc people can clean up some of the style and whitespace nits.

Is this "your" only role? If you want, any hint about it could be very, very useful.
I have a little confusion, in particular about what I'm supposed to do and what "you" are supposed to do.

I like your approach though, that learning by doing will bring you closer to the desired result.

Thank you. I'm trying to do my best and to learn.

rockyhotas_post.com edited the summary of this revision. (Show Details)

Also the second part has been completely revisited, to be more concise and more uniform to the original text.
Links to specific versions of the OS are still present. Please, refer to this previous comment for more details. How to fix this?

The whole xml file is included, too:

rockyhotas_post.com added a comment.EditedSep 19 2018, 2:42 PM

An update as regards the links issues. According to the Primer,

It is preferred to use the &man.command.section; notation

But only links generated in the extended way

<citerefentry>
    <refentrytitle>command</refentrytitle>
    <manvolnum>sectionnumber</manvolnum>
  </citerefentry>

are correctly shown in the output, as regards manpages like slapd-config(5), slapd(8), slappasswd(8). So, despite the Primer suggestion, is this the way they should be written in this case?
Moreover, should 8C be used instead of 8 as regards slapd(8) and slappasswd(8)?

bcr accepted this revision.Dec 20 2018, 8:30 PM

I think the patch is fine now. I'll see to it that it gets committed.
Thanks for your patience, Rocky Hotas.

This revision is now accepted and ready to land.Dec 20 2018, 8:30 PM
In D10600#397141, @bcr wrote:

I think the patch is fine now. I'll see to it that it gets committed.
Thanks for your patience, Rocky Hotas.

No problem. Just remember to adapt the beginning (removing the word "Draft") and the manpage issue mentioned in this comment, which is still present in the current version of the patch.

For any need, let me know. And thank you!

bcr added a comment.Dec 20 2018, 9:40 PM

Yes, I've changed those.
Now that the server part is about to be committed, do you also plan to do a section about the client? We have an article mentioning pam_ldap, but that article should either be reworked or put into the handbook altogether.

This revision was automatically updated to reflect the committed changes.
In D10600#397149, @bcr wrote:

Yes, I've changed those.

Ok!

Now that the server part is about to be committed, do you also plan to do a section about the client? We have an article mentioning pam_ldap, but that article should either be reworked or put into the handbook altogether.

Yes, maybe you are referring to this section.
I would be eager to write about the client. Being aware that it needs some weeks (I completed its configuration a while ago, must recall all the steps), I can proceed and keep you updated.
For the moment, a dedicated Handbook client Section seems the best solution to me: it would also merge and update some of the information in the mentioned article.
As a separate task, the article itself can be reconsidered: there is chance that it is entirely absorbed in the Handbook.

bcr added a comment.Dec 21 2018, 7:49 AM
In D10600#397149, @bcr wrote:

Yes, I've changed those.

Ok!

Now that the server part is about to be committed, do you also plan to do a section about the client? We have an article mentioning pam_ldap, but that article should either be reworked or put into the handbook altogether.

Yes, maybe you are referring to this section.

Indeed, that's the article I was referring to.

I would be eager to write about the client. Being aware that it needs some weeks (I completed its configuration a while ago, must recall all the steps), I can proceed and keep you updated.
For the moment, a dedicated Handbook client Section seems the best solution to me: it would also merge and update some of the information in the mentioned article.
As a separate task, the article itself can be reconsidered: there is chance that it is entirely absorbed in the Handbook.

I'd like that. Let's work together on that one. I've recently found out that pam_ldap is outdated for a couple of years and that there are some issues with it. Other clients like nss-pam-ldapd solve those (i.e., less connections to the server, better local caching) that I would like to give more prime-time by mentioning them in the setup. Integrating the article into the handbook seems worthwhile for consistencies sake. We can do this over email outside of this review. Let me know if you have any questions or need help.