Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use GSS-SPNEGO if connecting locally #547

Closed
wants to merge 1 commit into from
Closed

Conversation

simo5
Copy link
Contributor

@simo5 simo5 commented Mar 7, 2017

GSS-SPNEGO allows us to negotiate a SASL bind with less roundtrips
therefore use it when possible.

We only enable it for local connections for now because we only
recently fixed Cyrus SASL to do proper GSS-SPNEGO negotiation. This
change means a newer and an older version are not compatible.

Restricting ourselves to the local host prevents issues with
incompatible services, and it is ok for us as we are only really
looking for speedups for the local short-lived connections performed
by the framework. Most other clients have longer lived connections,
so peformance improvements there are not as important.

GSS-SPNEGO allows us to negotiate a SASL bind with less roundtrips
therefore use it when possible.

We only enable it for local connections for now because we only
recently fixed Cyrus SASL to do proper GSS-SPNEGO negotiation. This
change means a newer and an older version are not compatible.

Restricting ourselves to the local host prevents issues with
incompatible services, and it is ok for us as we are only really
looking for speedups for the local short-lived connections performed
by the framework. Most other clients have longer lived connections,
so peformance improvements there are not as important.

Ticket: https://pagure.io/freeipa/issue/6656

Signed-off-by: Simo Sorce <simo@redhat.com>
@abbra
Copy link
Contributor

abbra commented Mar 7, 2017

LGTM but I think we should also update Requires: in the spec file to use cyrus-sasl-2.1.26-29.fc26 or later.

@simo5
Copy link
Contributor Author

simo5 commented Mar 7, 2017

We actually do not need to put a strong require, this patch will work regardless, but won't provide any performance advantage on older versions.

You will add a stronger require when the GC work is done, so we can defer to that point to add it.

@tkrizek
Copy link
Contributor

tkrizek commented Mar 7, 2017

The patch works with both cyrus-sasl-2.1.26-26.2.fc24 and cyrus-sasl-2.1.26-29.fc26.

Since the newer version is not a hard dependency, we can add it later on, as @simo5 suggested.

@tkrizek tkrizek added ack Pull Request approved, can be merged pushed Pull Request has already been pushed labels Mar 7, 2017
@tkrizek
Copy link
Contributor

tkrizek commented Mar 7, 2017

master:

  • adf8aab Use GSS-SPNEGO if connecting locally

@tkrizek tkrizek closed this Mar 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
3 participants