[PATCH] Re: netlogon_creds_cli_validate() in master4-schannel

Andrew Bartlett abartlet at samba.org
Fri Dec 6 12:31:39 MST 2013


On Fri, 2013-12-06 at 12:43 +0100, Stefan (metze) Metzmacher wrote:
> Hi Garming,
> 
> > + sudo bin/net rpc join -S 192.168.122.249 -Uroot%password12#
> > No realm has been specified! Do you really want to join an Active
> > Directory server?
> > netlogon_creds_cli_ServerPasswordSet failed:
> > NT_STATUS_INVALID_PARAMETER_MIX
> > No realm has been specified! Do you really want to join an Active
> > Directory server?
> > netlogon_creds_cli_check failed with NT_STATUS_NOT_IMPLEMENTED
> > libnet_join_ok: failed to open schannel session on netlogon pipe to
> > server 192.168.122.249 for domain S3. Error was NT_STATUS_NOT_IMPLEMENTED
> > Failed to join domain: failed to verify domain membership after joining:
> > Not implemented
> 
> I've fixed the NT_STATUS_RPC_PROCNUM_OUT_OF_RANGE and
> NT_STATUS_NOT_IMPLEMENTED
> code pathes in netlogon_creds_cli_check_caps(). I also added some comments.
> 
> I also check result in netlogon_creds_cli_auth_srvauth_done() before the
> downgrade check.
> 
> I've updated my
> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master4-schannel-ok
> branch.
> 
> Can you reset with the new code. It would be good to know if
> netlogon_creds_cli_ServerPasswordSet
> still reports NT_STATUS_INVALID_PARAMETER_MIX.

Thanks.  We will test this on Monday.

I found it useful to do a diff between the tree I reviewed and the
current tree, to understand the small changes in the quite large patches
(particularly the first one).  For others looking over this, aside from
Garming's changes, this is the diff:

[abartlet at jesse samba-push]$ git diff master4-schannel-base..master4-schannel-ok
diff --git a/docs-xml/smbdotconf/winbind/disableaesschannel.xml b/docs-xml/smbdotconf/winbind/disableaesschannel.xml
index a355c62..2d2af39 100644
--- a/docs-xml/smbdotconf/winbind/disableaesschannel.xml
+++ b/docs-xml/smbdotconf/winbind/disableaesschannel.xml
@@ -1,7 +1,7 @@
 <samba:parameter name="disable aes schannel"
                  context="G"
                  type="boolean"
-                 advanced="1" developer="1"
+                 advanced="1"
                  xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
 <description>
        <para>This option controls whether winbindd does not try to negotiate
diff --git a/docs-xml/smbdotconf/winbind/rejectmd5servers.xml b/docs-xml/smbdotconf/winbind/rejectmd5servers.xml
index 05204ee..e30d78c 100644
--- a/docs-xml/smbdotconf/winbind/rejectmd5servers.xml
+++ b/docs-xml/smbdotconf/winbind/rejectmd5servers.xml
@@ -1,7 +1,7 @@
 <samba:parameter name="reject md5 servers"
                  context="G"
                  type="boolean"
-                 advanced="1" developer="1"
+                 advanced="1"
                  xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
 <description>
        <para>This option controls whether winbindd requires support
diff --git a/docs-xml/smbdotconf/winbind/requirestrongkey.xml b/docs-xml/smbdotconf/winbind/requirestrongkey.xml
index 1b2c0b6..f803f13 100644
--- a/docs-xml/smbdotconf/winbind/requirestrongkey.xml
+++ b/docs-xml/smbdotconf/winbind/requirestrongkey.xml
@@ -1,10 +1,9 @@
 <samba:parameter name="require strong key"
                  context="G"
                  type="boolean"
-                 advanced="1" developer="1"
+                 advanced="1"
                  xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
 <description>
-       <para>...</para>
        <para>This option controls whether winbindd requires support
        for md5 strong key support for the netlogon secure channel.</para>
 
@@ -12,7 +11,7 @@
        NETLOGON_NEG_ARCFOUR and NETLOGON_NEG_AUTHENTICATED_RPC.</para>
 
        <para>You can set this to no if some domain controllers only support des.
-       This might allows weak crypto to be negiated, may via downgrade attacks.</para>
+       This might allows weak crypto to be negotiated, may via downgrade attacks.</para>
 
        <para>The behavior can be controlled per netbios domain
        by using 'require strong key:NETBIOSDOMAIN = no' as option.</para>
diff --git a/libcli/auth/netlogon_creds_cli.c b/libcli/auth/netlogon_creds_cli.c
index 6b4e35d..a93cdb0 100644
--- a/libcli/auth/netlogon_creds_cli.c
+++ b/libcli/auth/netlogon_creds_cli.c
@@ -1167,6 +1167,13 @@ static void netlogon_creds_cli_auth_srvauth_done(struct tevent_req *subreq)
                }
        }
 
+       if (!NT_STATUS_IS_OK(result) &&
+           !NT_STATUS_EQUAL(result, NT_STATUS_ACCESS_DENIED))
+       {
+               tevent_req_nterror(req, result);
+               return;
+       }
+
        tmp_flags = state->creds->negotiate_flags;
        tmp_flags &= state->context->client.required_flags;
        if (tmp_flags != state->context->client.required_flags) {
@@ -1447,6 +1454,10 @@ static void netlogon_creds_cli_check_caps(struct tevent_req *subreq)
                uint32_t tmp = state->context->client.proposed_flags;
 
                if (tmp & NETLOGON_NEG_SUPPORTS_AES) {
+                       /*
+                        * Old Samba servers need
+                        * "disable aes schannel = yes"
+                        */
                        status = NT_STATUS_DOWNGRADE_DETECTED;
                        tevent_req_nterror(req, status);
                        netlogon_creds_cli_check_cleanup(req, status);
@@ -1455,12 +1466,14 @@ static void netlogon_creds_cli_check_caps(struct tevent_req *subreq)
 
                /*
                 * If we have not proposed NETLOGON_NEG_SUPPORTS_AES
+                * ("disable aes schannel = yes")
                 * it's ok to ignore NT_STATUS_RPC_PROCNUM_OUT_OF_RANGE.
                 *
                 * This is needed against old Samba servers.
                 */
-               status = NT_STATUS_OK;
-               result = NT_STATUS_NOT_IMPLEMENTED;
+               netlogon_creds_cli_check_cleanup(req, result);
+               tevent_req_done(req);
+               return;
        }
        if (tevent_req_nterror(req, status)) {
                netlogon_creds_cli_check_cleanup(req, status);
@@ -1468,10 +1481,17 @@ static void netlogon_creds_cli_check_caps(struct tevent_req *subreq)
        }
 
        if (NT_STATUS_EQUAL(result, NT_STATUS_NOT_IMPLEMENTED)) {
-               if (tevent_req_nterror(req, result)) {
-                       netlogon_creds_cli_check_cleanup(req, result);
-                       return;
-               }
+               /*
+                * This is ok, the server does not support
+                * NETLOGON_NEG_SUPPORTS_AES.
+                *
+                * netr_LogonGetCapabilities() was
+                * netr_LogonDummyRoutine1() before
+                * NETLOGON_NEG_SUPPORTS_AES was invented.
+                */
+               netlogon_creds_cli_check_cleanup(req, result);
+               tevent_req_done(req);
+               return;
        }
 
        ok = netlogon_creds_client_check(&state->tmp_creds,
@@ -1505,8 +1525,8 @@ static void netlogon_creds_cli_check_caps(struct tevent_req *subreq)
        *state->creds = state->tmp_creds;
        status = netlogon_creds_cli_store(state->context,
                                          &state->creds);
+       netlogon_creds_cli_check_cleanup(req, status);
        if (tevent_req_nterror(req, status)) {
-               netlogon_creds_cli_check_cleanup(req, status);
                return;
        }
 
diff --git a/source3/libnet/libnet_join.c b/source3/libnet/libnet_join.c
index b32b82a..b1907de 100644
--- a/source3/libnet/libnet_join.c
+++ b/source3/libnet/libnet_join.c
@@ -1250,10 +1250,6 @@ NTSTATUS libnet_join_ok(struct messaging_context *msg_ctx,
                return NT_STATUS_CANT_ACCESS_DOMAIN_INFO;
        }
 
-       /*
-        * We now have an anonymous connection to IPC$ on the domain password server.
-        */
-
        ok = get_trust_pw_clear(netbios_domain_name,
                                &machine_password,
                                &machine_name,
diff --git a/source3/rpc_server/rpc_server.c b/source3/rpc_server/rpc_server.c
index f283559..680445a 100644
--- a/source3/rpc_server/rpc_server.c
+++ b/source3/rpc_server/rpc_server.c
@@ -37,19 +37,6 @@
 #define SERVER_TCP_LOW_PORT  1024
 #define SERVER_TCP_HIGH_PORT 1300
 
-static NTSTATUS auth_anonymous_session_info(TALLOC_CTX *mem_ctx,
-                                           struct auth_session_info **session_info)
-{
-       NTSTATUS status;
-
-       status = make_session_info_guest(mem_ctx, session_info);
-       if (!NT_STATUS_IS_OK(status)) {
-               return status;
-       }
-
-       return NT_STATUS_OK;
-}
-
 /* Creates a pipes_struct and initializes it with the information
  * sent from the client */
 int make_server_pipes_struct(TALLOC_CTX *mem_ctx,
@@ -1074,11 +1061,14 @@ void dcerpc_ncacn_accept(struct tevent_context *ev_ctx,
        }
 
        if (ncacn_conn->session_info == NULL) {
-               status = auth_anonymous_session_info(ncacn_conn,
-                                                    &ncacn_conn->session_info);
+               /*
+                * TODO: use auth_anonymous_session_info() here?
+                */
+               status = make_session_info_guest(ncacn_conn,
+                                                &ncacn_conn->session_info);
                if (!NT_STATUS_IS_OK(status)) {
                        DEBUG(2, ("Failed to create "
-                                 "auth_anonymous_session_info - %s\n",
+                                 "make_session_info_guest - %s\n",
                                  nt_errstr(status)));
                        talloc_free(ncacn_conn);
                        return;

Andrew Bartlett
-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba




More information about the samba-technical mailing list