kdc: Allow plugin to override KDC reply e-data #1096
Conversation
|
Rebased on master. |
|
@nicowilliams It would be very helpful to get this merged, we think we have addressed all the concerns raised by @lhoward. This change is quite important to Samba because if we don't get the error replies matching windows exactly we get clients retrying in a way that locks them out twice as fast under "bad password lockout". |
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
This matches the behaviour of Windows. View with ‘git show -b’. Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
|
The recent update by @josephsutton1 is related to https://gitlab.com/samba-team/samba/-/merge_requests/3377 - without that change Heimdal clients against a Windows DC that is imposing a Silo / authentication policy restriction gets a very confusing |
| free_KERB_ERROR_DATA(&kerb_error_data); | ||
| } else { | ||
| /* OK. */ | ||
| free_KERB_ERROR_DATA(&kerb_error_data); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Just below this we have:
/*
* Unwrap KRB-ERROR, we are always calling this so that
* FAST can tell us if your peer KDC suddenly dropped FAST
* wrapping and its really an attacker's packet (or a bug
* in the KDC).
*/
ret = _krb5_fast_unwrap_error(context, ctx->nonce, &ctx->fast_state,
&ctx->md, &ctx->error);
and a similar call in the get_cred_kdc() case.
The problem is that KERB-ERROR-DATA is not wrapped in FX-FAST, but _krb5_fast_unwrap_error() will notice if the reply is supposed to be FAST‐armored (and give the potentially confusing error message “FAST fast response is missing FX-FAST”).
Is it safe to bypass this call and treat a KERB-ERROR-DATA response as authoritative when we can’t guarantee that the KDC really issued the response?
There was a problem hiding this comment.
I checked the FAST RFC, which says:
“If the Kerberos FAST padata is included in the request but not included in the error reply, it is a matter of the local policy on the client to accept the information in the error message without integrity protection. However, the client SHOULD process the KDC errors as the result of the KDC's inability to accept the AP_REQ armor and potentially retry another request with a different armor when applicable” — that’s not the case here. “The Kerberos client MAY process an error message without a PA-FX-FAST-REPLY, if that is only intended to return better error information to the application, typically for trouble-shooting purposes.”
…o error code Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
|
Now Samba will produce error messages that are a bit more helpful:
|
Don’t blindly use ‘clientname’ and ‘servername’, but check return codes to ensure their contents have been properly initialized. Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
This type is used by Windows to carry custom error data, usually an NTSTATUS code. Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
…-DATA) View with ‘git show -b’. Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
This field can be set to override the error data that the KDC would have returned. Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
The creation of FAST e-data is now handled by a new function, _kdc_fast_mk_e_data(). Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
…ta in reply If the e_data field in the KDC request structure has been set, use that in preference to creating the error data ourselves. Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
|
Sigh, MSVC appears to violate the C standard by supporting |
|
Instead, MSVC forces us to use |
|
That could be put into a define in roken if we wanted. |
…acros MSVC doesn’t define these macros. It uses its own implementation‐ specific format specifiers for fixed width integer types. Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
| free_KERB_ERROR_DATA(&error_data); | ||
| goto out; |
There was a problem hiding this comment.
This won’t work. We need to continue to the ret == KRB5KDC_ERR_KEY_EXP && ctx->runflags.change_password == 0 && ctx->runflags.change_password_prompt case so that the user can be prompted to change their password if it has expired.
This is useful in order to produce custom error responses, as Windows does for NTSTATUS codes.