[PATCH] Some cleanups from the attic
Volker Lendecke
Volker.Lendecke at SerNet.DE
Mon Feb 25 11:24:59 UTC 2019
Hi!
Review appreciated!
Thanks, Volker
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: 0551-370000-0, mailto:kontakt at sernet.de
Gesch.F.: Dr. Johannes Loxen und Reinhild Jung
AG Göttingen: HR-B 2816 - http://www.sernet.de
-------------- next part --------------
From 377c907e9cd6c644678b4393437161ebf18184a6 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 8 Feb 2019 17:26:04 +0100
Subject: [PATCH 1/4] torture: Use GUID_zero()
10 lines less...
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source4/torture/drs/rpc/dssync.c | 12 ++++--------
source4/torture/rpc/drsuapi.c | 18 ++++++------------
2 files changed, 10 insertions(+), 20 deletions(-)
diff --git a/source4/torture/drs/rpc/dssync.c b/source4/torture/drs/rpc/dssync.c
index 67fde7c2b61..6a8a8b5492b 100644
--- a/source4/torture/drs/rpc/dssync.c
+++ b/source4/torture/drs/rpc/dssync.c
@@ -677,7 +677,6 @@ static bool test_GetNCChanges(struct torture_context *tctx,
struct drsuapi_DsGetNCChangesCtr1 *ctr1 = NULL;
struct drsuapi_DsGetNCChangesCtr6 *ctr6 = NULL;
uint32_t out_level = 0;
- struct GUID null_guid;
struct dom_sid null_sid;
DATA_BLOB gensec_skey;
struct {
@@ -691,7 +690,6 @@ static bool test_GetNCChanges(struct torture_context *tctx,
}
};
- ZERO_STRUCT(null_guid);
ZERO_STRUCT(null_sid);
highest_usn = lpcfg_parm_int(tctx->lp_ctx, NULL, "dssync", "highest_usn", 0);
@@ -721,13 +719,13 @@ static bool test_GetNCChanges(struct torture_context *tctx,
switch (r.in.level) {
case 5:
- nc.guid = null_guid;
+ nc.guid = GUID_zero();
nc.sid = null_sid;
nc.dn = nc_dn_str;
r.in.req = &req;
r.in.req->req5.destination_dsa_guid = ctx->new_dc.invocation_id;
- r.in.req->req5.source_dsa_invocation_id = null_guid;
+ r.in.req->req5.source_dsa_invocation_id = GUID_zero();
r.in.req->req5.naming_context = &nc;
r.in.req->req5.highwatermark.tmp_highest_usn = highest_usn;
r.in.req->req5.highwatermark.reserved_usn = 0;
@@ -752,14 +750,14 @@ static bool test_GetNCChanges(struct torture_context *tctx,
break;
case 8:
- nc.guid = null_guid;
+ nc.guid = GUID_zero();
nc.sid = null_sid;
nc.dn = nc_dn_str;
/* nc.dn can be set to any other ad partition */
r.in.req = &req;
r.in.req->req8.destination_dsa_guid = ctx->new_dc.invocation_id;
- r.in.req->req8.source_dsa_invocation_id = null_guid;
+ r.in.req->req8.source_dsa_invocation_id = GUID_zero();
r.in.req->req8.naming_context = &nc;
r.in.req->req8.highwatermark.tmp_highest_usn = highest_usn;
r.in.req->req8.highwatermark.reserved_usn = 0;
@@ -934,10 +932,8 @@ static bool test_FetchNT4Data(struct torture_context *tctx,
union drsuapi_DsGetNT4ChangeLogRequest req;
union drsuapi_DsGetNT4ChangeLogInfo info;
uint32_t level_out = 0;
- struct GUID null_guid;
DATA_BLOB cookie;
- ZERO_STRUCT(null_guid);
ZERO_STRUCT(cookie);
ZERO_STRUCT(r);
diff --git a/source4/torture/rpc/drsuapi.c b/source4/torture/rpc/drsuapi.c
index a424a3160c6..2ae2ba031e9 100644
--- a/source4/torture/rpc/drsuapi.c
+++ b/source4/torture/rpc/drsuapi.c
@@ -399,7 +399,6 @@ static bool test_DsReplicaSync(struct torture_context *tctx,
struct drsuapi_DsReplicaSync r;
union drsuapi_DsReplicaSyncRequest sync_req;
struct drsuapi_DsReplicaObjectIdentifier nc;
- struct GUID null_guid;
struct dom_sid null_sid;
struct {
int32_t level;
@@ -419,7 +418,6 @@ static bool test_DsReplicaSync(struct torture_context *tctx,
return true;
}
- ZERO_STRUCT(null_guid);
ZERO_STRUCT(null_sid);
r.in.bind_handle = &priv->bind_handle;
@@ -431,7 +429,7 @@ static bool test_DsReplicaSync(struct torture_context *tctx,
r.in.level = array[i].level;
switch(r.in.level) {
case 1:
- nc.guid = null_guid;
+ nc.guid = GUID_zero();
nc.sid = null_sid;
nc.dn = priv->domain_obj_dn?priv->domain_obj_dn:"";
@@ -458,12 +456,10 @@ static bool test_DsReplicaUpdateRefs(struct torture_context *tctx,
struct dcerpc_pipe *p = priv->drs_pipe;
struct drsuapi_DsReplicaUpdateRefs r;
struct drsuapi_DsReplicaObjectIdentifier nc;
- struct GUID null_guid;
struct GUID dest_dsa_guid;
const char *dest_dsa_guid_str;
struct dom_sid null_sid;
- ZERO_STRUCT(null_guid);
ZERO_STRUCT(null_sid);
dest_dsa_guid = GUID_random();
dest_dsa_guid_str = GUID_string(tctx, &dest_dsa_guid);
@@ -472,7 +468,7 @@ static bool test_DsReplicaUpdateRefs(struct torture_context *tctx,
r.in.level = 1; /* Only version 1 is defined presently */
/* setup NC */
- nc.guid = priv->domain_obj_dn ? null_guid : priv->domain_guid;
+ nc.guid = priv->domain_obj_dn ? GUID_zero():priv->domain_guid;
nc.sid = null_sid;
nc.dn = priv->domain_obj_dn ? priv->domain_obj_dn : "";
@@ -552,7 +548,6 @@ static bool test_DsGetNCChanges(struct torture_context *tctx,
union drsuapi_DsGetNCChangesRequest req;
union drsuapi_DsGetNCChangesCtr ctr;
struct drsuapi_DsReplicaObjectIdentifier nc;
- struct GUID null_guid;
struct dom_sid null_sid;
uint32_t level_out;
struct {
@@ -571,7 +566,6 @@ static bool test_DsGetNCChanges(struct torture_context *tctx,
return true;
}
- ZERO_STRUCT(null_guid);
ZERO_STRUCT(null_sid);
for (i=0; i < ARRAY_SIZE(array); i++) {
@@ -586,13 +580,13 @@ static bool test_DsGetNCChanges(struct torture_context *tctx,
switch (r.in.level) {
case 5:
- nc.guid = null_guid;
+ nc.guid = GUID_zero();
nc.sid = null_sid;
nc.dn = priv->domain_obj_dn ? priv->domain_obj_dn : "";
r.in.req = &req;
r.in.req->req5.destination_dsa_guid = GUID_random();
- r.in.req->req5.source_dsa_invocation_id = null_guid;
+ r.in.req->req5.source_dsa_invocation_id = GUID_zero();
r.in.req->req5.naming_context = &nc;
r.in.req->req5.highwatermark.tmp_highest_usn = 0;
r.in.req->req5.highwatermark.reserved_usn = 0;
@@ -609,13 +603,13 @@ static bool test_DsGetNCChanges(struct torture_context *tctx,
break;
case 8:
- nc.guid = null_guid;
+ nc.guid = GUID_zero();
nc.sid = null_sid;
nc.dn = priv->domain_obj_dn ? priv->domain_obj_dn : "";
r.in.req = &req;
r.in.req->req8.destination_dsa_guid = GUID_random();
- r.in.req->req8.source_dsa_invocation_id = null_guid;
+ r.in.req->req8.source_dsa_invocation_id = GUID_zero();
r.in.req->req8.naming_context = &nc;
r.in.req->req8.highwatermark.tmp_highest_usn = 0;
r.in.req->req8.highwatermark.reserved_usn = 0;
--
2.11.0
From 0f8c074336989176f2f8ff5f5ae7d50658a8e165 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 11 Feb 2019 09:02:39 +0100
Subject: [PATCH 2/4] smbd: Align integer types
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/smb2_negprot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/smbd/smb2_negprot.c b/source3/smbd/smb2_negprot.c
index 2b725f30f75..835c25dea55 100644
--- a/source3/smbd/smb2_negprot.c
+++ b/source3/smbd/smb2_negprot.c
@@ -108,7 +108,7 @@ enum protocol_types smbd_smb2_protocol_dialect_match(const uint8_t *indyn,
size_t i;
for (i = 0; i < ARRAY_SIZE(pd); i ++) {
- size_t c = 0;
+ int c = 0;
if (lp_server_max_protocol() < pd[i].proto) {
continue;
--
2.11.0
From d7c083f50001e6b88588791499a85f1923006af6 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 15 Feb 2019 21:22:18 +0100
Subject: [PATCH 3/4] libsmb: Resolve special _recv handling in cli_ntcreate
cli_smb2_create_fnum_recv will gain output create blobs soon and thus
differ from the NT1 function.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/libsmb/clifile.c | 40 ++++++++++++++++++++++++++++------------
1 file changed, 28 insertions(+), 12 deletions(-)
diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c
index ff98ba60034..833db0ac925 100644
--- a/source3/libsmb/clifile.c
+++ b/source3/libsmb/clifile.c
@@ -2079,14 +2079,13 @@ static NTSTATUS cli_ntcreate1_recv(struct tevent_req *req,
}
struct cli_ntcreate_state {
- NTSTATUS (*recv)(struct tevent_req *req, uint16_t *fnum,
- struct smb_create_returns *cr);
struct smb_create_returns cr;
uint16_t fnum;
struct tevent_req *subreq;
};
-static void cli_ntcreate_done(struct tevent_req *subreq);
+static void cli_ntcreate_done_nt1(struct tevent_req *subreq);
+static void cli_ntcreate_done_smb2(struct tevent_req *subreq);
static bool cli_ntcreate_cancel(struct tevent_req *req);
struct tevent_req *cli_ntcreate_send(TALLOC_CTX *mem_ctx,
@@ -2111,8 +2110,6 @@ struct tevent_req *cli_ntcreate_send(TALLOC_CTX *mem_ctx,
}
if (smbXcli_conn_protocol(cli->conn) >= PROTOCOL_SMB2_02) {
- state->recv = cli_smb2_create_fnum_recv;
-
if (cli->use_oplocks) {
create_flags |= REQUEST_OPLOCK|REQUEST_BATCH_OPLOCK;
}
@@ -2122,17 +2119,20 @@ struct tevent_req *cli_ntcreate_send(TALLOC_CTX *mem_ctx,
impersonation_level, desired_access,
file_attributes, share_access, create_disposition,
create_options);
+ if (tevent_req_nomem(subreq, req)) {
+ return tevent_req_post(req, ev);
+ }
+ tevent_req_set_callback(subreq, cli_ntcreate_done_smb2, req);
} else {
- state->recv = cli_ntcreate1_recv;
subreq = cli_ntcreate1_send(
state, ev, cli, fname, create_flags, desired_access,
file_attributes, share_access, create_disposition,
create_options, impersonation_level, security_flags);
+ if (tevent_req_nomem(subreq, req)) {
+ return tevent_req_post(req, ev);
+ }
+ tevent_req_set_callback(subreq, cli_ntcreate_done_nt1, req);
}
- if (tevent_req_nomem(subreq, req)) {
- return tevent_req_post(req, ev);
- }
- tevent_req_set_callback(subreq, cli_ntcreate_done, req);
state->subreq = subreq;
tevent_req_set_cancel_fn(req, cli_ntcreate_cancel);
@@ -2140,7 +2140,23 @@ struct tevent_req *cli_ntcreate_send(TALLOC_CTX *mem_ctx,
return req;
}
-static void cli_ntcreate_done(struct tevent_req *subreq)
+static void cli_ntcreate_done_nt1(struct tevent_req *subreq)
+{
+ struct tevent_req *req = tevent_req_callback_data(
+ subreq, struct tevent_req);
+ struct cli_ntcreate_state *state = tevent_req_data(
+ req, struct cli_ntcreate_state);
+ NTSTATUS status;
+
+ status = cli_ntcreate1_recv(subreq, &state->fnum, &state->cr);
+ TALLOC_FREE(subreq);
+ if (tevent_req_nterror(req, status)) {
+ return;
+ }
+ tevent_req_done(req);
+}
+
+static void cli_ntcreate_done_smb2(struct tevent_req *subreq)
{
struct tevent_req *req = tevent_req_callback_data(
subreq, struct tevent_req);
@@ -2148,7 +2164,7 @@ static void cli_ntcreate_done(struct tevent_req *subreq)
req, struct cli_ntcreate_state);
NTSTATUS status;
- status = state->recv(subreq, &state->fnum, &state->cr);
+ status = cli_smb2_create_fnum_recv(subreq, &state->fnum, &state->cr);
TALLOC_FREE(subreq);
if (tevent_req_nterror(req, status)) {
return;
--
2.11.0
From 5c7b310c39afcfe209f7cf7993953b5b9b07109d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 11 Feb 2019 09:03:39 +0100
Subject: [PATCH 4/4] libcli: Pass buf/len to smb2_negotiate_context_add
Every caller did a data_blob_const right before calling
smb2_negotiate_context_add(). Avoid that.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
libcli/smb/smb2_negotiate_context.c | 19 ++++++++++---------
libcli/smb/smb2_negotiate_context.h | 7 +++++--
libcli/smb/smbXcli_base.c | 10 ++++------
source3/smbd/smb2_negprot.c | 20 ++++++++++++--------
4 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/libcli/smb/smb2_negotiate_context.c b/libcli/smb/smb2_negotiate_context.c
index 61c9e557e33..ac4a08e8910 100644
--- a/libcli/smb/smb2_negotiate_context.c
+++ b/libcli/smb/smb2_negotiate_context.c
@@ -39,7 +39,6 @@ NTSTATUS smb2_negotiate_context_parse(TALLOC_CTX *mem_ctx, const DATA_BLOB buffe
while (true) {
uint16_t data_length;
uint16_t type;
- DATA_BLOB b;
NTSTATUS status;
size_t pad;
uint32_t next_offset;
@@ -58,8 +57,8 @@ NTSTATUS smb2_negotiate_context_parse(TALLOC_CTX *mem_ctx, const DATA_BLOB buffe
return NT_STATUS_INVALID_PARAMETER;
}
- b = data_blob_const(data+0x08, data_length);
- status = smb2_negotiate_context_add(mem_ctx, contexts, type, b);
+ status = smb2_negotiate_context_add(
+ mem_ctx, contexts, type, data+0x08, data_length);
if (!NT_STATUS_IS_OK(status)) {
return status;
}
@@ -148,8 +147,11 @@ NTSTATUS smb2_negotiate_context_push(TALLOC_CTX *mem_ctx, DATA_BLOB *buffer,
return NT_STATUS_OK;
}
-NTSTATUS smb2_negotiate_context_add(TALLOC_CTX *mem_ctx, struct smb2_negotiate_contexts *c,
- uint16_t type, DATA_BLOB data)
+NTSTATUS smb2_negotiate_context_add(TALLOC_CTX *mem_ctx,
+ struct smb2_negotiate_contexts *c,
+ uint16_t type,
+ const uint8_t *buf,
+ size_t buflen)
{
struct smb2_negotiate_context *array;
@@ -161,10 +163,9 @@ NTSTATUS smb2_negotiate_context_add(TALLOC_CTX *mem_ctx, struct smb2_negotiate_c
c->contexts[c->num_contexts].type = type;
- if (data.data) {
- c->contexts[c->num_contexts].data = data_blob_talloc(c->contexts,
- data.data,
- data.length);
+ if (buf != NULL) {
+ c->contexts[c->num_contexts].data = data_blob_talloc(
+ c->contexts, buf, buflen);
NT_STATUS_HAVE_NO_MEMORY(c->contexts[c->num_contexts].data.data);
} else {
c->contexts[c->num_contexts].data = data_blob_null;
diff --git a/libcli/smb/smb2_negotiate_context.h b/libcli/smb/smb2_negotiate_context.h
index 55aa032665e..998cf90f5b8 100644
--- a/libcli/smb/smb2_negotiate_context.h
+++ b/libcli/smb/smb2_negotiate_context.h
@@ -42,8 +42,11 @@ NTSTATUS smb2_negotiate_context_parse(TALLOC_CTX *mem_ctx, const DATA_BLOB buffe
NTSTATUS smb2_negotiate_context_push(TALLOC_CTX *mem_ctx, DATA_BLOB *buffer,
const struct smb2_negotiate_contexts contexts);
-NTSTATUS smb2_negotiate_context_add(TALLOC_CTX *mem_ctx, struct smb2_negotiate_contexts *c,
- uint16_t type, DATA_BLOB data);
+NTSTATUS smb2_negotiate_context_add(TALLOC_CTX *mem_ctx,
+ struct smb2_negotiate_contexts *c,
+ uint16_t type,
+ const uint8_t *buf,
+ size_t buflen);
/*
* return the first context with the given tag
diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
index 2455b6deacd..9105b7c84a4 100644
--- a/libcli/smb/smbXcli_base.c
+++ b/libcli/smb/smbXcli_base.c
@@ -4768,9 +4768,8 @@ static struct tevent_req *smbXcli_negprot_smb2_subreq(struct smbXcli_negprot_sta
SSVAL(p, 4, SMB2_PREAUTH_INTEGRITY_SHA512);
generate_random_buffer(p + 6, 32);
- b = data_blob_const(p, 38);
- status = smb2_negotiate_context_add(state, &c,
- SMB2_PREAUTH_INTEGRITY_CAPABILITIES, b);
+ status = smb2_negotiate_context_add(
+ state, &c, SMB2_PREAUTH_INTEGRITY_CAPABILITIES, p, 38);
if (!NT_STATUS_IS_OK(status)) {
return NULL;
}
@@ -4783,9 +4782,8 @@ static struct tevent_req *smbXcli_negprot_smb2_subreq(struct smbXcli_negprot_sta
SSVAL(p, 2, SMB2_ENCRYPTION_AES128_CCM);
SSVAL(p, 4, SMB2_ENCRYPTION_AES128_GCM);
- b = data_blob_const(p, 6);
- status = smb2_negotiate_context_add(state, &c,
- SMB2_ENCRYPTION_CAPABILITIES, b);
+ status = smb2_negotiate_context_add(
+ state, &c, SMB2_ENCRYPTION_CAPABILITIES, p, 6);
if (!NT_STATUS_IS_OK(status)) {
return NULL;
}
diff --git a/source3/smbd/smb2_negprot.c b/source3/smbd/smb2_negprot.c
index 835c25dea55..528d3f8cc74 100644
--- a/source3/smbd/smb2_negprot.c
+++ b/source3/smbd/smb2_negprot.c
@@ -388,7 +388,6 @@ NTSTATUS smbd_smb2_request_process_negprot(struct smbd_smb2_request *req)
uint16_t selected_preauth = 0;
const uint8_t *p;
uint8_t buf[38];
- DATA_BLOB b;
size_t i;
if (in_preauth->data.length < needed) {
@@ -435,9 +434,12 @@ NTSTATUS smbd_smb2_request_process_negprot(struct smbd_smb2_request *req)
SSVAL(buf, 4, selected_preauth);
generate_random_buffer(buf + 6, 32);
- b = data_blob_const(buf, sizeof(buf));
- status = smb2_negotiate_context_add(req, &out_c,
- SMB2_PREAUTH_INTEGRITY_CAPABILITIES, b);
+ status = smb2_negotiate_context_add(
+ req,
+ &out_c,
+ SMB2_PREAUTH_INTEGRITY_CAPABILITIES,
+ buf,
+ sizeof(buf));
if (!NT_STATUS_IS_OK(status)) {
return smbd_smb2_request_error(req, status);
}
@@ -450,7 +452,6 @@ NTSTATUS smbd_smb2_request_process_negprot(struct smbd_smb2_request *req)
uint16_t cipher_count;
const uint8_t *p;
uint8_t buf[4];
- DATA_BLOB b;
size_t i;
bool aes_128_ccm_supported = false;
bool aes_128_gcm_supported = false;
@@ -504,9 +505,12 @@ NTSTATUS smbd_smb2_request_process_negprot(struct smbd_smb2_request *req)
SSVAL(buf, 0, 1); /* ChiperCount */
SSVAL(buf, 2, xconn->smb2.server.cipher);
- b = data_blob_const(buf, sizeof(buf));
- status = smb2_negotiate_context_add(req, &out_c,
- SMB2_ENCRYPTION_CAPABILITIES, b);
+ status = smb2_negotiate_context_add(
+ req,
+ &out_c,
+ SMB2_ENCRYPTION_CAPABILITIES,
+ buf,
+ sizeof(buf));
if (!NT_STATUS_IS_OK(status)) {
return smbd_smb2_request_error(req, status);
}
--
2.11.0
More information about the samba-technical
mailing list