diff options
-rw-r--r-- | source4/auth/ntlmssp/ntlmssp.c | 7 | ||||
-rw-r--r-- | source4/auth/ntlmssp/ntlmssp_client.c | 12 | ||||
-rw-r--r-- | source4/libcli/composite/connect.c | 6 | ||||
-rw-r--r-- | source4/libcli/composite/sesssetup.c | 144 | ||||
-rw-r--r-- | source4/libcli/ldap/ldap_bind.c | 34 | ||||
-rw-r--r-- | source4/librpc/rpc/dcerpc_auth.c | 59 |
6 files changed, 165 insertions, 97 deletions
diff --git a/source4/auth/ntlmssp/ntlmssp.c b/source4/auth/ntlmssp/ntlmssp.c index 82d6dd0e8f..ef870af3bf 100644 --- a/source4/auth/ntlmssp/ntlmssp.c +++ b/source4/auth/ntlmssp/ntlmssp.c @@ -123,7 +123,12 @@ static NTSTATUS gensec_ntlmssp_update(struct gensec_security *gensec_security, *out = data_blob(NULL, 0); if (gensec_ntlmssp_state->expected_state == NTLMSSP_DONE) { - return NT_STATUS_OK; + /* We are strict here because other modules, which we + * don't fully control (such as GSSAPI) are also + * strict, but are tested less often */ + + DEBUG(1, ("Called NTLMSSP after state machine was 'done'\n")); + return NT_STATUS_INVALID_PARAMETER; } if (!out_mem_ctx) { diff --git a/source4/auth/ntlmssp/ntlmssp_client.c b/source4/auth/ntlmssp/ntlmssp_client.c index 726885c82f..add774f84e 100644 --- a/source4/auth/ntlmssp/ntlmssp_client.c +++ b/source4/auth/ntlmssp/ntlmssp_client.c @@ -353,11 +353,13 @@ NTSTATUS ntlmssp_client_challenge(struct gensec_security *gensec_security, gensec_ntlmssp_state->expected_state = NTLMSSP_DONE; - nt_status = ntlmssp_sign_init(gensec_ntlmssp_state); - if (!NT_STATUS_IS_OK(nt_status)) { - DEBUG(1, ("Could not setup NTLMSSP signing/sealing system (error was: %s)\n", - nt_errstr(nt_status))); - return nt_status; + if (gensec_security->want_features & GENSEC_FEATURE_SIGN|GENSEC_FEATURE_SEAL) { + nt_status = ntlmssp_sign_init(gensec_ntlmssp_state); + if (!NT_STATUS_IS_OK(nt_status)) { + DEBUG(1, ("Could not setup NTLMSSP signing/sealing system (error was: %s)\n", + nt_errstr(nt_status))); + return nt_status; + } } return nt_status; diff --git a/source4/libcli/composite/connect.c b/source4/libcli/composite/connect.c index adac9bcf67..4d35f5c73a 100644 --- a/source4/libcli/composite/connect.c +++ b/source4/libcli/composite/connect.c @@ -136,6 +136,9 @@ static NTSTATUS connect_session_setup(struct composite_context *c, state->req = smb_raw_tcon_send(io->out.tree, state->io_tcon); NT_STATUS_HAVE_NO_MEMORY(state->req); + if (state->req->state == SMBCLI_REQUEST_ERROR) { + return state->req->status; + } state->req->async.fn = request_handler; state->req->async.private = c; @@ -171,6 +174,9 @@ static NTSTATUS connect_negprot(struct composite_context *c, state->creq = smb_composite_sesssetup_send(state->session, state->io_setup); NT_STATUS_HAVE_NO_MEMORY(state->creq); + if (state->creq->state == SMBCLI_REQUEST_ERROR) { + return state->creq->status; + } state->creq->async.fn = composite_handler; state->creq->async.private = c; diff --git a/source4/libcli/composite/sesssetup.c b/source4/libcli/composite/sesssetup.c index 832f7e3d60..b27290d5e4 100644 --- a/source4/libcli/composite/sesssetup.c +++ b/source4/libcli/composite/sesssetup.c @@ -29,7 +29,7 @@ struct sesssetup_state { union smb_sesssetup setup; - NTSTATUS session_key_err; + NTSTATUS gensec_status; struct smb_composite_sesssetup *io; struct smbcli_request *req; }; @@ -78,6 +78,7 @@ static void request_handler(struct smbcli_request *req) struct smbcli_session *session = req->session; DATA_BLOB session_key = data_blob(NULL, 0); DATA_BLOB null_data_blob = data_blob(NULL, 0); + NTSTATUS session_key_err; c->status = smb_raw_sesssetup_recv(req, state, &state->setup); @@ -96,29 +97,41 @@ static void request_handler(struct smbcli_request *req) !NT_STATUS_IS_OK(c->status)) { break; } - c->status = gensec_update(session->gensec, state, - state->setup.spnego.out.secblob, - &state->setup.spnego.in.secblob); - if (!NT_STATUS_EQUAL(c->status, NT_STATUS_MORE_PROCESSING_REQUIRED) && - !NT_STATUS_IS_OK(c->status)) { - break; - } - if (state->setup.spnego.in.secblob.length == 0) { - break; + if (NT_STATUS_EQUAL(state->gensec_status, NT_STATUS_MORE_PROCESSING_REQUIRED)) { + + /* The status value here, from the earlier pass at GENSEC is + * vital to the security of the system. Even if the other end + * accepts, if GENSEC claims 'MORE_PROCESSING_REQUIRED' then + * you must keep feeding it blobs, or else the remote + * host/attacker might avoid mutal authentication + * requirements */ + + state->gensec_status = gensec_update(session->gensec, state, + state->setup.spnego.out.secblob, + &state->setup.spnego.in.secblob); + c->status = state->gensec_status; + if (!NT_STATUS_EQUAL(c->status, NT_STATUS_MORE_PROCESSING_REQUIRED) && + !NT_STATUS_IS_OK(c->status)) { + break; + } + } else { + state->setup.spnego.in.secblob = data_blob(NULL, 0); } - + /* we need to do another round of session setup. We keep going until both sides are happy */ - state->session_key_err = gensec_session_key(session->gensec, &session_key); - if (NT_STATUS_IS_OK(state->session_key_err)) { + session_key_err = gensec_session_key(session->gensec, &session_key); + if (NT_STATUS_IS_OK(session_key_err)) { set_user_session_key(session, &session_key); smbcli_transport_simple_set_signing(session->transport, session_key, null_data_blob); } - state->req = smb_raw_sesssetup_send(session, &state->setup); - state->req->async.fn = request_handler; - state->req->async.private = c; - return; + if (state->setup.spnego.in.secblob.length) { + state->req = smb_raw_sesssetup_send(session, &state->setup); + state->req->async.fn = request_handler; + state->req->async.private = c; + return; + } } /* enforce the local signing required flag */ @@ -144,9 +157,10 @@ static void request_handler(struct smbcli_request *req) /* send a nt1 style session setup */ -static struct smbcli_request *session_setup_nt1(struct composite_context *c, - struct smbcli_session *session, - struct smb_composite_sesssetup *io) +static NTSTATUS session_setup_nt1(struct composite_context *c, + struct smbcli_session *session, + struct smb_composite_sesssetup *io, + struct smbcli_request **req) { struct sesssetup_state *state = talloc_get_type(c->private, struct sesssetup_state); const struct samr_Password *nt_hash = cli_credentials_get_nt_hash(io->in.credentials, state); @@ -180,7 +194,7 @@ static struct smbcli_request *session_setup_nt1(struct composite_context *c, &lmv2_response, &ntlmv2_response, &lmv2_session_key, &session_key)) { data_blob_free(&names_blob); - return NULL; + return NT_STATUS_NO_MEMORY; } data_blob_free(&names_blob); state->setup.nt1.in.password1 = lmv2_response; @@ -218,19 +232,24 @@ static struct smbcli_request *session_setup_nt1(struct composite_context *c, state->setup.nt1.in.password2 = data_blob(NULL, 0); } else { /* could match windows client and return 'cannot logon from this workstation', but it just confuses everybody */ - return NULL; + return NT_STATUS_INVALID_PARAMETER; } - return smb_raw_sesssetup_send(session, &state->setup); + *req = smb_raw_sesssetup_send(session, &state->setup); + if (!*req) { + return NT_STATUS_NO_MEMORY; + } + return (*req)->status; } /* old style session setup (pre NT1 protocol level) */ -static struct smbcli_request *session_setup_old(struct composite_context *c, - struct smbcli_session *session, - struct smb_composite_sesssetup *io) +static NTSTATUS session_setup_old(struct composite_context *c, + struct smbcli_session *session, + struct smb_composite_sesssetup *io, + struct smbcli_request **req) { struct sesssetup_state *state = talloc_get_type(c->private, struct sesssetup_state); const char *password = cli_credentials_get_password(io->in.credentials); @@ -256,19 +275,24 @@ static struct smbcli_request *session_setup_old(struct composite_context *c, strlen(password)); } - return smb_raw_sesssetup_send(session, &state->setup); + *req = smb_raw_sesssetup_send(session, &state->setup); + if (!*req) { + return NT_STATUS_NO_MEMORY; + } + return (*req)->status; } /* - old style session setup (pre NT1 protocol level) + Modern, all singing, all dancing extended security (and possibly SPNEGO) request */ -static struct smbcli_request *session_setup_spnego(struct composite_context *c, - struct smbcli_session *session, - struct smb_composite_sesssetup *io) +static NTSTATUS session_setup_spnego(struct composite_context *c, + struct smbcli_session *session, + struct smb_composite_sesssetup *io, + struct smbcli_request **req) { struct sesssetup_state *state = talloc_get_type(c->private, struct sesssetup_state); - NTSTATUS status; + NTSTATUS status, session_key_err; DATA_BLOB session_key = data_blob(NULL, 0); DATA_BLOB null_data_blob = data_blob(NULL, 0); const char *chosen_oid = NULL; @@ -290,7 +314,7 @@ static struct smbcli_request *session_setup_spnego(struct composite_context *c, status = gensec_client_start(session, &session->gensec, c->event_ctx); if (!NT_STATUS_IS_OK(status)) { DEBUG(1, ("Failed to start GENSEC client mode: %s\n", nt_errstr(status))); - return NULL; + return status; } gensec_want_feature(session->gensec, GENSEC_FEATURE_SESSION_KEY); @@ -299,21 +323,21 @@ static struct smbcli_request *session_setup_spnego(struct composite_context *c, if (!NT_STATUS_IS_OK(status)) { DEBUG(1, ("Failed to start set GENSEC client credentails: %s\n", nt_errstr(status))); - return NULL; + return status; } status = gensec_set_target_hostname(session->gensec, session->transport->socket->hostname); if (!NT_STATUS_IS_OK(status)) { DEBUG(1, ("Failed to start set GENSEC target hostname: %s\n", nt_errstr(status))); - return NULL; + return status; } status = gensec_set_target_service(session->gensec, "cifs"); if (!NT_STATUS_IS_OK(status)) { DEBUG(1, ("Failed to start set GENSEC target service: %s\n", nt_errstr(status))); - return NULL; + return status; } if (session->transport->negotiate.secblob.length) { @@ -327,24 +351,30 @@ static struct smbcli_request *session_setup_spnego(struct composite_context *c, if (!NT_STATUS_IS_OK(status)) { DEBUG(1, ("Failed to start set GENSEC client SPNEGO mechanism %s: %s\n", gensec_get_name_by_oid(chosen_oid), nt_errstr(status))); - return NULL; + return status; } status = gensec_update(session->gensec, state, session->transport->negotiate.secblob, &state->setup.spnego.in.secblob); - if (!NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) { + if (!NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED) && + !NT_STATUS_IS_OK(status)) { DEBUG(1, ("Failed initial gensec_update with mechanism %s: %s\n", gensec_get_name_by_oid(chosen_oid), nt_errstr(status))); - return NULL; + return status; } + state->gensec_status = status; - state->session_key_err = gensec_session_key(session->gensec, &session_key); - if (NT_STATUS_IS_OK(state->session_key_err)) { + session_key_err = gensec_session_key(session->gensec, &session_key); + if (NT_STATUS_IS_OK(session_key_err)) { smbcli_transport_simple_set_signing(session->transport, session_key, null_data_blob); } - return smb_raw_sesssetup_send(session, &state->setup); + *req = smb_raw_sesssetup_send(session, &state->setup); + if (!*req) { + return NT_STATUS_NO_MEMORY; + } + return (*req)->status; } @@ -358,12 +388,16 @@ struct composite_context *smb_composite_sesssetup_send(struct smbcli_session *se { struct composite_context *c; struct sesssetup_state *state; + NTSTATUS status; c = talloc_zero(session, struct composite_context); - if (c == NULL) goto failed; + if (c == NULL) return NULL; state = talloc(c, struct sesssetup_state); - if (state == NULL) goto failed; + if (state == NULL) { + c->state = SMBCLI_REQUEST_ERROR; + c->status = NT_STATUS_NO_MEMORY; + } state->io = io; @@ -380,24 +414,24 @@ struct composite_context *smb_composite_sesssetup_send(struct smbcli_session *se /* see what session setup interface we will use */ if (session->transport->negotiate.protocol < PROTOCOL_NT1) { - state->req = session_setup_old(c, session, io); + status = session_setup_old(c, session, io, &state->req); } else if (!session->transport->options.use_spnego || !(io->in.capabilities & CAP_EXTENDED_SECURITY)) { - state->req = session_setup_nt1(c, session, io); + status = session_setup_nt1(c, session, io, &state->req); } else { - state->req = session_setup_spnego(c, session, io); + status = session_setup_spnego(c, session, io, &state->req); } - if (state->req == NULL) goto failed; - - state->req->async.fn = request_handler; - state->req->async.private = c; + if (NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED) || + NT_STATUS_IS_OK(status)) { + state->req->async.fn = request_handler; + state->req->async.private = c; + return c; + } + c->state = SMBCLI_REQUEST_ERROR; + c->status = status; return c; - -failed: - talloc_free(c); - return NULL; } diff --git a/source4/libcli/ldap/ldap_bind.c b/source4/libcli/ldap/ldap_bind.c index e70a56779b..738222da86 100644 --- a/source4/libcli/ldap/ldap_bind.c +++ b/source4/libcli/ldap/ldap_bind.c @@ -141,6 +141,7 @@ NTSTATUS ldap_bind_sasl(struct ldap_connection *conn, struct cli_credentials *cr { NTSTATUS status; TALLOC_CTX *tmp_ctx = NULL; + DATA_BLOB input = data_blob(NULL, 0); DATA_BLOB output = data_blob(NULL, 0); @@ -183,21 +184,35 @@ NTSTATUS ldap_bind_sasl(struct ldap_connection *conn, struct cli_credentials *cr tmp_ctx = talloc_new(conn); if (tmp_ctx == NULL) goto failed; - status = gensec_update(conn->gensec, tmp_ctx, input, &output); - while (1) { + NTSTATUS gensec_status; struct ldap_message *response; struct ldap_message *msg; struct ldap_request *req; int result = LDAP_OTHER; - if (NT_STATUS_IS_OK(status) && output.length == 0) { - break; - } + status = gensec_update(conn->gensec, tmp_ctx, + input, + &output); + /* The status value here, from GENSEC is vital to the security + * of the system. Even if the other end accepts, if GENSEC + * claims 'MORE_PROCESSING_REQUIRED' then you must keep + * feeding it blobs, or else the remote host/attacker might + * avoid mutal authentication requirements. + * + * Likewise, you must not feed GENSEC too much (after the OK), + * it doesn't like that either + */ + + gensec_status = status; + if (!NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED) && !NT_STATUS_IS_OK(status)) { break; } + if (output.length == 0) { + break; + } msg = new_ldap_sasl_bind_msg(tmp_ctx, "GSS-SPNEGO", &output); if (msg == NULL) { @@ -225,12 +240,15 @@ NTSTATUS ldap_bind_sasl(struct ldap_connection *conn, struct cli_credentials *cr result = response->r.BindResponse.response.resultcode; if (result != LDAP_SUCCESS && result != LDAP_SASL_BIND_IN_PROGRESS) { + status = NT_STATUS_UNEXPECTED_NETWORK_ERROR; break; } - status = gensec_update(conn->gensec, tmp_ctx, - response->r.BindResponse.SASL.secblob, - &output); + /* This is where we check if GENSEC wanted to be fed more data */ + if (!NT_STATUS_EQUAL(gensec_status, NT_STATUS_MORE_PROCESSING_REQUIRED)) { + break; + } + input = response->r.BindResponse.SASL.secblob; } if (NT_STATUS_IS_OK(status) && diff --git a/source4/librpc/rpc/dcerpc_auth.c b/source4/librpc/rpc/dcerpc_auth.c index 7aa563cb9d..8ad3be4ecd 100644 --- a/source4/librpc/rpc/dcerpc_auth.c +++ b/source4/librpc/rpc/dcerpc_auth.c @@ -50,6 +50,8 @@ NTSTATUS dcerpc_bind_auth(struct dcerpc_pipe *p, uint8_t auth_type, uint8_t auth DATA_BLOB credentials; DATA_BLOB null_data_blob = data_blob(NULL, 0); + int num_passes = 0; + if (!p->conn->security_state.generic_state) { status = gensec_client_start(p, &p->conn->security_state.generic_state, p->conn->event_ctx); @@ -73,33 +75,27 @@ NTSTATUS dcerpc_bind_auth(struct dcerpc_pipe *p, uint8_t auth_type, uint8_t auth p->conn->security_state.auth_info->auth_context_id = random(); p->conn->security_state.auth_info->credentials = null_data_blob; - status = gensec_update(p->conn->security_state.generic_state, tmp_ctx, - null_data_blob, - &credentials); - - p->conn->security_state.auth_info->credentials = credentials; - - if (NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) { - /* We are demanding a reply, so use a request that will get us one */ - status = dcerpc_bind_byuuid(p, tmp_ctx, uuid, version); - if (!NT_STATUS_IS_OK(status)) { - goto done; - } - } else if (NT_STATUS_IS_OK(status)) { - /* We don't care for the reply, so jump to the end */ - status = dcerpc_bind_byuuid(p, tmp_ctx, uuid, version); - goto done; - } else { - /* Something broke in GENSEC - bail */ - goto done; - } - while (1) { + num_passes++; status = gensec_update(p->conn->security_state.generic_state, tmp_ctx, p->conn->security_state.auth_info->credentials, &credentials); + + /* The status value here, from GENSEC is vital to the security + * of the system. Even if the other end accepts, if GENSEC + * claims 'MORE_PROCESSING_REQUIRED' then you must keep + * feeding it blobs, or else the remote host/attacker might + * avoid mutal authentication requirements. + * + * Likewise, you must not feed GENSEC too much (after the OK), + * it doesn't like that either + */ + if (!NT_STATUS_IS_OK(status) && !NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) { + DEBUG(1, ("Failed DCERPC client gensec_update with mechanism %s: %s\n", + gensec_get_name_by_authtype(auth_type), nt_errstr(status))); + break; } @@ -110,18 +106,25 @@ NTSTATUS dcerpc_bind_auth(struct dcerpc_pipe *p, uint8_t auth_type, uint8_t auth p->conn->security_state.auth_info->credentials = credentials; if (NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) { - /* We are demanding a reply, so use a request that will get us one */ - status = dcerpc_alter_context(p, tmp_ctx, &p->syntax, &p->transfer_syntax); + if (num_passes == 1) { + status = dcerpc_bind_byuuid(p, tmp_ctx, uuid, version); + } else { + /* We are demanding a reply, so use a request that will get us one */ + status = dcerpc_alter_context(p, tmp_ctx, &p->syntax, &p->transfer_syntax); + } if (!NT_STATUS_IS_OK(status)) { break; } - } else { + } else if (NT_STATUS_IS_OK(status)) { /* NO reply expected, so just send it */ - status = dcerpc_auth3(p->conn, tmp_ctx); - credentials = data_blob(NULL, 0); - if (!NT_STATUS_IS_OK(status)) { - break; + if (num_passes == 1) { + status = dcerpc_bind_byuuid(p, tmp_ctx, uuid, version); + } else { + status = dcerpc_auth3(p->conn, tmp_ctx); } + break; + } else { + break; } }; |