Sspi.MAX_TOKEN_SIZE too small

[ehrbar]

We ran into the problem that a call to Secur32.INSTANCE.InitializeSecurityContext() returned error code W32Errors.SEC_E_INSUFFICIENT_MEMORY in the case one specific user. It turned out that the buffer for the token (argument pOutput) was too small, because that specific user has assigned a large number of group memberships. This was surprising since we used the constant Sspi.MAX_TOKEN_SIZE to allocate the buffer.

This Microsoft KB Article gives an explanantion to this behaviour:
http://support.microsoft.com/default.aspx?scid=kb;EN-US;327825

In JNA, Sspi.MAX_TOKEN_SIZE ist set to 12kBytes, which is the maximum token size in Windows Server 2003. But since then, maximum token size has been increased up to 48kBytes in Windows Server 2012.

In order to allocate enough memory for token buffers, the value of Sspi.MAX_TOKEN_SIZE should be set to 49152.

[dblock]

I'd gladly take a pull request for this, please.

[dblock]

Aside of raising the max, clients should be looping and allocating more memory until they no longer get SEC_E_INSUFFICIENT_MEMORY. The MSDN article suggests setting some registry setting to whatever is needed based on the number of domain groups. That's just a workaround.

[dblock]

See Waffle/waffle@ebedaf7 for an implementation example.

[ehrbar]

IMHO, the loop-until-it-fits approach ist more like a workaround than having set the MAX_TOKEN_SIZE to a sufficiently large value. The MAX part in the constant's name implies to me, that there is no need for a bigger sized buffer.

However, I suggest to check in a recent Windows SDK (sspi.h) what value for MAX_TOKEN_SIZE is defined by Microsoft and use that value for Sspi.MAX_TOKEN_SIZE.

[dblock]

@ehrbar : where's my pull request with the updated value? :)

[michael-o]

Here are two other great resources for that:
http://blogs.technet.com/b/askds/archive/2012/09/12/maxtokensize-and-windows-8-and-windows-server-2012.aspx
http://blogs.technet.com/b/shanecothran/archive/2010/07/16/maxtokensize-and-kerberos-token-bloat.aspx

[matthiasblaesing]

I think there is no reason to keep this open - from my perspective any hardcoded value is bound to fail. The MSDN/technet articles all suggest, that the value is variable and subject to change.

There is a dynamic option: The security package metadata comes with the necessary information:

SecPkgInfo#cbMaxToken

There is one option in JNA 4.4 and before: Secur32.INSTANCE.EnumerateSecurityPackages.

With the enhancements from:

#839

com.sun.jna.platform.win32.Secur32.QuerySecurityPackageInfo(String, PSecPkgInfo)

will become available and can report the information per package

[rolandyoung]

Per the resources referenced by michael-o, there is a good reason to consider increasing MAX_TOKEN_SIZE to 48 * 1024. This is because a token larger than this will fall foul of the maximum size of an HTTP header, while a smaller buffer will be less than the default maximum token size by recent Windows releases. That is, there is a reasonable, best practice value and it is not 12288.

Please could the maintainers reopen this issue? I am happy to submit a PR; especially if a maintainer will provide guidance here as to

• which branch this should be submitted against;
• whether any additional tests are required, given that this constant is used extensively in the existing unit tests.

Background: after a day or two of remote debugging with a very large client, we have determined that we have run into https://issues.apache.org/jira/browse/HTTPCLIENT-1582 . We intend to temporarily fork the Apache code to work around the issue; we could even patch it to use code similar to Waffle, but this seems unnecessarily complicated, given that a reasonable maximum value exists.

[michael-o]

Why not use ALLOCATE_MEMORY and free after that?

[rolandyoung]

@michael-o - sorry, please can you say further what you mean? In our project, we are not calling JNA directly; we use the Apache httpcomponents-client, which contains code very similar to what Waffle had before the patch referred to above. We have tried building the httpclient-win-4.5.3.jar from source patched to use 48*1024 instead of Sspi.MAX_TOKEN_SIZE and this fixes our problem. I am not sure what ALLOCATE_MEMORY you are referring to.

[matthiasblaesing]

I'll repeat my last comment: The required size can be queried from the security package (from MSDN):

https://msdn.microsoft.com/en-us/library/windows/desktop/aa380097(v=vs.85).aspx

_SecPkgContext_Sizes

cbMaxToken
Specifies the maximum size of the security token used in the authentication exchanges.

If you want to hardcode a certain value, you can do it - nobody is stopping apache http client to do exactly that.

[rolandyoung]

OK - I understand that, in principle, using a hard-coded value is a bad idea. However, as it stands, this library provides an obsolete value for this constant and documents it in a way that encourages its misuse.

I propose that the documentation of MAX_TOKEN_SIZE be updated to something like:
"Legacy constant, useful for test purposes only. Do not use in production code. Kerberos tokens may exceed this size if a user is a member of a large number of groups."

And that the documentation of the pOutput parameter to InitializeSecurityContext be updated to warn that if a buffer is allocated using MAX_TOKEN_SIZE, the function may return SEC_E_INSUFFICIENT_MEMORY.

If InitializeSecurityContext supports ISC_REQ_ALLOCATE_MEMORY, then its use should at least be mentioned in the documentation of the pOutput parameter.

[michael-o]

ISC_REQ_ALLOCATE_MEMORY this is way I have recommened years ago, I wouldn't any but this in C. The worst which could happen that someone has hundreds of groups, the accounts has migrated groups and you get the entire SID history. In a forest outside of yours or the machine one's. SID compression doesn't work and you have huge PAC data block. So happened with our forest move which claims to be the largest one on the planet.

[dblock]

From pure JNA perspective the value of MAX_TOKEN_SIZE should be the same as in sspi.h. What's the value these days?

[matthiasblaesing]

Good question :-) I looked into the header files I have installed and non contains such a definition. I only found references to registry entries, that define that value:

https://blogs.technet.microsoft.com/shanecothran/2010/07/16/maxtokensize-and-kerberos-token-bloat/

12.000 bytes is the default, 48.000 bytes is recommed in the above article. So - we could adjust the value, deprecate the value, remove the value.

[dblock]

Did they remove it? We wouldn't have put this value in there without it existing in a header.

If it's not there anymore I would remove it.

[matthiasblaesing]

I rethought this. 12 kB is the default MaxTokenSize (not present in the headers) and can be overridden in the registry. So the java file represents the "default" value. If someone wants to change it or remove it, I'm willing to review, but I won't do it.
Everybody criticizing JNA for offering the default value and requiring changes to documentation should know: These are security functions, you better know what they do and how they work. If you need to follow a default value blindly, stop implementing immediately and go reading. You also need to be aware, that these are just function bindings and as such you can always turn to the original documentation in the MSDN.

[rolandyoung]

OK. It's your project after all. I can, and did "go reading" to find out why the Apache HttpClient throws an utterly inscrutable error when the token is bigger than it expects, but it would have really helped me if it was more obvious from your docs that this is their bug, not mine, or yours.

For the very reason that these are security functions, the maintainers of such projects are reluctant to touch their code unless documentation exists that shows they are getting it wrong and at least hints at what to do about it.

[dblock]

@rolandyoung I think what @matthiasblaesing is saying is that you should PR removal of this value with proper CHANGES if you think that's the right thing to do. The next major release is a good time to do it, this way developers will have to choose the default value and really think about it. Sounds reasonable to me.