Bug 9075 - Samba 3.5.x and 3.6.x do not seem to support TREE_CONNECT_ANDX_EXTENDED_SIGNATURES ...
Samba 3.5.x and 3.6.x do not seem to support TREE_CONNECT_ANDX_EXTENDED_SIGNA...
Status: RESOLVED INVALID
Product: Samba 3.6
Classification: Unclassified
Component: File services
unspecified
All All
: P5 normal
: ---
Assigned To: Jeremy Allison
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-31 21:28 UTC by Richard Sharpe
Modified: 2012-09-29 08:15 UTC (History)
1 user (show)

See Also:


Attachments
The first patch that might fix this ... prepare the new hash based on [MS-SMB2] (2.81 KB, patch)
2012-07-31 22:39 UTC, Richard Sharpe
no flags Details
draft patches for master (47.81 KB, patch)
2012-08-01 10:36 UTC, Stefan Metzmacher
no flags Details
Capture of net rpc * (10.53 KB, application/octet-stream)
2012-08-08 09:11 UTC, Stefan Metzmacher
no flags Details
Proposed patch for 3.6.next ? (1.96 KB, patch)
2012-08-13 22:36 UTC, Jeremy Allison
metze: review-
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Sharpe 2012-07-31 21:28:46 UTC
We have run into a situation where a customer's clients are requesting
Session Key Protection via the above flag on a TREE_CONNECT_AND (see
3.2.4.2.5) of [MS-SMB].

This seems to be designed to prevent applications running on the
server from divulging the client's actual session keys, but I don't
know which registry key/keys are used to enable this.

This seems to be related to a change in the behavior of Win-7/W2K08
with respect to the Local Security Policy->Local Policies->Security
Options:

Microsoft network client: Digitally sign communications (always)

If you have it enabled (and have the other one, Digitally Sign
communications (if server agrees enabled) then Win-7 and W2K8 does not
like it if Samba does not implement Extended Signatures and resets the
connection.

W2K3 seemed to be happy to go on without the extended signatures.
Comment 1 Richard Sharpe 2012-07-31 21:33:34 UTC
It would seem that the best way to handle this would be to add a function to libsmb/smb_signing called something like smb_signing_protect_keys that performs the functions required in section 3.3.5.4 of [MS-SMB] under Session Key Protection. They are:

-----------------------
If the client has set the TREE_CONNECT_ANDX_EXTENDED_SIGNATURE bit in the Flags field of the SMB_COM_TREE_CONNECT_ANDX request, then the server MUST hash the session key of the calling user. This protects the key used for signing by making it unavailable to server-side applications.

    The one-way hash MUST be performed on the user session key by using the 
    HMAC-MD5 algorithm, as specified in [RFC2104]. The steps are as follows:

    1. Take the 16-byte user session key from Server.Session.SessionKey.

      - If this is an LM authentication where the session key is only 8 bytes, 
        then zero extend it to 16 bytes.

      - If the session key is more than 16 bytes, then use only the first
        16 bytes.

    2. Calculate the one-way hash as follows:

    CALL hmac_md5( SSKeyHash, 256, session key, session key length, digest )
    SET user session key = digest

    The resulting 16-byte digest is treated as the user's new session key
    and returned to the caller who requests it. SSKeyHash is the well-known
    constant array that is described in section 2.2.2.5.

After the session key has been hashed, the server MUST place the hash into Server.Session.SessionKey and set Server.Session.SessionKeyState to Available, which allows applications to query the session key. If the TREE_CONNECT_ANDX_EXTENDED_SIGNATURE bit is not set, then the Server.Session.SessionKey is not changed and Server.Session.SessionKeyState MUST be set to Available.
-----------------------------------

SSKeyHash is defined elsewhere in the document.
Comment 2 Richard Sharpe 2012-07-31 22:33:51 UTC
The following patch might partially do the trick ... I will attach it and test it after adding some more code:

--- libsmb/smb_signing.c.orig   2012-07-31 14:49:24.000000000 -0700
+++ libsmb/smb_signing.c        2012-07-31 15:37:11.000000000 -0700
@@ -45,6 +45,41 @@ struct smb_signing_state {
        uint32_t seqnum;
 };
 
+static unsigned char const SSKeyHash[256] = {
+       0x53, 0x65, 0x63, 0x75, 0x72, 0x69, 0x74, 0x79,
+       0x20, 0x53, 0x69, 0x67, 0x6e, 0x61, 0x74, 0x75,
+       0x72, 0x65, 0x20, 0x4b, 0x65, 0x79, 0x20, 0x55,
+       0x70, 0x67, 0x72, 0x61, 0x64, 0x65, 0x79, 0x07,
+       0x6e, 0x28, 0x2e, 0x69, 0x88, 0x10, 0xb3, 0xdb,
+       0x01, 0x55, 0x72, 0xfb, 0x74, 0x14, 0xfb, 0xc4,
+       0xc5, 0xaf, 0x3b, 0x41, 0x65, 0x32, 0x17, 0xba,
+       0xa3, 0x29, 0x08, 0xc1, 0xde, 0x16, 0x61, 0x7e,
+       0x66, 0x98, 0xa4, 0x0b, 0xfe, 0x06, 0x83, 0x53,
+       0x4d, 0x05, 0xdf, 0x6d, 0xa7, 0x51, 0x10, 0x73,
+       0xc5, 0x50, 0xdc, 0x5e, 0xf8, 0x21, 0x46, 0xaa,
+       0x96, 0x14, 0x33, 0xd7, 0x52, 0xeb, 0xaf, 0x1f,
+       0xbf, 0x36, 0x6c, 0xfc, 0xb7, 0x1d, 0x21, 0x19,
+       0x81, 0xd0, 0x6b, 0xfa, 0x77, 0xad, 0xbe, 0x18,
+       0x78, 0xcf, 0x10, 0xbd, 0xd8, 0x78, 0xf7, 0xd3,
+       0xc6, 0xdf, 0x43, 0x32, 0x19, 0xd3, 0x9b, 0xa8,
+       0x4d, 0x9e, 0xaa, 0x41, 0xaf, 0xcb, 0xc6, 0xb9,
+       0x34, 0xe7, 0x48, 0x25, 0xd4, 0x88, 0xc4, 0x51,
+       0x60, 0x38, 0xd9, 0x62, 0xe8, 0x8d, 0x5b, 0x83,
+       0x92, 0x7f, 0xb5, 0x0e, 0x1c, 0x2d, 0x06, 0x91,
+       0xc3, 0x75, 0xb3, 0xcc, 0xf8, 0xf7, 0x92, 0x91,
+       0x0b, 0x3d, 0xa1, 0x10, 0x5b, 0xd5, 0x0f, 0xa8,
+       0x3f, 0x5d, 0x13, 0x83, 0x0a, 0x6b, 0x72, 0x93,
+       0x14, 0x59, 0xd5, 0xab, 0xde, 0x26, 0x15, 0x6d,
+       0x60, 0x67, 0x71, 0x06, 0x6e, 0x3d, 0x0d, 0xa7,
+       0xcb, 0x70, 0xe9, 0x08, 0x5c, 0x99, 0xfa, 0x0a,
+       0x5f, 0x3d, 0x44, 0xa3, 0x8b, 0xc0, 0x8d, 0xda,
+       0xe2, 0x68, 0xd0, 0x0d, 0xcd, 0x7f, 0x3d, 0xf8,
+       0x73, 0x7e, 0x35, 0x7f, 0x07, 0x02, 0x0a, 0xb5,
+       0xe9, 0xb7, 0x87, 0xfb, 0xa1, 0xbf, 0xcb, 0x32,
+       0x31, 0x66, 0x09, 0x48, 0x88, 0xcc, 0x18, 0xa3,
+       0xb2, 0x1f, 0x1f, 0x1b, 0x90, 0x4e, 0xd7, 0xe1
+};
+
 static void smb_signing_reset_info(struct smb_signing_state *si)
 {
        si->active = false;
@@ -74,6 +109,42 @@ struct smb_signing_state *smb_signing_in
        return si;
 }
 
+static bool smb_signing_protect_key(struct smb_signing_state *si)
+{
+       unsigned char digest[64];
+
+       /* If signing is not active, then this is wrong */
+       if (!si->active) {
+               return false;
+       }
+
+       /* The key cannot be more than 16 bytes in length */
+       if (si->mac_key.length > 16) {
+               si->mac_key.length = 16;
+       }
+
+       /* If the key is 8 bytes, zero fill to 16 */
+       if (si->mac_key.length == 8) {
+               int i = 0;
+
+               if (!data_blob_realloc(si, &si->mac_key, 16)) {
+                       return false;
+               }
+
+               for (i = 8; i < 16; i++)
+                       si->mac_key.data[i] = 0;
+
+               si->mac_key.length = 16;
+       }
+
+       hmac_md5(si->mac_key.data, SSKeyHash, sizeof(SSKeyHash), digest);
+
+       /* Replace the session key with the digest */
+       memcpy(si->mac_key.data, digest, 16);
+
+       return true;
+}
+
 static bool smb_signing_good(struct smb_signing_state *si,
                             bool good, uint32_t seq)
 {
Comment 3 Richard Sharpe 2012-07-31 22:39:58 UTC
Created attachment 7732 [details]
The first patch that might fix this ... prepare the new hash based on [MS-SMB2]

This patch implements a function to calculate the new signing key and update it.

Next up a small change in reply_tcon_andx or whatever is needed to call this function.

Probably need some DEBUG calls in error paths as well.
Comment 4 Richard Sharpe 2012-08-01 02:26:45 UTC
OK, the master branch is lacking in this functionality as well.

I will work up my patches against master.
Comment 5 Stefan Metzmacher 2012-08-01 06:08:31 UTC
(In reply to comment #2)
> The following patch might partially do the trick ... I will attach it and test
> it after adding some more code:
> 
> --- libsmb/smb_signing.c.orig   2012-07-31 14:49:24.000000000 -0700
> +++ libsmb/smb_signing.c        2012-07-31 15:37:11.000000000 -0700
> @@ -45,6 +45,41 @@ struct smb_signing_state {
>         uint32_t seqnum;
>  };
> 
> +static unsigned char const SSKeyHash[256] = {
> +       0x53, 0x65, 0x63, 0x75, 0x72, 0x69, 0x74, 0x79,
> +       0x20, 0x53, 0x69, 0x67, 0x6e, 0x61, 0x74, 0x75,
> +       0x72, 0x65, 0x20, 0x4b, 0x65, 0x79, 0x20, 0x55,
> +       0x70, 0x67, 0x72, 0x61, 0x64, 0x65, 0x79, 0x07,
> +       0x6e, 0x28, 0x2e, 0x69, 0x88, 0x10, 0xb3, 0xdb,
> +       0x01, 0x55, 0x72, 0xfb, 0x74, 0x14, 0xfb, 0xc4,
> +       0xc5, 0xaf, 0x3b, 0x41, 0x65, 0x32, 0x17, 0xba,
> +       0xa3, 0x29, 0x08, 0xc1, 0xde, 0x16, 0x61, 0x7e,
> +       0x66, 0x98, 0xa4, 0x0b, 0xfe, 0x06, 0x83, 0x53,
> +       0x4d, 0x05, 0xdf, 0x6d, 0xa7, 0x51, 0x10, 0x73,
> +       0xc5, 0x50, 0xdc, 0x5e, 0xf8, 0x21, 0x46, 0xaa,
> +       0x96, 0x14, 0x33, 0xd7, 0x52, 0xeb, 0xaf, 0x1f,
> +       0xbf, 0x36, 0x6c, 0xfc, 0xb7, 0x1d, 0x21, 0x19,
> +       0x81, 0xd0, 0x6b, 0xfa, 0x77, 0xad, 0xbe, 0x18,
> +       0x78, 0xcf, 0x10, 0xbd, 0xd8, 0x78, 0xf7, 0xd3,
> +       0xc6, 0xdf, 0x43, 0x32, 0x19, 0xd3, 0x9b, 0xa8,
> +       0x4d, 0x9e, 0xaa, 0x41, 0xaf, 0xcb, 0xc6, 0xb9,
> +       0x34, 0xe7, 0x48, 0x25, 0xd4, 0x88, 0xc4, 0x51,
> +       0x60, 0x38, 0xd9, 0x62, 0xe8, 0x8d, 0x5b, 0x83,
> +       0x92, 0x7f, 0xb5, 0x0e, 0x1c, 0x2d, 0x06, 0x91,
> +       0xc3, 0x75, 0xb3, 0xcc, 0xf8, 0xf7, 0x92, 0x91,
> +       0x0b, 0x3d, 0xa1, 0x10, 0x5b, 0xd5, 0x0f, 0xa8,
> +       0x3f, 0x5d, 0x13, 0x83, 0x0a, 0x6b, 0x72, 0x93,
> +       0x14, 0x59, 0xd5, 0xab, 0xde, 0x26, 0x15, 0x6d,
> +       0x60, 0x67, 0x71, 0x06, 0x6e, 0x3d, 0x0d, 0xa7,
> +       0xcb, 0x70, 0xe9, 0x08, 0x5c, 0x99, 0xfa, 0x0a,
> +       0x5f, 0x3d, 0x44, 0xa3, 0x8b, 0xc0, 0x8d, 0xda,
> +       0xe2, 0x68, 0xd0, 0x0d, 0xcd, 0x7f, 0x3d, 0xf8,
> +       0x73, 0x7e, 0x35, 0x7f, 0x07, 0x02, 0x0a, 0xb5,
> +       0xe9, 0xb7, 0x87, 0xfb, 0xa1, 0xbf, 0xcb, 0x32,
> +       0x31, 0x66, 0x09, 0x48, 0x88, 0xcc, 0x18, 0xa3,
> +       0xb2, 0x1f, 0x1f, 0x1b, 0x90, 0x4e, 0xd7, 0xe1
> +};
> +
>  static void smb_signing_reset_info(struct smb_signing_state *si)
>  {
>         si->active = false;
> @@ -74,6 +109,42 @@ struct smb_signing_state *smb_signing_in
>         return si;
>  }
> 
> +static bool smb_signing_protect_key(struct smb_signing_state *si)
> +{
> +       unsigned char digest[64];
> +
> +       /* If signing is not active, then this is wrong */
> +       if (!si->active) {
> +               return false;
> +       }
> +
> +       /* The key cannot be more than 16 bytes in length */
> +       if (si->mac_key.length > 16) {
> +               si->mac_key.length = 16;
> +       }
> +
> +       /* If the key is 8 bytes, zero fill to 16 */
> +       if (si->mac_key.length == 8) {
> +               int i = 0;
> +
> +               if (!data_blob_realloc(si, &si->mac_key, 16)) {
> +                       return false;
> +               }
> +
> +               for (i = 8; i < 16; i++)
> +                       si->mac_key.data[i] = 0;
> +
> +               si->mac_key.length = 16;
> +       }
> +
> +       hmac_md5(si->mac_key.data, SSKeyHash, sizeof(SSKeyHash), digest);
> +
> +       /* Replace the session key with the digest */
> +       memcpy(si->mac_key.data, digest, 16);
> +
> +       return true;
> +}
> +
>  static bool smb_signing_good(struct smb_signing_state *si,
>                              bool good, uint32_t seq)
>  {

I'm not sure this is correct. I read it as
"the signing key is protected from the application"
not "the signing key is changed".

This means the change applies to the session key which is exposed
to the named-pipe/RPC-layer.

See
https://gitweb.samba.org/?p=samba.git;a=blob;f=source3/smbd/smb2_sesssetup.c;hb=d4bce355a#l235

For the SMB2 example, it's the session_info->session_key which needs to be changed.
Comment 6 Stefan Metzmacher 2012-08-01 10:33:36 UTC
I have prepared our client support and added a TODO marker
in the server code in this branch

https://gitweb.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master3-signing

This needs testing of the client against Windows and
a completed server implementation.
Comment 7 Stefan Metzmacher 2012-08-01 10:36:06 UTC
Created attachment 7733 [details]
draft patches for master

The commits without TODO are already in autobuild...
Comment 8 Richard Sharpe 2012-08-01 13:31:11 UTC
Sure, I was not sure what it was referring to, and I did not notice that the User Session Key was being protected.

It seems that there are only three references, or so, to User Session Key and it is not explained.

With reference to this:

+	if (session->smb1.application_key.length != 16) {
+		return NT_STATUS_INVALID_PARAMETER_MIX;
+	}

This does not conform to the spec, which says that if the session is only 8 bytes it should be extended to 16 with zeros (only in the LM authentication case, I guess) and if it is greater than 16 bytes it should be truncated.
Comment 9 Stefan Metzmacher 2012-08-01 13:49:57 UTC
(In reply to comment #8)
> Sure, I was not sure what it was referring to, and I did not notice that the
> User Session Key was being protected.
> 
> It seems that there are only three references, or so, to User Session Key and
> it is not explained.
> 
> With reference to this:
> 
> +    if (session->smb1.application_key.length != 16) {
> +        return NT_STATUS_INVALID_PARAMETER_MIX;
> +    }
> 
> This does not conform to the spec, which says that if the session is only 8
> bytes it should be extended to 16 with zeros (only in the LM authentication
> case, I guess) and if it is greater than 16 bytes it should be truncated.

At that stage we have already expanded it to 16 bytes.

See uint8_t session_key[16]; in smb1cli_session_set_session_key()

But see my master3-signing branch for the latest code, I've fixed some
minor bugs to make "make test" happy.
Comment 10 Stefan Metzmacher 2012-08-04 12:55:18 UTC
Should be fixed in master with commit 401860cab6ab3d88659361bd333f6667da071d7b,
Richard can you please test current master?
Comment 11 Richard Sharpe 2012-08-04 14:01:46 UTC
(In reply to comment #10)
> Should be fixed in master with commit 401860cab6ab3d88659361bd333f6667da071d7b,
> Richard can you please test current master?

OK, I will have a look at this next week. The simplest way to test it might be to back-port the change to 3.5.15 ...
Comment 12 Richard Sharpe 2012-08-04 14:05:17 UTC
(In reply to comment #10)
> Should be fixed in master with commit 401860cab6ab3d88659361bd333f6667da071d7b,
> Richard can you please test current master?

Hmmm, it looks like there are about a dozen commits related to this ...
Comment 13 Richard Sharpe 2012-08-06 20:15:15 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > Should be fixed in master with commit 401860cab6ab3d88659361bd333f6667da071d7b,
> > Richard can you please test current master?
> 
> OK, I will have a look at this next week. The simplest way to test it might be
> to back-port the change to 3.5.15 ...

OK, so I have tested this, and it seems that Samba no longer drops the connection, however, it is also not setting the bit in the response saying that extended signatures are enabled.

This appears be be because in sourc3/smbd/reply.c:reply_tcon_and_X at around line 854 we have:

        if (session->global->application_key.length == 0 &&
            session->global->signing_key.length > 0)

However, in source3/smbd/sesssetup.c:reply_sesssetup_and_X at about line 959 we have:

                session->global->application_key =
                        data_blob_talloc(session->global,
                                         session_key,
                                         sizeof(session_key));

So, the test above for application_key.length == 0 looks wrong. The application key will exist and be equal to the signing key. I think we need to defer setting the application key, probably (in which case it is the same as the signing key.)
Comment 14 Richard Sharpe 2012-08-06 21:21:40 UTC
Following up on my last comment, since [MS-SMB] 3.3.4.2 says that if Server.Session.SessionKeyState is Unavailable a request by an application for a User Session Key must be denied (STATUS_ACCESS_DENIED), and SessionKeyState is set to available only during TREE_CONNECT_ANDX processing and is set regardless of the state of the TREE_CONNECT_ANDX_EXTENDED_SIGNATURE bit,

This suggests that for SMB1 SESSION_SETUP_ANDX requests we should not set the Application Key, but rather we should set it during TREE_CONNECT_ANDX processing, and should set it based on the value of the TREE_CONNECT_ANDX_EXTENDED_SIGNATURE bit and then, if TREE_CONNECT_ANDX_EXTENDED_SIGNATURE is set, or that into optional.
Comment 15 Stefan Metzmacher 2012-08-07 04:45:58 UTC
(In reply to comment #14)
> Following up on my last comment, since [MS-SMB] 3.3.4.2 says that if
> Server.Session.SessionKeyState is Unavailable a request by an application for a
> User Session Key must be denied (STATUS_ACCESS_DENIED), and SessionKeyState is
> set to available only during TREE_CONNECT_ANDX processing and is set regardless
> of the state of the TREE_CONNECT_ANDX_EXTENDED_SIGNATURE bit,
> 
> This suggests that for SMB1 SESSION_SETUP_ANDX requests we should not set the
> Application Key, but rather we should set it during TREE_CONNECT_ANDX
> processing, and should set it based on the value of the
> TREE_CONNECT_ANDX_EXTENDED_SIGNATURE bit and then, if
> TREE_CONNECT_ANDX_EXTENDED_SIGNATURE is set, or that into optional.

That's what we do for spnego session setups see line 306 in sesssetup.c,
then only the first TCONX sets the application key and may return the SMB_EXTENDED_SIGNATURES bit.

If tested that against windows and any following TCONX does not touch the application key and don't set the SMB_EXTENDED_SIGNATURES bit.
Comment 16 Stefan Metzmacher 2012-08-07 04:53:01 UTC
(In reply to comment #14)
> Following up on my last comment, since [MS-SMB] 3.3.4.2 says that if
> Server.Session.SessionKeyState is Unavailable a request by an application for a
> User Session Key must be denied (STATUS_ACCESS_DENIED), and SessionKeyState is
> set to available only during TREE_CONNECT_ANDX processing and is set regardless
> of the state of the TREE_CONNECT_ANDX_EXTENDED_SIGNATURE bit,

The docs might be a bit unclear, but for the client is says in
3.2.5.3 Receiving an SMB_COM_SESSION_SETUP_ANDX Response:
...
NTLM Authentication
If the CAP_EXTENDED_SECURITY bit in Client.Connection.ServerCapabilities is not set ...
... The client MUST set Client.Session.SessionKeyState to Available.

I mean we could try against windows really happens without a spnego session setup.
Comment 17 Richard Sharpe 2012-08-07 14:55:07 UTC
(In reply to comment #16)
> (In reply to comment #14)
> > Following up on my last comment, since [MS-SMB] 3.3.4.2 says that if
> > Server.Session.SessionKeyState is Unavailable a request by an application for a
> > User Session Key must be denied (STATUS_ACCESS_DENIED), and SessionKeyState is
> > set to available only during TREE_CONNECT_ANDX processing and is set regardless
> > of the state of the TREE_CONNECT_ANDX_EXTENDED_SIGNATURE bit,
> 
> The docs might be a bit unclear, but for the client is says in
> 3.2.5.3 Receiving an SMB_COM_SESSION_SETUP_ANDX Response:
> ...
> NTLM Authentication
> If the CAP_EXTENDED_SECURITY bit in Client.Connection.ServerCapabilities is not
> set ...
> ... The client MUST set Client.Session.SessionKeyState to Available.
> 
> I mean we could try against windows really happens without a spnego session
> setup.

You are correct. I need to check WWWD as well.

Also, I have not verified that before your changes things worked with my current setup. I will attempt to do this check today.

As for switching off SPNEGO on Windows, if I have understood what you are saying, I am not sure I know how to do that.
Comment 18 Stefan Metzmacher 2012-08-08 09:09:06 UTC
(In reply to comment #17)
> (In reply to comment #16)
> > (In reply to comment #14)
> > > Following up on my last comment, since [MS-SMB] 3.3.4.2 says that if
> > > Server.Session.SessionKeyState is Unavailable a request by an application for a
> > > User Session Key must be denied (STATUS_ACCESS_DENIED), and SessionKeyState is
> > > set to available only during TREE_CONNECT_ANDX processing and is set regardless
> > > of the state of the TREE_CONNECT_ANDX_EXTENDED_SIGNATURE bit,
> > 
> > The docs might be a bit unclear, but for the client is says in
> > 3.2.5.3 Receiving an SMB_COM_SESSION_SETUP_ANDX Response:
> > ...
> > NTLM Authentication
> > If the CAP_EXTENDED_SECURITY bit in Client.Connection.ServerCapabilities is not
> > set ...
> > ... The client MUST set Client.Session.SessionKeyState to Available.
> > 
> > I mean we could try against windows really happens without a spnego session
> > setup.
> 
> You are correct. I need to check WWWD as well.
> 
> Also, I have not verified that before your changes things worked with my
> current setup. I will attempt to do this check today.
> 
> As for switching off SPNEGO on Windows, if I have understood what you are
> saying, I am not sure I know how to do that.

Play with "use spnego = no" for smbd and "client use spnego = no" for "net".

Against Windows I've tested
net rpc user add protected A1b2C3d4P -Uadministrator%A1b2C3d4 -W BLA -S w2k8r2-219.bla.base

net rpc user delete protected -Uadministrator%A1b2C3d4 -W BLA -S w2k8r2-219.bla.base

compared to

net rpc user add protected A1b2C3d4P -Uadministrator%A1b2C3d4 -W BLA -S w2k8r2-219.bla.base --option="client use spnego = no" --option="client ntlmv2 auth = no"

net rpc user delete protected -Uadministrator%A1b2C3d4 -W BLA -S w2k8r2-219.bla.base --option="client use spnego = no" --option="client ntlmv2 auth = no"
Comment 19 Stefan Metzmacher 2012-08-08 09:11:19 UTC
Created attachment 7747 [details]
Capture of net rpc *

This is a capture of the 4 commands in comment 18.
Comment 20 Jeremy Allison 2012-08-10 04:22:53 UTC
We now have another OEM hitting this in a production site. We must get this fixed for 3.6.next (IMHO).

Jeremy.
Comment 21 Richard Sharpe 2012-08-10 05:26:02 UTC
OK, I was wrong in my bug report.

The problem I encountered came down to not setting "server signing = auto"

I actually do not know how to provoke the problem that I originally opened this bug for.

Can anyone describe how to actually provoke this problem?
Comment 22 Stefan Metzmacher 2012-08-10 06:46:49 UTC
If it can be fixed by "server signing = auto",
we should think about changing the default to auto/if_required.

In master in srv_init_signing() handle SMB_SIGNING_DEFAULT as
SMB_SIGNING_IF_REQUIRED instead of SMB_SIGNING_OFF.
Comment 23 Jeremy Allison 2012-08-13 21:08:44 UTC
Ok, I've done some testing from a rpcclient against a Win7 server from master, and a modified 3.6.next, using SMB signed connections and SIGNED+SEALED rpc requests from rpcclient and always setting the TCONX_FLAG_EXTENDED_SIGNATURES bit.

Essentially this is a no-op for the server, as neither the SMB signing code or the RPC signing/sealing code use the modified application key.

So I think setting server signing = auto will fix it, and we can just add a fix inside the reply_tconx code in 3.6.x that just reflects back the TCONX_FLAG_EXTENDED_SIGNATURES bit if sent.

Jeremy.
Comment 24 Jeremy Allison 2012-08-13 22:36:30 UTC
Created attachment 7764 [details]
Proposed patch for 3.6.next ?

Metze and Richard, what do you think of this for 3.6.next ? It's based on the code in master.

Jeremy.
Comment 25 Richard Sharpe 2012-08-13 22:40:59 UTC
(In reply to comment #24)
> Created attachment 7764 [details]
> Proposed patch for 3.6.next ?
> 
> Metze and Richard, what do you think of this for 3.6.next ? It's based on the
> code in master.
> 
> Jeremy.

Thumbs up from me.
Comment 26 Stefan Metzmacher 2012-08-14 05:38:08 UTC
Comment on attachment 7764 [details]
Proposed patch for 3.6.next ?

Why do you want to say the session key is protected while it's not?

We should only backport the EXTENDED_SIGNATURE code if it's really needed.

And from Richards latest comments I think it's not needed.
Comment 27 Stefan Metzmacher 2012-08-14 06:11:08 UTC
(In reply to comment #23)
> Ok, I've done some testing from a rpcclient against a Win7 server from master,
> and a modified 3.6.next, using SMB signed connections and SIGNED+SEALED rpc
> requests from rpcclient and always setting the TCONX_FLAG_EXTENDED_SIGNATURES
> bit.
> 
> Essentially this is a no-op for the server, as neither the SMB signing code or
> the RPC signing/sealing code use the modified application key.

If you use RPC level authentication, the inherited application key from
SMB layer is not used. Also this is called application key, because it's used
at the application layer, not at the SMB signing or RPC transport layer.

The application key is used to encrypt the password fields in some SAMR/LSA calls.

See the net rpc commands in comment 18, they demonstrate that it's not a no-op,
if you just change one byte in SSKeyHash, you'll get NT_STATUS_WRONG_PASSWORD (or something similar) while setting the password.

> So I think setting server signing = auto will fix it, and we can just add a fix
> inside the reply_tconx code in 3.6.x that just reflects back the
> TCONX_FLAG_EXTENDED_SIGNATURES bit if sent.

If "server signing = auto" doesn't fix it without modifications,
we need to backport the whole logic.
Comment 28 Stefan Metzmacher 2012-09-29 08:15:51 UTC
As "server signing = auto" should fix it, I'm marking it as invalid