Bug 7146 - Samba miss-parses authenticated RPC packets.
Summary: Samba miss-parses authenticated RPC packets.
Alias: None
Product: Samba 3.5
Classification: Unclassified
Component: User & Group Accounts (show other bugs)
Version: 3.5.0pre2
Hardware: Other Linux
: P3 normal
Target Milestone: ---
Assignee: Jeremy Allison
QA Contact: Samba QA Contact
Depends on:
Reported: 2010-02-17 17:15 UTC by Jeremy Allison
Modified: 2016-05-20 09:35 UTC (History)
1 user (show)

See Also:

git-am patch for 3.5.x (34.51 KB, patch)
2010-02-17 17:53 UTC, Jeremy Allison
no flags Details
Second git-am fix for 3.5.x. (2.88 KB, patch)
2010-02-18 17:07 UTC, Jeremy Allison
no flags Details
git-am patch for 3.5.x. (36.88 KB, patch)
2010-02-18 18:20 UTC, Jeremy Allison
jra: review? (vl)

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2010-02-17 17:15:45 UTC
Parts of the Samba RPC client and server code misinterpret authenticated packets.

DCE authenticated packets actually look like this :

|header                    |
| ... frag_len (packet len)|
| ... auth_len             |
|                          |
| Data payload             |
...                     ....
|                          |
|                          |
| auth_pad_len bytes       |
|                          |
| Auth footer              |
| auth_pad_len value       |
|                          |
| Auth payload             |

That's right. The pad bytes come *before* the footer specifying how many pad bytes there are. In order to read this you must seek to the end of the packet and subtract the auth_len (in the packet header) and the auth footer length (a known value).

The client and server code gets this right (mostly) in 3.0.x -> 3.4.x so long as the pad alignment is on an 8 byte boundary (there are some special cases in the code for this).

Tridge discovered there are some (DRS replication) cases where on 64-bit machines where the pad alignment is on a 16-byte boundary. This breaks the existing S3 hand-optimized rpc code.

I have a patch for 3.5.x (and master) that removes all the special cases in client and server code, and allows the pad alignment for generated packets to be specified by changing a constant in include/local.h (this doesn't appect received packets, the new code always handles them correctly whatever pad alignment is used).

Comment 1 Jeremy Allison 2010-02-17 17:17:22 UTC
NB. In the above diagram, "Auth payload" is of length "auth_len" (from the packet header).

Re-assigning to Jeremy to attach the patch.
Comment 2 Jeremy Allison 2010-02-17 17:53:19 UTC
Created attachment 5372 [details]
git-am patch for 3.5.x

Patch for 3.5.x. Which "x" we chose in this case I leave up to the reviewers :-).

Comment 3 Jeremy Allison 2010-02-17 17:53:42 UTC
Comment on attachment 5372 [details]
git-am patch for 3.5.x

Adding metze to review list.
Comment 4 Jeremy Allison 2010-02-17 17:54:04 UTC
Comment on attachment 5372 [details]
git-am patch for 3.5.x

Adding Guenther to the review list.
Comment 5 Jeremy Allison 2010-02-18 17:07:07 UTC
Created attachment 5389 [details]
Second git-am fix for 3.5.x.

Second part of fix.

Ensure we calculate the space correctly (including the ss_padding_len)
when constructing reply packets.
Comment 6 Jeremy Allison 2010-02-18 18:20:58 UTC
Created attachment 5390 [details]
git-am patch for 3.5.x.

Jumbo patch that contains all fixes applied to master as one git-am fix. Actually easier to understand this way as it removes intermediate incorrect space calculations from the patch stream.

If I need to add any more fixes to master I'll keep this jumbo patch updated.

Comment 7 Karolin Seeger 2010-05-27 04:46:27 UTC
Is there a chance to get this one into 3.5.4?
Comment 8 Stefan Metzmacher 2016-05-20 09:35:22 UTC
This is fixed in all current releases