Fix IDTOKENS backwards-incompatibilities

Description

  1. Depending on the results of BrianB’s testing, “double” either the pool password when used as an IDTOKENS signing key, or all signing keys.

  2. Fix bug in “doubling” code where the returned length is incorrect / not enough bytes are returned.

  3. Undo the change which introduced an “early out” in the pruning code (to allow nonroot clients to use tokens again).

Activity

Show:
Todd L Miller
March 29, 2021, 5:34 PM

Code Review

[Second review requested by ToddT because we’re releasing 8.9.12 with basically no burn-in period.]

The patch looks more complicated than it really is because it (also) renames some parameters, but it’s actually pretty simple. Looks good to me.

Todd Tannenbaum
March 29, 2021, 5:32 PM

Code review comments addressed. No need for documentation as this ticket fixes bugs in unreleased (or pulled, sigh) versions.

Todd Tannenbaum
March 29, 2021, 5:01 PM
Edited

CODE REVEW

  1. Tested non-root client using a token to authenticate: It now no longer strips IDTOKENS out of the client list if /etc/condor/passwords.d is not world-readable. Woot! The patch is sufficient until is resolved.

  2. Tested using using signing keys with and without NULLs…. signing keys without NULLs now work across versions as expected (meaning “doubling” code fixed properly).

  3. SEC_TOKEN_POOL_SIGNING_KEY_IS_PASSWORD: Decision is give up on this knob, leave it undocumented until removed from the code, and instead go with the tool introduced in HTCONDOR-369. Given this, there is no need to merge the Pull Request #163 attached to this ticket.

  4. Change default for SEC_TOKEN_POOL_SIGNING_KEY_FILE to equal $(SEC_PASSWORD_FILE)

Todd L Miller
March 29, 2021, 4:21 PM

ToddT will either do the second code review or ensure that BrianB does.

Brian Bockelman
March 29, 2021, 3:48 PM

I think if we want to do a backward compatibility break with no apparent benefit to sites other than “cleaning up the defaults”, then we need to do a bit more planning here. I don’t want to make breaks in a pinch.

This change didn’t make it into the release notes for 8.9.12.

Due date

2021/03/29

Time remaining

0m

Assignee

John (TJ) Knoeller

Is PATh development

None

Fix versions

Priority

Blocker

HTCondorCustomerGroup

Other

Components

Reporter

Todd L Miller