Add SPNEGO special case for NTLMSSP+MechListMIC

Bug #1643708 reported by Joshua R. Poulson
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
krb5 (Ubuntu)
Fix Released
Undecided
Unassigned
Trusty
Fix Released
Undecided
Unassigned
Xenial
Fix Released
Undecided
Unassigned
Yakkety
Fix Released
Undecided
Unassigned

Bug Description

[Impact]
MS-SPNG section 3.3.5.1 documents an odd behavior the SPNEGO layer
needs to implement specifically for the NTLMSSP mechanism. This is
required for compatibility with Windows services.

Upstream commit: https://github.com/krb5/krb5/commit/cb96ca52a3354e5a0ea52e12495ff375de54f9b7

We've run into this issue with Linux to Windows negotiation with encrypted http using GSSAPI.

[Test Case]

create a file with some credentials:

$ echo F23:guest:guest > ~/ntlmcreds.txt
$ export NTLM_USER_FILE=~/ntlmcreds.txt
$ python
import gssapi

spnego = gssapi.raw.oids.OID.from_int_seq('1.3.6.1.5.5.2')
c = gssapi.creds.Credentials(mechs=[spnego], usage='initiate')
tname = gssapi.raw.names.import_name("F23/server", name_type=gssapi.raw.types.NameType.hostbased_service)
ac = gssapi.creds.Credentials(mechs=[spnego], usage='accept')

seci = gssapi.SecurityContext(creds=c, name=tname, mech=spnego, usage='initiate')
seca = gssapi.SecurityContext(creds=ac, usage='accept')
it = seci.step(token=None)
ot = seca.step(token=it)
it = seci.step(token=ot)
ot = seca.step(token=it)
it = seci.step(token=ot)

e = seci.wrap("Secrets", True)
o = seca.unwrap(e.message)

o.message
'Secrets'

Revision history for this message
Joshua R. Poulson (jrp) wrote :

This is needed for Ubuntu 14.04, 16.04, and 16.10.

Revision history for this message
Steve Langasek (vorlon) wrote :

Hi Joshua,

To SRU this into stable releases, we would need a test case for the problem. Can you provide an easy-to-follow reproducer that lets us verify this bug?

Revision history for this message
Steve Langasek (vorlon) wrote :

This patch is included in krb5 1.15~beta1-1, which is in zesty-proposed currently and should reach zesty soon.

Changed in krb5 (Ubuntu):
status: New → Fix Released
Changed in krb5 (Ubuntu Trusty):
status: New → Incomplete
Changed in krb5 (Ubuntu Xenial):
status: New → Incomplete
Changed in krb5 (Ubuntu Yakkety):
status: New → Incomplete
Revision history for this message
Robie Basak (racb) wrote :

This bug is blocked in the SRU process because it has no SRU information. Please see https://wiki.ubuntu.com/StableReleaseUpdates#Procedure

Revision history for this message
Joshua R. Poulson (jrp) wrote :

The test case in https://github.com/krb5/krb5/pull/436#issuecomment-209017277 is the best we can offer without going through the trouble of setting up OMI. We are happy to test any preliminary builds, or -proposed, as necessary.

Revision history for this message
Robie Basak (racb) wrote :

> The test case in https://github.com/krb5/krb5/pull/436#issuecomment-209017277 is the best we can offer without going through the trouble of setting up OMI.

This is fine, but it should be explained in this bug so people processing this update do not get confused. Please follow https://wiki.ubuntu.com/StableReleaseUpdates#Procedure step 3, which is not yet complete. If you can't edit the bug description, adding that information in a comment is fine.

Revision history for this message
Joshua R. Poulson (jrp) wrote :

[Test Case]

create a file with some credentials:

$ echo F23:guest:guest > ~/ntlmcreds.txt
$ export NTLM_USER_FILE=~/ntlmcreds.txt
$ python
import gssapi

spnego = gssapi.raw.oids.OID.from_int_seq('1.3.6.1.5.5.2')
c = gssapi.creds.Credentials(mechs=[spnego], usage='initiate')
tname = gssapi.raw.names.import_name("F23/server", name_type=gssapi.raw.types.NameType.hostbased_service)
ac = gssapi.creds.Credentials(mechs=[spnego], usage='accept')

seci = gssapi.SecurityContext(creds=c, name=tname, mech=spnego, usage='initiate')
seca = gssapi.SecurityContext(creds=ac, usage='accept')
it = seci.step(token=None)
ot = seca.step(token=it)
it = seci.step(token=ot)
ot = seca.step(token=it)
it = seci.step(token=ot)

e = seci.wrap("Secrets", True)
o = seca.unwrap(e.message)

o.message
'Secrets'

So far I checked with GDB that the seq numbers were reset after the MEchListMIC operation happened and started again from 0.

Revision history for this message
Timo Aaltonen (tjaalton) wrote : Please test proposed package

Hello Joshua, or anyone else affected,

Accepted krb5 into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/krb5/1.13.2+dfsg-5ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

description: updated
Changed in krb5 (Ubuntu Xenial):
status: Incomplete → Fix Committed
tags: added: verification-needed
Changed in krb5 (Ubuntu Trusty):
status: Incomplete → Fix Committed
Revision history for this message
Timo Aaltonen (tjaalton) wrote :

Hello Joshua, or anyone else affected,

Accepted krb5 into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/krb5/1.12+dfsg-2ubuntu5.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in krb5 (Ubuntu Yakkety):
status: Incomplete → Fix Committed
Revision history for this message
Timo Aaltonen (tjaalton) wrote :

Hello Joshua, or anyone else affected,

Accepted krb5 into yakkety-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/krb5/1.14.3+dfsg-2ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Revision history for this message
Bruce Campbell (yakman2020) wrote : Re: [Bug 1643708] Please test proposed package

I set up yakkety with the proposed packages and built omi, and connected
propertly with win10 encrypted. This had been failing.

The patch appears to work.

On Fri, Dec 16, 2016 at 2:22 AM, Timo Aaltonen <email address hidden> wrote:

> Hello Joshua, or anyone else affected,
>
> Accepted krb5 into yakkety-proposed. The package will build now and be
> available at https://launchpad.net/ubuntu/+source/krb5/1.14.3+dfsg-
> 2ubuntu1 in a few hours, and then in the -proposed repository.
>
> Please help us by testing this new package. See
> https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how
> to enable and use -proposed.Your feedback will aid us getting this
> update out to other Ubuntu users.
>
> If this package fixes the bug for you, please add a comment to this bug,
> mentioning the version of the package you tested, and change the tag
> from verification-needed to verification-done. If it does not fix the
> bug for you, please add a comment stating that, and change the tag to
> verification-failed. In either case, details of your testing will help
> us make a better decision.
>
> Further information regarding the verification process can be found at
> https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in
> advance!
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1643708
>
> Title:
> Add SPNEGO special case for NTLMSSP+MechListMIC
>
> Status in krb5 package in Ubuntu:
> Fix Released
> Status in krb5 source package in Trusty:
> Fix Committed
> Status in krb5 source package in Xenial:
> Fix Committed
> Status in krb5 source package in Yakkety:
> Fix Committed
>
> Bug description:
> [Impact]
> MS-SPNG section 3.3.5.1 documents an odd behavior the SPNEGO layer
> needs to implement specifically for the NTLMSSP mechanism. This is
> required for compatibility with Windows services.
>
> Upstream commit:
> https://github.com/krb5/krb5/commit/cb96ca52a3354e5a0ea52e12495ff3
> 75de54f9b7
>
> We've run into this issue with Linux to Windows negotiation with
> encrypted http using GSSAPI.
>
> [Test Case]
>
> create a file with some credentials:
>
> $ echo F23:guest:guest > ~/ntlmcreds.txt
> $ export NTLM_USER_FILE=~/ntlmcreds.txt
> $ python
> import gssapi
>
> spnego = gssapi.raw.oids.OID.from_int_seq('1.3.6.1.5.5.2')
> c = gssapi.creds.Credentials(mechs=[spnego], usage='initiate')
> tname = gssapi.raw.names.import_name("F23/server",
> name_type=gssapi.raw.types.NameType.hostbased_service)
> ac = gssapi.creds.Credentials(mechs=[spnego], usage='accept')
>
> seci = gssapi.SecurityContext(creds=c, name=tname, mech=spnego,
> usage='initiate')
> seca = gssapi.SecurityContext(creds=ac, usage='accept')
> it = seci.step(token=None)
> ot = seca.step(token=it)
> it = seci.step(token=ot)
> ot = seca.step(token=it)
> it = seci.step(token=ot)
>
> e = seci.wrap("Secrets", True)
> o = seca.unwrap(e.message)
>
> o.message
> 'Secrets'
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/ubuntu/+source/krb5/+bug/1643708/+subscriptions
>

Revision history for this message
Joshua R. Poulson (jrp) wrote :

This change in proposed corrects part of what is needed and makes the Windows to Linux part of our scenario work. We will file a new bug to pick up a change from gssntlmssp-0.6.0 to 0.7.0 to correct a sequence numbering mismatch we've observed after applying this fix.

tags: added: verification-done
removed: verification-needed
Revision history for this message
Joshua R. Poulson (jrp) wrote :
Revision history for this message
Robie Basak (racb) wrote :

<rbasak> tjaalton: looking at bug 1643708, I'm concerned that there's nothing in the test plan to make sure that existing users of the SPNEGO are not broken, so I'm not comfortable releasing it even though it is verification-done now. As an aside the reporter possibly hasn't checked all three releases. Could you take care of it please?

Revision history for this message
Robie Basak (racb) wrote :

<rbasak> tjaalton: there's no Regression Potential consideration in that bug either.

Revision history for this message
Timo Aaltonen (tjaalton) wrote :

Did you test xenial and trusty too? While the patch looks fine, did you test for regressing other use cases?

Revision history for this message
Joshua R. Poulson (jrp) wrote :

We tested Xenial as well. Trusty is missing some other required packages.

Revision history for this message
Joshua R. Poulson (jrp) wrote :

Bruce is saying that he tested Trusty as well.

Revision history for this message
Bruce Campbell (yakman2020) wrote :

We have a considerable regression suite for testing normal and not so normal gssapi functions as part of exercising omi and powershell. We have been running that quite often, as passing it is required for checkin. In addition we have been using it a fair amount in more informal testing.

I built a custom gss-ntlmssp-0.7.0 deb for trusty from the sources from the page on fedorahosted and the package info for the xenial .deb. The setup on trusty is a little weird (/usr/etc/gss/mech rather than /etc/gss/mech.d/ntlmssp.conf for example) but it has represented no problems with that modification.

With the packages, everything works a treat. Without we are unable to encrypt traffic using NTLMv2 and have it understood by windows.

Revision history for this message
Robie Basak (racb) wrote :

@Bruce

Thank you for detailing your testing. In your test suite, do you cover any interoperability with SPNEGO but not-Windows, whether in integration or code path coverage? That's the use case I'm concerned about - that someone will come along and tell us that we regressed SPNEGO against WebSphere or something because we focused on just testing Windows.

Revision history for this message
Sam Hartman (hartmans) wrote : Re: [Bug 1643708] Re: Add SPNEGO special case for NTLMSSP+MechListMIC

>>>>> "Robie" == Robie Basak <email address hidden> writes:

    Robie> @Bruce Thank you for detailing your testing. In your test
    Robie> suite, do you cover any interoperability with SPNEGO but
    Robie> not-Windows, whether in integration or code path coverage?
    Robie> That's the use case I'm concerned about - that someone will
    Robie> come along and tell us that we regressed SPNEGO against
    Robie> WebSphere or something because we focused on just testing
    Robie> Windows.

Hi.
As I understand it, this is a backport of an upstream change.
It's always possible there is an interop regression.
In this instance though, given where the patch comes from originally,
and that it's been in upstream releases for a while, I think you're
relatively safe.
SPNEGO interop is really hard to test though; it's not something that
you can get good coverage for without a specific interoperability lab
and careful test plans.

I don't know if upstream has done that for this patch, although I do
have high confidence that people do interop tests against the upstream
version.

So, while I think your concern is reasonable, I'd urge you to consider
that you're setting a really high bar here for backporting a patch that
an interoperability-conscious upstream has vetted.
Yes, the MIT folks have messed up interop (just as everyone else), but
they are fairly careful and conservative.

If you do want to do interop testing, the interesting cases to cover
are:

* Initiator prefers Kerberos; other side does not support it

* Acceptor prefers Kerberos, initiator does not support it

* Initiator prefers NTLM and some non-Kerberos third mechanism

* Acceptor prefers NTLM, doesn't have Kerberos, but does have some third
  mechanism

I think setting all that up is a good week's worth of work with someone
who really knows what they are doing.

--Sam

Revision history for this message
Robie Basak (racb) wrote :

Hi Sam,

Thank you for weighing in on this. I appreciate your opinion.

On Fri, Jan 20, 2017 at 02:03:11PM -0000, Sam Hartman wrote:
> So, while I think your concern is reasonable, I'd urge you to consider
> that you're setting a really high bar here for backporting a patch that
> an interoperability-conscious upstream has vetted.

I'm more bothered that we've considered and weighed up the non-Windows
use case. I hadn't yet set a bar - I was just asking for regressions in
all use cases to be considered and that consideration documented, as is
SRU policy. This helps us reach a decision, and should ideally have
happened first, before the SRU was accepted into the proposed pocket.

I'm convinced by your arguments, so I'm happy with the testing already
performed. Thank you to Bruce and Joshua for your work on this.

We don't usually release SRUs on Fridays in case of regression, but I'd
be happy to release this on Monday, subject to the usual checks.

Robie

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package krb5 - 1.14.3+dfsg-2ubuntu1

---------------
krb5 (1.14.3+dfsg-2ubuntu1) yakkety; urgency=medium

  * d/p/upstream/0001-Add-SPNEGO-special-case-for-NTLMSSP-MechListMIC.patch:
    Cherry-pick from upstream to add SPNEGO special case for
    NTLMSSP+MechListMIC. LP: #1643708.

 -- Steve Langasek <email address hidden> Mon, 21 Nov 2016 17:01:33 -0800

Changed in krb5 (Ubuntu Yakkety):
status: Fix Committed → Fix Released
Revision history for this message
Andy Whitcroft (apw) wrote : Update Released

The verification of the Stable Release Update for krb5 has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package krb5 - 1.13.2+dfsg-5ubuntu1

---------------
krb5 (1.13.2+dfsg-5ubuntu1) xenial; urgency=medium

  * d/p/upstream/0001-Add-SPNEGO-special-case-for-NTLMSSP-MechListMIC.patch:
    Cherry-pick from upstream to add SPNEGO special case for
    NTLMSSP+MechListMIC. LP: #1643708.

 -- Steve Langasek <email address hidden> Mon, 21 Nov 2016 17:28:15 -0800

Changed in krb5 (Ubuntu Xenial):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package krb5 - 1.12+dfsg-2ubuntu5.3

---------------
krb5 (1.12+dfsg-2ubuntu5.3) trusty; urgency=medium

  * d/p/upstream/0001-Add-SPNEGO-special-case-for-NTLMSSP-MechListMIC.patch:
    Cherry-pick from upstream to add SPNEGO special case for
    NTLMSSP+MechListMIC. LP: #1643708.

 -- Steve Langasek <email address hidden> Mon, 21 Nov 2016 18:14:47 -0800

Changed in krb5 (Ubuntu Trusty):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.