Total dcerpc response payload more than 0x400000

[Stefan Metzmacher]

Am 17.06.2016 um 23:32 schrieb Andrew Bartlett:
> On Fri, 2016-06-17 at 13:32 -0700, Jeremy Allison wrote:
>> On Fri, Jun 17, 2016 at 01:11:14PM +1200, Andrew Bartlett wrote:
>>>
>>> I totally agree.  Thanks for filing the bug about this, as this is
>>> a
>>> serious regression in the DRS replication case.
>>>
>>>  https://bugzilla.samba.org/show_bug.cgi?id=11948
>>>
>>> What happens here is that we stop being able to replicate in any
>>> domain
>>> where an object is more than 4MB, or indeed any domains where each
>>> object is more than 10k on average, which will break on say a
>>> domain
>>> with a jpegPhoto.  Indeed, a jpegPhoto of 4MB is not unreasonable
>>> to
>>> expect, given the default from a camera these days.
>>>
>>> We need to remove this limit, or change it to more like 240MB
>>> (talloc
>>> won't allocate larger the 256MB at the moment). 
>>>
>>> The limit makes sense thought of from the context of simple
>>> administrative calls, but it is also used for bulk data transport,
>>> server->client.  I've not changed the client->server direction, but
>>> I
>>> have heard of spoolss calls where the windows client would send
>>> over
>>> 16MB of zeros in a in/out buffer, so we may need to fix that also. 
>>>
>>> Also, at the point when we get this, if the connection is
>>> authenticated, then the authentication has been checked, and the
>>> packet
>>> is already in memory, so we can be generous here.
>>>
>>> Metze: Can you please ACK this change?
>>
>> Metze is away on vacation at the moment, but no one person's
>> absence should prevent bugs getting fixed :-).
>>
>> Currently this patch checks:
>>
>>  +     if (req->payload.length + length >
>> DCERPC_NCACN_CLIENT_PAYLOAD_MAX_SIZE) {
>>
>> then checks:
>>
>>  +	if (length > DCERPC_NCACN_CLIENT_PAYLOAD_MAX_SIZE) {
>>
>> I think that should be the other way around - the length check
>> can never be true if the first check isn't true.
>>
>> Can you change the patch to check the following 3 things in order:
>>
>> 1). Current packet length check.
>>
>>  +      if (length > DCERPC_NCACN_CLIENT_PAYLOAD_MAX_SIZE) {
>>
>> 2). Integer wrap check.
>>
>>  +      if (req->payload.length + length < length) {
>>
>> 3). Combination check.
>>
>>  +     if (req->payload.length + length >
>> DCERPC_NCACN_CLIENT_PAYLOAD_MAX_SIZE) {
>>
>> Then I'll re-review. If metze really hates this he can
>> fix it when he gets back :-) - plus I'll ask dochelp next
>> week when I'm up at Microsoft about the limits Windows
>> has on RPC packet reassembly.
> 
> Thanks.  There are some more clues and details from metze in the bug,
> which I was finally able to access late yesterday, including where he
> got the 4MB number from.

I think using 16 MB should be a good enough fix for now.

> I'll tidy this up and get it back to you however, as 'clearly' it could
> cause nasty regressions for some, and we and re-instant the clamp once
> we figure out how to do safely.  
> If we have to, then 16MB may be a compromise later on, once we add code
> to retry DRS replication in this case.  I expect we will need to add a
> large-object replication test.

https://msdn.microsoft.com/en-us/library/ms954378.aspx
indicates the following:

  5. Attribute Values Must Not Exceed 500KB in Length,
     and the Total Object Size Must Not Exceed 1MB

  Active Directory does not support streaming of attribute values
  and is not appropriate for very large data. For this reason,
  the maximum length of an attribute value stored in
  Active Directory must not exceed 500KB in size, and the
  total object size must not exceed 1MB.

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160620/1ec44ed4/signature.sig>