cyrus-sasl can't gssapi + ldaps + maxssf=1 anymore?

[ghost]

The following command fails with cyrus-sasl 2.1.26:

ldapsearch -LLL -Y GSSAPI -H ldaps://example.com  -O "maxssf=1" -s "base" -b "" supportedSASLMechanisms

ldap_sasl_interactive_bind_s: Local error (-2)
        additional info: SASL(-1): generic failure: GSSAPI Error:  A token was invalid (unknown mech-code 0 for mech unknown)

Whereas it works fine in cyrus-sasl 2.1.23.

Is there somewhere I can submit proper logs and get advice on debugging this? Is there any chance this is a known issue with a workaround? Thanks.

[ghost]

Holy crap. Is this bug really from 2014? http://web.archive.org/web/20140725050152/https://bugzilla.cyrusimap.org/show_bug.cgi?id=3480

[quanah]

2015? The bug says it was filed in 2011. ;)

Reported: 	2011-06-03 03:26 EDT by Dan White

[JanParcel]

On 02/14/2017 03:44 PM, Quanah Gibson-Mount wrote: 2015? The bug says it was filed in 2011. ;) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#419 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AWZUIyDg35wyC8fcC1N3dSOx_BGOXayRks5rcjxegaJpZM4L-S_F>.

Not wrt that bug (though it sure sounds familiar) but do we have to learn to use git in order to pass fixes or contributions upstream? (I have a passel of man pages and a few possible bug fixes to contribute.)

…

-- Jan Parcel, Developer Oracle Systems, SPARC & Solaris System Software Engineering

[quanah]

I'm not part of the dev team, so couldn't say for sure, but my guess would be yes, you should spend the time to learn how to use git and how to create merge requests via github. But that's just a best guess. :)

[ghost]

@ksmurchison Will this be addressed by the patch discussed in ML?

[ksmurchison]

Please attach the patch to this issue or create a pull request with the fix

[quanah]

@ksmurchison I can't connect to the cyrus bugzilla where the patch is attached, it's not in the wayway back machine, and it doesn't appear to have been posted to the debian list discussion. Were the bugzilla attachments preserved when it was migrated to github?

[ksmurchison]

I thought the script that @brong used to migrate from bugzilla was supposed to preserve attachments. I will have to ask.

[quanah]

@ksmurchison I will see if it is in the debian cyrus-sasl patch set as well

[quanah]

Hm, I don't see a patch specifically referencing the debian bug. However, they do have 35 patches to the 2.1.26 source that may be worth reviewing.

[nacho]

See that we had a lot of patches downstream in fedora which probably they are the same in debian that were already pushed to master

[quanah]

@nacho I'm checking them now. The first one I checked is not currently committed into the SASL project.

[ksmurchison]

Pull requests would be greatly appreciated

[quanah]

@ksmurchison Yep, as soon as I get a full list :)

[quanah]

Found the attachment: https://github.com/cyrusimap/BugzillaImport/blob/master/attachments/1393/revert-flags.diff

[quanah]

Commit that broke things was 080e51c

[ksmurchison]

@aamelnikov Can you give this a look? I also assigned a bunch of other GSSAPI/Kerberos bugs to you

[Jakuje]

@quanah the patch is just a revert of 080e51c without the whitespace changes.
It was already said that the patch is correct according to the RFC [1], but it might not be well accepted by the other party.

[1] https://lists.andrew.cmu.edu/pipermail/cyrus-sasl/2015-April/002809.html

[quanah]

@Jakuje I'm simply noting that #349 is a duplicate of this one. Clearly the behavior change broke things (See #378 as well).

[quanah]

@Jakuje Also, reading that response you linked to, it appears to say the code was RFC compliant w/o the patch as well...

[aamelnikov]

maxssf controls SSF provided by a combination of external SSF (e.g. TLS) and the mechanism SSF. If you are using ldaps, you are likely already getting 128+ SSF. So when you also include "maxssf=1", this means that GSSAPI can't satisfy the request, because external SSF (say 128) + mech ssf (say 0) > maxssf of 1. Basically GSSAPI can't comply with the request.

So can you try "maxssf=N+1", where N is the TLS SSF (symmetric cipher strength)?

There might be other problems with the patch, but at the moment what you are trying to do is not legal.

[quanah]

@aamelnikov Your analysis is incorrect. The "-O" parameter to ldapsearch involves the SASL security strength layer, not the TLS security strength layer.

``` -O security-properties
Specify SASL security properties.

[quanah]

@ksmurchison any update on this? @aamelnikov seems nowhere to be found. The referenced email thread does not indicate that fixing this would be contrary to the specs.

[aamelnikov]

I talked to Ken and I am pretty sure that the proposed patch (to roll back an earlier change) is incorrect.

1. The current GSSAPI code matches RFC 4752, which was created based on input from people from MIT, such as Sam Hartman. If what RFC 4752 says doesn't interoperate for some reason, there must be a comment added to the patch explaining why it doesn't work, for example by explaining how MIT or Heimdal Kerberos V5 implementations work.

2. If you look at Cyrus SASL code, external ssf is subtracted from min_ssf/max_ssf pretty much everywhere in the code (*). If you want to provide your own analysis and prove me wrong - be my guest!
   So, if ldapsearch is setting external ssf on SASL context, max_ssf = 1 will never work with GSSAPI.

(*) The way how (max_ssf - external_ssf) < 0 is handled in different plugins is not entirely consistent and it might result in ssf = 0 in some, while others might stop working altogether. This is something that needs fixing in Cyrus SASL, but that is definitely to the fix suggested in this thread.

3. The proposed change has the effect of not requesting GSS_C_INTEG_FLAG unless max_ssf > external_ssf. This is likely to have an effect on security flags returned from the established GSSAPI context once authentication is complete, down in the same function:

   if ((out_req_flags & GSS_C_INTEG_FLAG) == 0) {
   /* if the integ_avail flag is not set in the context,
   then no security layer can be offered or accepted. /
   text->qop = LAYER_NONE;
   } else if ((out_req_flags & GSS_C_CONF_FLAG) == 0) {
   / If the conf_avail flag is not set in the context,
   then no security layer with confidentiality can be offered
   or accepted. */
   text->qop = LAYER_NONE | LAYER_INTEGRITY;
   } else {
   text->qop = LAYER_NONE | LAYER_INTEGRITY | LAYER_CONFIDENTIALITY;
   }

I suspect that no SASL security layer is negotiated for max_ssf = 1 + ldaps, which means that the code is not working as you expect anyway.

Having said that, I don't claim that the code is perfect or even that is working as intended. In order to make progress on this issue I will need some help from people, in particular, I will need the following information:

Some confirmation on whether external SSF is non 0 (I can provide a debug patch for this).
Values of text->qop and/or out_req_flags variable once GSSAPI authentication is complete.
Version and implementation name of Kerberos V5 used when seeing the problem.
OS version.

[quanah]

As someone who experienced in a non MSAD environment, I can confirm it's not solely limited to MSAD. See also the comment from ' Jakuje commented on Nov 24, 2017 ' and issue #349 and issue #378

[Jakuje]

We have some automated tests against older version of cyrus-sasl and some MSADs, but I am not sure if I tested the latest rc with the patch reverted or not.
I will try to have a look into that in coming days, ideally with a new rc.

[Jakuje]

I am sorry for delay. I finally built the latest snapshot, but I am hitting some other issues with cyrus-sasl in the tests so I will have to investigate it further what is going on.

[Jakuje]

Unfortunately I can confirm that the old version we have in Fedora works fine against ADs, but not the latest rc7 (unless I revert the commit 080e51c).

I can only say that the committed change did not fix the problem we encountered.

I will try to investigate more what went wrong if I will be able to figure out more, but I can't promise anything. Unfortunately, the test case itself would not be helpful since it depends on internal AD servers we test against. But the test does not involve anything more complicated than a ldapsearch:

ldapsearch -Y GSSAPI -H ldap://sec-ad2.example.com -b dc=ad2,dc=baseos,dc=qe CN=Amy

The connection does not work regardless the -O maxssf= argument with the rc7 release.

[quanah]

@Jakuje Thanks for the confirmation that this problem is not fixed, as suspected.

[Jakuje]

Sorry, I had partially bad look, but the result is that it does not work flawlessly "as before". The test is running various maxssf values against various servers (IPA, AD), probably with different versions. I would have to look into that further

For IPA2 server (probably RHEL7?), it works flawlessly

All the others i see failures with maxssf=0 and maxssf=4294967296 (bogus large enough value?). The error now looks this way:

SASL/GSSAPI authentication started
ldap_sasl_interactive_bind_s: Local error (-2)
        additional info: SASL(-1): generic failure: GSSAPI Error: A required input parameter could not be read (Unknown error)

The tested values maxssf=1 and maxssf=56 work fine in all the cases that I tested.

I am not completely sure how to interpret these results, since it is change from previous versions, but "reasonable" values seems to work. What do you think?

[JanParcel]

On 2/13/2018 9:39 AM, Jakub Jelen wrote: Sorry, I had partially bad look, but the result is that it does not work flawlessly "as before". The test is running various maxssf values against various servers (IPA, AD), probably with different versions. I would have to look into that further For IPA2 server (probably RHEL7?), it works flawlessly All the others i see failures with |maxssf=0| and |maxssf=4294967296| (bogus large enough value?). The error now looks this way: |SASL/GSSAPI authentication started ldap_sasl_interactive_bind_s: Local error (-2) additional info: SASL(-1): generic failure: GSSAPI Error: A required input parameter could not be read (Unknown error) | The tested values |maxssf=1| and |maxssf=56| work fine in all the cases that I tested. I am not completely sure how to interpret these results, since it is change from previous versions, but "reasonable" values seems to work. What do you think?

I would expect a check of the value and an explicit error message saying what the limit is and that it exceeded that limit.

…

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <#419 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AWZUI0QfjXE84Jj-HFT2aH03gBv4WgDlks5tUcjlgaJpZM4L-S_F>.

-- Jan Parcel, Software Developer Oracle Systems Server & Cloud Engineering

[Jakuje]

With maxssf=0 I see the cyrus-sasl code invoked, gssapi_client_mech_step() called, but this error comes from plugins/gssapi.c, which fails:

1932 maj_stat = gss_unwrap(&min_stat,
1940 if (GSS_ERROR(maj_stat)) {
(gdb) p maj_stat
$1 = 16777216 (GSS_S_CALL_INACCESSIBLE_READ?)

I did not go deeper into the gssapi code to investigate further. I have the test system if you would have some ideas what more I can try.

[quanah]

See the response in #433 (comment)

[Jakuje]

So let's revert this change finally, and do a release? Or is there any more feedback needed?

Or shall we also work on addressing the referenced FIX ME?

[quanah]

This can be closed now, right @ksmurchison ? Since 2.1.27 has released with your commit. ;)

[michael-o]

@ksmurchison Just stumbled upon this. With e41cfb9 reverting 080e51c

https://github.com/cyrusimap/cyrus-sasl/blob/master/plugins/gssapi.c#L1762-L1774

does not comply with RFC 4752, section 3.1:

The client calls GSS_Init_sec_context, passing in
input_context_handle of 0 (initially), mech_type of the Kerberos V5
GSS-API mechanism [KRB5GSS], chan_binding of NULL, and targ_name
equal to output_name from GSS_Import_Name called with input_name_type
of GSS_C_NT_HOSTBASED_SERVICE (*) and input_name_string of
"service@hostname" where "service" is the service name specified in
the protocol's profile, and "hostname" is the fully qualified host
name of the server.  When calling the GSS_Init_sec_context, the
client MUST pass the integ_req_flag of TRUE (**).  If the client will
be requesting a security layer, it MUST also supply to the
GSS_Init_sec_context a mutual_req_flag of TRUE, and a
sequence_req_flag of TRUE.  If the client will be requesting a
security layer providing confidentiality protection, it MUST also
supply to the GSS_Init_sec_context a conf_req_flag of TRUE.

anymore.

[quanah]

@michael-o Please read the full discussion. This was already referenced in the ticket, but again:

https://lists.andrew.cmu.edu/pipermail/cyrus-sasl/2015-April/002809.html

This was also discussed directly with the Heimdal Kerberos team and Ken Hornstein at the time this was being reviewed to confirm that this code change does in fact comply with the RFC.

[michael-o]

@quanah I have read the entire thread again, all linked tickets as well as Ken Hornstein's mail. I do not understand his argumentation, it does not necessary mean that it is wrong, but it lacks description why his interpretation (!) of the RFC is not wrong. From a pure POV, leaving external_ssf aside, I'd say that the revert is wrong and @aamelnikov is right. The code does not comply with the RFC anymore. Going further reading the code one might argue that if external (TLS) has established int of conf it does not require Kerebros to do so anymore. If that is the given interpretation at the moment, committers really need to document the code for this.
It would also be very helpful to add/write:

$ ldapsearch -H ldaps://ad001.siemens.net   -O "maxssf=256"
SASL/GSSAPI authentication started
SASL username: osipovmi@AD001.SIEMENS.NET
SASL SSF: 0
SASL external SSF: 256
# extended LDIF
...

Note the external SSF.

[quanah]

@michael-o Please see #433 (comment) as well.

[michael-o]

@quanah I have read that one too. I do not fully understand what you are trying to say with that. If we put aside that SASL must check flags after the context is established, I only partially agree that int and conf are hints to GSS-API, but not to Kerberos. If you don't set that hint the mech below will never see it. So please explain what you are trying to say. I really want to understand!

[hyc]

Just to comment - RFC4752 is a bit strange here. In 3.3 we see there are only 3 defined flags on the wire:

The security layers and their corresponding bit-masks are as follows:

      1 No security layer
      2 Integrity protection.
        Sender calls GSS_Wrap with conf_flag set to FALSE
      4 Confidentiality protection.
        Sender calls GSS_Wrap with conf_flag set to TRUE

and yet the API has 4 flags

When calling the GSS_Init_sec_context, the
client MUST pass the integ_req_flag of TRUE (**).  If the client will
be requesting a security layer, it MUST also supply to the
GSS_Init_sec_context a mutual_req_flag of TRUE, and a
sequence_req_flag of TRUE.  If the client will be requesting a
security layer providing confidentiality protection, it MUST also
supply to the GSS_Init_sec_context a conf_req_flag of TRUE.

And the mutual auth and sequence req flags don't correspond to anything in the on-the-wire
flags at all. And it's counter-intuitive to have "no security layer" equal to bit 1 (I would think all-flags-zero would have been more straightforward).

At any rate, it is clear that the original intent was to allow the authentication to proceed without using integrity checks (otherwise no bitflag for integrity would be needed on the wire) so the section 3.1 text requiring integ_req_flag to always be TRUE must be erroneous. It does not appear that this has been noted in any official errata yet. https://www.rfc-editor.org/errata/rfc4752

The on-the-wire flags don't really make sense as defined; the intent is that you can specify no security layer, integrity-check-only, or integrity+confidentiality. I.e., this should be a two-bit enum, not a bitmask at all. Using a bitmask indicates that integrity and confidentiality may be selected independently but that's not in fact what is intended.

[michael-o]

@hyc mutual and out of sequence detection do not require any wrapping/unwrapping. You still have three: none, auth-int and auth-conf (implies auth-int).

[hyc]

@michael-o sorry, edited my comment further, please see updated comment.

[hyc]

Also note that the previous version of the RFC, RFC2222 section 7.2.1 didn't give any requirements on flags to use. Anyway, since the mechanisms are specifically intended to allow operating without an integrity layer, I'll reiterate that this part of RFC4752 section 3.1 is simply wrong and should be disregarded.

[michael-o]

@hyc If you think they are wrong, discuss this with Alexey Melnikov and request and updated RFC.

[hyc]

@aamelnikov has participated on this ticket, he should already have been pinged.

[michael-o]

@hyc I don't see how the RFC implies a bitmask. For me it is an enum. either or. At some point in time Microsoft's SASL implementation interpreted this as a bitmask and required both bits int and conf to be set. I remember patching SASL for this for a colleague to avoid event on the domain controller. I think this issue has been resolved in never Windows Server versions.

[hyc]

@michael-o Don't waste my time. The RFC says explicitly "The security layers and their corresponding bit-masks are as follows:" - it is explicit, not implied.

[michael-o]

And you believe it makes sense to set off and auth-conf atvthe same time? That's nonsense. It is just bad wording requiring some errata.

[hyc]

@michael-o Now you're just repeating what I already said. #419 (comment)

But it is not merely bad wording - this is the description of the on-the-wire format, which means any changes made here will break compatibility with any existing software. You have demonstrated that you're unable to read carefully enough to follow this conversation, and unable to reason well enough to participate. Please go away.

[michael-o]

@hyc Up, up, and away I am.