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

[API Proposal]: Expose SessionKey on NegotiateAuthentication #111099

Open
jborean93 opened this issue Jan 5, 2025 · 14 comments
Open

[API Proposal]: Expose SessionKey on NegotiateAuthentication #111099

jborean93 opened this issue Jan 5, 2025 · 14 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Security
Milestone

Comments

@jborean93
Copy link
Contributor

jborean93 commented Jan 5, 2025

Background and motivation

The NegotiateAuthentication class is a nice manager wrapper around SSPI/GSSAPI and deals with all the API differences and platform quirks to be able to authenticate using Negotiate/Kerberos/NTLM auth and also sign/seal messages after authentication. Some protocols do not use the builtin signing/sealing mechanisms that is done through SSPI/GSSAPI but instead uses the GSSAPI "session key". An example of this is SMB where after authenticating it derives the encryption/signing key from that key https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-smb2/7fd079ca-17e6-4f02-8449-46b606ea289c

Session.SessionKey MUST be set to the first 16 bytes of the cryptographic key queried from the GSS protocol for this authenticated context. If the cryptographic key is less than 16 bytes, it is right-padded with zero bytes. If Connection.Dialect is “3.1.1” and Connection.CipherId is AES-256-CCM or AES-256-GCM, Session.FullSessionKey MUST be set to the cryptographic key as queried from the GSS protocol for this authenticated context. For information about how this is calculated for Kerberos authentication using Generic Security Service Application Programming Interface (GSS-API), see [MS-KILE] section 3.1.1.2. For information about how this is calculated for NTLM authentication using GSS-API, see [MS-NLMP] section 3.1.5.1.

If I want to implement my own SMB, or other protocol, implementation that uses this key in .NET I cannot use NegotiateAuthentication because it doesn't expose any public way of retrieving the session key. I'll have to setup my own PInvokes and call GSSAPI/SSPI myself.

To retrieve this key on SSPI you would call QueryContextAttributesW with ulAttribute being SECPKG_ATTR_SESSION_KEY (9) and a pBuffer being a pointer to SecPkgContext_SessionKey. Once copied or no longer needed the SecPkgContext_SessionKey.SessionKey is passed to FreeContextBuffer to free the memory.

To retrieve this key on GSSAPI you would call gss_inquire_sec_context_by_oid with the OID GSS_C_INQ_SSPI_SESSION_KEY (1.2.840.113554.1.2.2.5.5). Once copied or no longer needed the data_set is passed to gss_release_buffer_set to free the memory. While gss_inquire_sec_context_by_oid is an extension method it has been part of the three major GSSAPI implementations since:

  • MIT krb5 - since 1.7 in 2009
  • Heimdal - since 1.5 in 2011
  • GSS.Framework (macOS) - See above, was forked after Heimdal 1.5

API Proposal

public partial class NegotiateAuthentication
{
    // Original API here for historical purposes
    // public byte[] GetSessionKey();

    public void DeriveKeyFromSessionKey(Action<ReadOnlySpan<byte>> keyDerivationFunction);
    public void DeriveKeyFromSessionKey<TState>(Action<ReadOnlySpan<byte>, TState> keyDerivationFunction, TState state);
    public TReturn DeriveKeyFromSessionKey<TReturn>(Func<ReadOnlySpan<byte>, TReturn> keyDerivationFunction);
    public TReturn DeriveKeyFromSessionKey<TState, TReturn>(Func<ReadOnlySpan<byte>, TState, TReturn> keyDerivationFunction, TState state);
}

API Usage

var a = new NegotiateAuthentication(new NegotiateAuthenticationClientOptions());

// Perform authentication
while (!a.IsAuthenticated)
{
    a.GetOutgoingBlob(...);
    ...
}

// After authentication, retrieve session key
byte[] sessionKey = a.GetSessionKey();

// Derive signing key from session key based on MS-SMB2
Span<byte> signingKey = stackallock byte[16];
using SP800108HmacCounterKdf smb3kdf = new(key, HashAlgorithmName.SHA256);
smb3kdf.DeriveKey(
    "SMBSigningKey\0"u8,
    preauthHash,  // Hash of preceding SMB messages
    signingKey);

Alternative Designs

A potential alternative is to have the caller pass in a Span<byte> and instead of returning the byte[] the code copies the session key to the provided span if large enough.

public partial class NegotiateAuthentication
{
    public void GetSessionKey(Span<byte> destination);
    public bool TryGetSessionKey(Span<byte> destination, out int bytesWritten);
}

A downside to this approach is that I'm unsure if the key size can be queried from GSSAPI/SSPI and that the sizes may change depending on the environment/protocol used. For example NTLM will always stay at 16 bytes but Kerberos is based on the etype used, i.e 32 bytes for an AES256 based etype but 16 bytes for an AES128 based etype.

Risks

The main risk is really just expanding the support area of SSPI/GSSAPI. The documentation should stress that this key is not normally meant to be used directly and is exposed for compatibility with implementations that require this key like SMB.

@jborean93 jborean93 added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 5, 2025
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jan 5, 2025
Copy link
Contributor

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

Copy link
Contributor

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

@filipnavara
Copy link
Member

The request generally sounds reasonable.

In the managed NTLM implementation we have the key calculated as exportedSessionKey here. We do not keep a copy around at the moment so a little refactoring would be necessary.

@jborean93
Copy link
Contributor Author

@filipnavara do you have any thoughts as to whether it should a byte[] or use the Span<byte> destination example? I like the idea if avoiding an extra copy but not having a way to know the size of the key beforehand can potentially be problematic.

@filipnavara
Copy link
Member

filipnavara commented Jan 7, 2025

I generally prefer Spans. We already have some prior art in NegotiateAuthentication to use IBufferWriter<T> to solve the size issue. Unfortunately that's cumbersome to use for one-off calls.

There are two things we need to consider - performance and security.

I am only concerned with the buffer reuse for repetitive calls. This one seems more like a one-off and saving a few allocations on this code path is not going to make a big difference.

On the other hand, the key is a sensitive material. With Spans you can place it in non-relocatable memory (such as stackalloc) and ensure that it gets zeroed after use. Such guarantee cannot be made about arrays which can get relocated by GC and leave a copy of the key in memory.

(I wish we had Secret<T> that's still in limbo at draft specification stage)

@jborean93
Copy link
Contributor Author

jborean93 commented Jan 7, 2025

Could another option be something similar to the string.Create with the SpanAction overload, in this case providing a ReadOnlySpanAction. That way the action is provided with a ReadOnlySpan<byte> the points to the SSPI/GSSAPI structure value. The delegate is responsible for copying or using the key for whatever purpose it has and after running it can be freed like normal. In the case of SMB the KDF function only needs a read only copy to generate the subkeys used for signing and encryption.

I’m unsure if that’s overkill but it does seem like it solves the performance, secret, and not knowing the key size problem.

@filipnavara
Copy link
Member

Could another option be something similar to the string.Create with the SpanAction overload, in this case providing a ReadOnlySpanAction.

I am not sure I like it from API design standpoint (delegate overhead, etc.). Perhaps @bartonjs can point us to some guidance.

From the security point of view it's tempting though. We can avoid copying the key around and we can enforce that the memory gets cleared after use.

@bartonjs
Copy link
Member

bartonjs commented Jan 7, 2025

Logically it seems like the "I'll let you see a ReadOnlySpan of my data" Encode overload we added to AsnWriter: #75759

Here it'd probably be something like

public TReturn DeriveKeyFromSessionKey<TReturn>(Func<ReadOnlySpan<byte>, TReturn> keyDerivationFunction);
public TReturn DeriveKeyFromSessionKey<TState, TReturn>(Func<ReadOnlySpan<byte>, TState, TReturn> keyDerivationFunction, TState state);

might need some sort of HasSessionKey to go with it?

@jborean93
Copy link
Contributor Author

jborean93 commented Jan 7, 2025

Just in case I'm wrong is the main difference between ReadOnlySpanAction and what you are proposing is that yours allows a return value from the delegate?

might need some sort of HasSessionKey to go with it?

AFAIK all the implemented negotiate protocols and C APIs support this functionality so I don't think it would add too much benefit.

@bartonjs
Copy link
Member

bartonjs commented Jan 7, 2025

ReadOnlySpanAction is a legacy from when ReadOnlySpan couldn't be used in generics. Otherwise, it's just the difference between Func and Action... mine allows a return, and ReadOnlySpanAction wouldn't.

It looked like the SMB thing would be something like

using SP800108HmacCounterKdf smb3kdf = a.DeriveKeyFromSessionKey(key => new(key, HashAlgorithmName.SHA256));

Span<byte> signingKey = stackallock byte[16];

smb3kdf.DeriveKey(
    "SMBSigningKey\0"u8,
    preauthHash,  // Hash of preceding SMB messages
    signingKey);

Or to keep it more enclosed:

Span<byte> signingKey = stackallock byte[16];
a.DeriveKeyFromSessionKey(
  static (key, state) =>
  {
    using SP800108HmacCounterKdf smb3kdf = a.DeriveKeyFromSessionKey(key, HashAlgorithmName.SHA256);
    smb3Kdf.DeriveKey("SMBSigningKey\0"u8, state.Item1, state.Item2);
    return 0;
  },
  (preauthHash, signingKey));

For the later, Action would be nicer, so maybe you want to overload it for both Func and Action.

@jborean93
Copy link
Contributor Author

jborean93 commented Jan 7, 2025

Yea in the SMB case I think having the Action overload would be nicer so both would be good. It's technically more complex as there are 4 keys to derive (depending on the SMB dialect negotiated) and session key could potentially be truncated/padded to 16 bytes depending on the SMB cipher negotiated. But either a Func or Action would still fit the requirements here.

Just for clarity the proposal would be to add the following?

public void DeriveKeyFromSessionKey(Action<ReadOnlySpan<byte>> keyDerivationFunction);
public void DeriveKeyFromSessionKey<TState>(Action<ReadOnlySpan<byte>, TState> keyDerivationFunction, TState state);
public TReturn DeriveKeyFromSessionKey<TReturn>(Func<ReadOnlySpan<byte>, TReturn> keyDerivationFunction);
public TReturn DeriveKeyFromSessionKey<TState, TReturn>(Func<ReadOnlySpan<byte>, TState, TReturn> keyDerivationFunction, TState state);

Does it make sense to have the first overload, Action without a TState argument?

@bartonjs
Copy link
Member

bartonjs commented Jan 8, 2025

Does it make sense to have the first overload, Action without a TState argument?

I can certainly imagine people using it with captures... a.DeriveKeyFromSessionKey(key => other.SetKey(key)), and they'd be better off as a.DeriveKeyFromSessionKey(static (key, state) => state.SetKey(key), other)... but it's not like it really hurts. They're all the same in the end.

public void DeriveKeyFromSessionKey(Action<ReadOnlySpan<byte>> keyDerivationFunction) =>
  DeriveKeyFromSessionKey(static (key, state) => state(key), keyDerivationFunction);

public void DeriveKeyFromSessionKey<TState>(Action<ReadOnlySpan<byte>, TState> keyDerivationFunction, TState state) =>
  DeriveKeyFromSessionKey(
    static (key, state)
    {
      state.Kdf(key, state.State);
      return 0;
    },
    (Kdf: keyDerivationFunction, State: state));

public TReturn DeriveKeyFromSessionKey<TReturn>(Func<ReadOnlySpan<byte>, TReturn> keyDerivationFunction) =>
  DeriveKeyFromSessionKey(static (key, state) => state(key), keyDerivationFunction);

public TReturn DeriveKeyFromSessionKey<TState, TReturn>(Func<ReadOnlySpan<byte>, TState, TReturn> keyDerivationFunction, TState state)
{
   // OK, this one needs to do something :)
}

@jborean93
Copy link
Contributor Author

Cool, I'll update the original post with that proposal. I'm also happy to try and look at implementing this unless @filipnavara or anything else wants to hold off or do it themselves.

@wfurt wfurt removed the untriaged New issue has not been triaged by the area owner label Jan 14, 2025
@wfurt wfurt added this to the Future milestone Jan 14, 2025
@wfurt
Copy link
Member

wfurt commented Jan 14, 2025

I'm fine with the proposal assuming we can get it working on at least Windows & Linux without much hassle. Once we agree on the shape I can push it through the review process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Security
Projects
None yet
Development

No branches or pull requests

5 participants