libtacplus: Allow additional AV pairs to be configured.

[des]

Summary

• Replace hand-rolled input tokenizer with openpam_readlinev() which supports line continuations and has better quoting and escaping.
• Simplify string handling by merging struct clnt_str and struct srvr_str into just struct tac_str.
• Each server entry in the configuration file can now have up to 255 AV pairs which will be appended to the ones returned by the server in response to a successful authorization request.

This allows nss_tacplus(8) to be used with servers which do not provide identity information beyond confirming the existence of the user.

This adds a dependency on libpam, however libtacplus is currently only used by pam_tacplus(8) (which is already always used with libpam) and the very recently added nss_tacplus(8) (which is extremely niche). In the longer term it might be a good idea to split this out into a separate library.

MFC after: 1 week
Sponsored by: Klara, Inc.

[des]

Fix brain glitch in man page.

[pauamma_gundo.com]

lib/libtacplus/tacplus.conf.5

48–49

This is saying that "#foobar" is a valid field but "foo#bar" isn't. Is that still true with the new tokenizer? openpam_readlinev(3) leads me to openpam_readword(3), which states

If a hash character occurs within a word, however, it is preserved as-is.

which I interpret as permitting "foo#bar".

[des]

lib/libtacplus/tacplus.conf.5

48–49

Thanks for catching that, I'll check.

[des]

lib/libtacplus/tacplus.conf.5

48–49

OK so I think you misunderstood, it's not talking about the legality of a # character but about whether it will be seen as the start of a comment or as a regular character within a field. In both cases # starts a comment only if it is the first character on the line or is preceded by unescaped whitespace. In all other cases it is a regular character. This is unchanged. What has changed is mainly support for line continuations and improved quoting.

[des]

Shorten man page, add UPDATING entry.

[markj]

lib/libtacplus/taclib.c

413

Presumably this should be explicit_bzero() or memset_s()?

733

Can't you write len += strlen(p); instead of having this loop?

736

Is len supposed to include the =? It looks like it does not.

737

Is srvp->navs guaranteed to be initialized? It might be nice to reset it at the beginning of the loop regardless.

854

Suppose strlen(fields[2]) == 0, i.e., the timeout was specified as "" in the config. Then we don't increment i, so below we'll compare "" with "single-connection", which doesn't seem to be the desired behaviour. In that case, I'd expect the timeout to be the default, and that we continue processing the next field.

874

Indentation here is wrong.

[markj]

lib/libtacplus/taclib.c

734

xstrdup() does not fail.

[des]

lib/libtacplus/taclib.c

733

That's a matter of taste.

734

Yes it does.

736

It is and it does.

854

That is exactly the desired behavior. If you want the default timeout value, don't specify a timeout.

[des]

Incorporate review feedback.

[markj]

UPDATING

35

RELNOTES is a more appropriate place to document this, since the change will affect anyone upgrading to 14.0, not just users tracking main. Or, add a "Relnotes: yes" tag to the commit log message.