Semantics for multiple pkinit_anchors/pkinit_pool lines
ghudson at mit.edu
Thu Jan 28 12:26:39 EST 2021
On 1/28/21 10:50 AM, Ken Hornstein wrote:
> Just so I'm clear on the semantics ... obviously, you need at least one
> working pkinit_anchors line. Do you want to have at least one working
> pkinit_pool line as well?
Yes, if pkinit_pool is specified.
> So, fine, seems pretty obvious that more KRB5_TRACE
> points would be helpful for everyone
> I'm curious about why the
> TRACE macro is never called directly (I guess it IS called directly
> once in Leash)
When the trace logging system was designed, there was some theoretical
interest in integrating it with DTrace. Centralizing the trace points
could facilitate this; an OS packager could perhaps change all of the
macros in k5-trace.h and pkinit-trace.h to call DTrace functions. I
don't think anyone has taken advantage of that, though.
> I am wondering if there are
> any unwritten style issues with the use of TRACE().
See the comments in include/k5-trace.h. The note about not tracing
before an unrecoverable errors is less applicable to PKINIT, because
errors from PKINIT are often obscured by the preauth framework.
> - It seems like nearly all preauth errors are flattened down to the
> error KRB5_PREAUTH_FAILED.
Or KRB5KDC_ERR_PREAUTH_FAILED if they come from the KDC. SAM-2 is a
little unusual among preauth mechanisms because the password being wrong
is detected on the client side (it has this in common with the
no-preauth scenario, where a wrong password leads to a
KRB5KRB_AP_ERR_BAD_INTEGRITY error when trying to decrypt the KDC reply).
> So I understand that the "real" error is being appended to the error
> message, which is fine. But this presents a problem in practice, as
> many programs don't display the error message string and just the
> output of com_err on the error code.
Our programs or out-of-tree programs?
> And the specific issue we're
> running into is if a user is using the SAM2 preauth mechanism and
> they enter the wrong password, instead of saying "Password incorrect",
> they get "Preauth failed", because the correct error IS returned by the
> preauth mechanism but never makes it up to the application.
kinit specifically looks for KRB5KRB_AP_ERR_BAD_INTEGRITY and
KRB5KDC_ERR_PREAUTH_FAILED after prompting for a password (see kinit.c
line 810). The second check could probably be extended to cover
KRB5_PREAUTH_FAILED as well.
> I have done enough stuff with the preauth code over the years to
> know that it is ... complicated. So I could believe that returning
> the "correct" error code could cause some unexpected issues; I do
> not believe that it would, but I could be wrong.
There's a bit of history here:
* In commit 632260bd1fccfb420f0827b59c85c329203eafc9 (ticket 7517) we
started doing what you proposed, returning the error code from the first
failed non-informational preauth mech.
* In commit 560e11dabb63b141df29c54aaa2e120309a1e021 (ticket 8457) we
switched to wrapping with KRB5_PREAUTH_FAILED, because the S4U2Self code
expected to see that error code during realm identification.
* In commit 9d6847c8f0187a0dd6466420335b5460de5c449e we added a better
internal API for realm identification, removing the consideration that
led to the wrapping.
So we could perhaps return to the ticket 7517 behavior. But for the
specific SAM-2 problem you describe, I think it would be better for
krb5_get_init_creds_password() to recognize password-incorrect errors
using the logic currently in kinit (extended to include client-side
preauth errors). There is currently no com_err code for "password
incorrect", so we'd have to add one to k5e1_err.et.
More information about the krbdev