DCERPC Security Context Multiplexing (ready for review)

[Jeremy Allison]

On Thu, Dec 20, 2018 at 12:53:52AM +0100, Stefan Metzmacher wrote:
> 
> I've fixed the missing settimeout in the pylibsmb bindings and fixed the
> commit messages.
> 
> This is ready for review now.
> 
> I'm not attaching the patches here as they're over 500kb in size, which
> is not allowed on the list.
> 
> They can be downloaded here:
> https://gitlab.com/samba-team/samba/merge_requests/173.patch
> 
> >> Please review and comment:-)
> 
> Please review and push:-)

First comment - in [PATCH 033/134] librpc: add dcerpc_get_auth_length() helper function
you have:

+uint16_t dcerpc_get_auth_length(const DATA_BLOB *blob)
+{
+       if (CVAL(blob->data,DCERPC_DREP_OFFSET) & DCERPC_DREP_LE) {
+               return SVAL(blob->data, DCERPC_AUTH_LEN_OFFSET);
+       } else {
+               return RSVAL(blob->data, DCERPC_AUTH_LEN_OFFSET);
+       }
+}

As DATA_BLOB has a length already included, for safety's sake
can you check it's larger than DCERPC_AUTH_LEN_OFFSET (which
is 10, DCERPC_DREP_OFFSET is 4) plus the reading size before indirecting.

I'm sure you already check this elsewhere :-), but adding it
inside this function makes it self-sufficient and obviously safe.

Something like:

NTSTATUS dcerpc_get_auth_length(const DATA_BLOB *blob, uint16_t *plen)
{
	if (blob->length < DCERPC_DREP_OFFSET+1 ||
			blob->length < DCERPC_AUTH_LEN_OFFSET+2) {
		return NT_STATUS_INVALID_PARAMETER;
	}
	if (CVAL(blob->data,DCERPC_DREP_OFFSET) & DCERPC_DREP_LE) {
		*plen = SVAL(blob->data, DCERPC_AUTH_LEN_OFFSET);
	} else {
		*plen = RSVAL(blob->data, DCERPC_AUTH_LEN_OFFSET);
	}
	return NT_STATUS_OK;
}

Yeah I know it makes it more of a pain to use elsewhere, sorry :-(.

But it does make the code standalone safe, even for cut-n-paste :-).

Jeremy.