Add -g option for custom group attribute

[ankor2023]

Changes:

• Added -g option:
  Allows specifying the annotation attribute name used by the helper
  to return groups. This enables using attributes like clt_conn_tag
  for group annotation within a connection.

• Updated helper output format:
  Groups are now returned in a single key with comma separated values:

  group=group1,group2,group3

Efficiency:
The new format reduces overhead by removing redundant attribute names
for each group, allowing more groups to fit within the same buffer size.

[yadij]

The change to send list syntax instead of separate notes has been on my TODO list for a long while. Thank you for implementing.

However, IMO we should not make the key name configurable;

• first because "group=" has documented semantics and special handling associated to ensure the behaviour happens, and
• changing the name looses all indications that the values presented are group names, and
• secondly because in the use-case where "clt_conn_tag=" is useful, it is likely that "group=" will be needed at the same time (eg. by other helpers).
  • What that case actually needs is all notes from an Negotiate and NTLM auth'n helper applied to the connection notes. Which is a separate feature change out of scope here.

[ankor2023]

However, IMO we should not make the key name configurable;

• first because "group=" has documented semantics and special handling associated to ensure the behaviour happens, and
• changing the name looses all indications that the values presented are group names, and
• secondly because in the use-case where "clt_conn_tag=" is useful, it is likely that "group=" will be needed at the same time (eg. by other helpers).

We provide administrators with the ability to customize helper behavior while keeping the default behavior unchanged.

As an alternative, we could maintain the current 'group' annotation while duplicating its value to an additional attribute under the -a flag.

Or would it be better to wait for a more generic functionality to copy all annotations from authentication helpers?

[ankor2023]

I'm also planning a subsequent PR to implement group filtering. In large environments, Kerberos tickets often contain 200–300 groups, while only a few are actually used in Squid policies. Filtering them out would significantly reduce overhead, improve performance, and simplify policy administration.
Would this feature be consistent with the current development roadmap?

[rousskov]

I'm also planning a subsequent PR to implement group filtering. In large environments, Kerberos tickets often contain 200–300 groups, while only a few are actually used in Squid policies. Filtering them out would significantly reduce overhead, improve performance, and simplify policy administration. Would this feature be consistent with the current development roadmap?

My answer would depend on whether that filtering is universal or highly custom:

• If virtually none of the deployed negotiate_kerberos_auth helpers would be hurt by that filtering and many would benefit from it, then it would be worth discussing such a filtering algorithm further (preferably before a PR with a lot of code changes needs to be reviewed!).
• Otherwise, the algorithm probably belongs to a custom helper rather than official Squid sources.

Please note that an algorithm that can be easily implemented as helper output filter, should probably not go into the helper:

negotiate_kerberos_auth ... | sed 's/,groupFoo,/,/'

The above sketch illustrates the concept of an external filter. It is not meant to be used "as is", of course.

[ankor2023]

@rousskov

• If virtually none of the deployed negotiate_kerberos_auth helpers would be hurt by that filtering and many would benefit from it, then it would be worth discussing such a filtering algorithm further (preferably before a PR with a lot of code changes needs to be reviewed!).

I would like to implement a new -f option that accepts a comma-separated list of groups (base64-encoded SIDs or standard SID strings (S-1-5-...)) to pass to Squid. All other groups will be filtered out:
-f SID1,SID2,SID3

I also intend to add SID-to-name mapping:
-f SID1:groupVIP,SID2:groupADM,SID3:groupEMPL

With this, the helper will return group names to Squid instead of SIDs.
This will allow administrators to use readable names in policies instead of cryptic SIDs, making them much easier to understand.
Implementing this directly in the helper's C code will improve performance when handling high loads.

Alex, where’s the best place to discuss the filtering logic?

[ankor2023]

@yadij

• What that case actually needs is all notes from an Negotiate and NTLM auth'n helper applied to the connection notes. Which is a separate feature change out of scope here.

Amos, I agree that if this is implemented in the near future, the -g/-a option we're discussing will become redundant. I took a quick look at the Squid codebase, but it's a bit too complex for me to handle this PR on my own.

I’ve located the function that processes helper annotations. However, how do we distinguish whether the response was received from the Negotiate or NTLM helper specifically?

void
UpdateRequestNotes(ConnStateData *csd, HttpRequest &request, NotePairs const &helperNotes)
{
    // Tag client connection if the helper responded with clt_conn_tag=tag.
    const char *cltTag = "clt_conn_tag";
    if (const char *connTag = helperNotes.findFirst(cltTag)) {
        if (csd) {
            csd->notes()->remove(cltTag);
            csd->notes()->add(cltTag, connTag);
        }
    }
    request.notes()->replaceOrAdd(&helperNotes);
}

On the other hand, annotations from Basic auth helpers should also be connection-bound. Thus, we can create a new annotation update function UpdateAuthRequestNotes() for auth helpers and call it from authTryGetUser().
Something like this:

void
UpdateAuthRequestNotes(ConnStateData *csd, HttpRequest &request, NotePairs const &helperNotes)
{
    if (csd) {
        csd->notes()->replaceOrAdd(&helperNotes);
    }
    request.notes()->replaceOrAdd(&helperNotes);
}