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

NegotiateAuthentication.ComputeIntegrityCheck on Windows sets an invalid QOP #103461

Closed
jborean93 opened this issue Jun 14, 2024 · 9 comments · Fixed by #105605
Closed

NegotiateAuthentication.ComputeIntegrityCheck on Windows sets an invalid QOP #103461

jborean93 opened this issue Jun 14, 2024 · 9 comments · Fixed by #105605
Labels
area-System.Net.Security in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@jborean93
Copy link
Contributor

jborean93 commented Jun 14, 2024

Description

When using NegotiateAuthentication.ComputeIntegrityCheck on Windows with a context with Sign and not EncryptAndSign, it calls MakeSignature with the QoP value of SECQOP_WRAP_NO_ENCRYPT. This fails with SEC_E_INVALID_TOKEN because as far as I know this QoP setting is for EncryptMessage and not MakeSignature.

Reproduction Steps

using NegotiateAuthentication client = new(new NegotiateAuthenticationClientOptions()
{
    TargetName = "test/dotnet",
    Package = "Kerberos",
    RequiredProtectionLevel = Enum.Parse<ProtectionLevel>(args[0], true),
    RequireMutualAuthentication = true,
});
using NegotiateAuthentication server = new(new NegotiateAuthenticationServerOptions()
{
    Package = "Kerberos",
});

byte[]? clientOut = client.GetOutgoingBlob(Array.Empty<byte>(), out var clientStatus);
byte[]? serverOut = server.GetOutgoingBlob(clientOut, out var serverStatus);
client.GetOutgoingBlob(serverOut, out clientStatus);

Span<byte> toSign = [0];
ArrayBufferWriter<byte> signatureWriter = new();
client.ComputeIntegrityCheck(toSign, signatureWriter);

Console.WriteLine($"Done: {Convert.ToBase64String(signatureWriter.WrittenSpan)}");

In this case I'm running as a domain user that has added test/dotnet as a servicePrincipalName to its AD account. This allows me to replicate Kerberos authentication in the same process when running as that user.

Expected behavior

$ dotnet run EncryptAndSign

Done: BAQE//////8AAAAAO4pNAKis8sZkw/46ZtDrzQ==

Signature is generated

Actual behavior

$ dotnet run Sign

Unhandled exception. System.ComponentModel.Win32Exception (0x80090308): The token supplied to the function is invalid
   at System.Net.NegotiateAuthenticationPal.WindowsNegotiateAuthenticationPal.GetMIC(ReadOnlySpan`1 message, IBufferWriter`1 signature)
   at SigTest.Program.Main(String[] args) in c:\temp\SigTest\Program.cs:line 29

Exception, MakeSignature fails with SEC_E_INVALID_TOKEN.

Regression?

No

Known Workarounds

Ensure you set the NegotiateAuthenticationClientOptions.RequiredProtectionLevel to EncryptAndSign and not Sign. I don't believe this would cause an issue except that it would just request extra capabilities but I don't believe you should have to request confidentiality if all you need is integrity.

Configuration

.NET Version - 9.0.100-preview.5.24307.3
OS - Windows Server 2022
Arch - x64

I don't believe this is specific to the OS or arch version. I believe SSPI doesn't accept any QoP values for MakeSignature, at least for the builtin authentication providers.

Other information

No response

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jun 14, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

@jborean93
Copy link
Contributor Author

@filipnavara this is the issue I was talking about earlier. All good if you just want to close this but I thought I would report it in any case.

@filipnavara
Copy link
Member

cc @SteveSyfuhs for advice on using the API correctly. We lack test coverage for this on Windows, unfortunately.

@rzikm rzikm added this to the Future milestone Jun 14, 2024
@rzikm rzikm removed the untriaged New issue has not been triaged by the area owner label Jun 14, 2024
@SteveSyfuhs
Copy link

Well, we sort of support QOP for MakeSignature, but it's relatively limited. The offending call eventually hits this paraphrased block:

if (QualityOfProtection == KERB_WRAP_NO_ENCRYPT)
{
    if (!Encrypt)
    {
        return STATUS_INVALID_PARAMETER;
    }

    Signature->SealAlgorithm[0] = KERB_GSS_NO_SEAL;
    Signature->SealAlgorithm[1] = KERB_GSS_NO_SEAL_SECOND;
}

I don't have a better answer on what is and isn't supported and by extension what the best way to call this is. Mixing and matching encrypt and sign is always a complex mess so I recommend staying continuous and always encrypting and signing everything, unless you must interop with something in particular that's doing weird things.

@filipnavara
Copy link
Member

Thanks, that implies we should change these lines:

uint qop = IsEncrypted ? 0 : Interop.SspiCli.SECQOP_WRAP_NO_ENCRYPT;
int errorCode = Interop.SspiCli.MakeSignature(ref _securityContext._handle, qop, ref sdcInOut, 0);

to just:

                        int errorCode = Interop.SspiCli.MakeSignature(ref _securityContext._handle, 0, ref sdcInOut, 0);

@wfurt
Copy link
Member

wfurt commented Jun 14, 2024

How does it interoperate with Linux? e.g. would it produce same result?
And do we need switch for this @filipnavara?

@filipnavara
Copy link
Member

How does it interoperate with Linux? e.g. would it produce same result?

I assume it would. We already specify GSS_C_QOP_DEFAULT (0) on Linux:

uint32_t majorStatus =
gss_get_mic(minorStatus, contextHandle, GSS_C_QOP_DEFAULT, &inputMessageBuffer, &gssBuffer);

Last night I tracked down the flags we pass on Windows to this article: https://learn.microsoft.com/en-us/windows/win32/secauthn/sspi-kerberos-interoperability-with-gssapi

It was likely done under the assumption that EncryptMessage and MakeSignature behave the same w.r.t. the QOP flag.

And do we need switch for this @filipnavara?

No. This should be a simple two line fix.

@jborean93
Copy link
Contributor Author

How does it interoperate with Linux? e.g. would it produce same result?

Just as some background, I came across this when trying to implement support for gssapi-with-mic on an SSH library. I can confirm that a qop of 0 still produces the correct signature thag GSSAPI on the ssh service is able to validate it properly.

jborean93 added a commit to jborean93/runtime that referenced this issue Jul 28, 2024
Fix calling NegotiateAuthentication.ComputeIntegrityCheck on SSPI when
the negotiation context was built with ProtectionLevel.Sign. The
SECQOP_WRAP_NO_ENCRYPT QoP flag should not be set when calling GetMIC as
no encryption is involved and some authentication providers fail when
this is set.

Fix dotnet#103461
@jborean93
Copy link
Contributor Author

I've opened #105605 which remove the QoP flag and always uses 0 for the GetMIC call.

@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Aug 19, 2024
@rzikm rzikm closed this as completed in 3622bfa Aug 20, 2024
@karelz karelz modified the milestones: Future, 9.0.0 Sep 3, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 4, 2024
mikelle-rogers pushed a commit to mikelle-rogers/runtime that referenced this issue Dec 10, 2024
Fix calling NegotiateAuthentication.ComputeIntegrityCheck on SSPI when
the negotiation context was built with ProtectionLevel.Sign. The
SECQOP_WRAP_NO_ENCRYPT QoP flag should not be set when calling GetMIC as
no encryption is involved and some authentication providers fail when
this is set.

Fix dotnet#103461

Co-authored-by: Tomas Weinfurt <tweinfurt@yahoo.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Security in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants