From 76f6cf5bbfc1eececa3c76f492372fd66f5fa7ed Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 5 Jan 2016 11:18:12 -0800 Subject: [PATCH 01/30] CVE-2015-7560: s3: smbd: Add refuse_symlink() function that can be used to prevent operations on a symlink. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11648 Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam --- source3/smbd/trans2.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index d5a38d4..345daac 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -54,6 +54,34 @@ static char *store_file_unix_basic_info2(connection_struct *conn, files_struct *fsp, const SMB_STRUCT_STAT *psbuf); +/**************************************************************************** + Check if an open file handle or pathname is a symlink. +****************************************************************************/ + +static NTSTATUS refuse_symlink(connection_struct *conn, + const files_struct *fsp, + const char *name) +{ + SMB_STRUCT_STAT sbuf; + const SMB_STRUCT_STAT *pst = NULL; + + if (fsp) { + pst = &fsp->fsp_name->st; + } else { + int ret = vfs_stat_smb_basename(conn, + name, + &sbuf); + if (ret == -1) { + return map_nt_error_from_unix(errno); + } + pst = &sbuf; + } + if (S_ISLNK(pst->st_ex_mode)) { + return NT_STATUS_ACCESS_DENIED; + } + return NT_STATUS_OK; +} + /******************************************************************** The canonical "check access" based on object handle or path function. ********************************************************************/ -- 1.9.1 From fa1c482083cc1b0f124490bd40ad79dd7e94de2c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 5 Jan 2016 10:38:28 -0800 Subject: [PATCH 02/30] CVE-2015-7560: s3: smbd: Refuse to get an ACL from a POSIX file handle on a symlink. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11648 Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam --- source3/smbd/nttrans.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c index 04dddee..f812e85 100644 --- a/source3/smbd/nttrans.c +++ b/source3/smbd/nttrans.c @@ -1905,6 +1905,13 @@ NTSTATUS smbd_do_query_security_desc(connection_struct *conn, return NT_STATUS_ACCESS_DENIED; } + if (S_ISLNK(fsp->fsp_name->st.st_ex_mode)) { + DEBUG(10, ("ACL get on symlink %s denied.\n", + fsp_str_dbg(fsp))); + TALLOC_FREE(frame); + return NT_STATUS_ACCESS_DENIED; + } + if (security_info_wanted & (SECINFO_DACL|SECINFO_OWNER| SECINFO_GROUP|SECINFO_SACL)) { /* Don't return SECINFO_LABEL if anything else was -- 1.9.1 From 774e210f891023bbd76aba14545b0e5eb0cc1512 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 5 Jan 2016 10:52:50 -0800 Subject: [PATCH 03/30] CVE-2015-7560: s3: smbd: Refuse to set an ACL from a POSIX file handle on a symlink. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11648 Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam --- source3/smbd/nttrans.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c index f812e85..9233738 100644 --- a/source3/smbd/nttrans.c +++ b/source3/smbd/nttrans.c @@ -875,6 +875,12 @@ NTSTATUS set_sd(files_struct *fsp, struct security_descriptor *psd, return NT_STATUS_OK; } + if (S_ISLNK(fsp->fsp_name->st.st_ex_mode)) { + DEBUG(10, ("ACL set on symlink %s denied.\n", + fsp_str_dbg(fsp))); + return NT_STATUS_ACCESS_DENIED; + } + if (psd->owner_sid == NULL) { security_info_sent &= ~SECINFO_OWNER; } -- 1.9.1 From 0be03f1b14f8da5d6657f660c5c4853fe3dfc0c5 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 5 Jan 2016 11:22:12 -0800 Subject: [PATCH 04/30] CVE-2015-7560: s3: smbd: Refuse to set a POSIX ACL on a symlink. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11648 Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam --- source3/smbd/trans2.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 345daac..f107a4d 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -6776,6 +6776,7 @@ static NTSTATUS smb_set_posix_acl(connection_struct *conn, uint16_t num_def_acls; bool valid_file_acls = True; bool valid_def_acls = True; + NTSTATUS status; if (total_data < SMB_POSIX_ACL_HEADER_SIZE) { return NT_STATUS_INVALID_PARAMETER; @@ -6803,6 +6804,11 @@ static NTSTATUS smb_set_posix_acl(connection_struct *conn, return NT_STATUS_INVALID_PARAMETER; } + status = refuse_symlink(conn, fsp, smb_fname->base_name); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + DEBUG(10,("smb_set_posix_acl: file %s num_file_acls = %u, num_def_acls = %u\n", smb_fname ? smb_fname_str_dbg(smb_fname) : fsp_str_dbg(fsp), (unsigned int)num_file_acls, -- 1.9.1 From 2907193961139c5398c95815aaa4c501af35a507 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 5 Jan 2016 11:24:36 -0800 Subject: [PATCH 05/30] CVE-2015-7560: s3: smbd: Refuse to get a POSIX ACL on a symlink. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11648 Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam --- source3/smbd/trans2.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index f107a4d..a971e14 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -5279,6 +5279,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn, uint16_t num_file_acls = 0; uint16_t num_def_acls = 0; + status = refuse_symlink(conn, + fsp, + smb_fname->base_name); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + if (fsp && fsp->fh->fd != -1) { file_acl = SMB_VFS_SYS_ACL_GET_FD(fsp, talloc_tos()); -- 1.9.1 From e27f9a419420ed49190fc5ca44e984a021d8de15 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 5 Jan 2016 11:05:48 -0800 Subject: [PATCH 06/30] CVE-2015-7560: s3: smbd: Set return values early, allows removal of code duplication. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11648 Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam --- source3/smbd/trans2.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index a971e14..e093467 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -238,11 +238,12 @@ NTSTATUS get_ea_names_from_file(TALLOC_CTX *mem_ctx, connection_struct *conn, size_t num_names; ssize_t sizeret = -1; + if (pnames) { + *pnames = NULL; + } + *pnum_names = 0; + if (!lp_ea_support(SNUM(conn))) { - if (pnames) { - *pnames = NULL; - } - *pnum_names = 0; return NT_STATUS_OK; } @@ -292,10 +293,6 @@ NTSTATUS get_ea_names_from_file(TALLOC_CTX *mem_ctx, connection_struct *conn, if (sizeret == 0) { TALLOC_FREE(names); - if (pnames) { - *pnames = NULL; - } - *pnum_names = 0; return NT_STATUS_OK; } -- 1.9.1 From 062876f6dd9db6ce573a69f644571b75eb894efb Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 5 Jan 2016 11:29:38 -0800 Subject: [PATCH 07/30] CVE-2015-7560: s3: smbd: Silently return no EA's available on a symlink. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11648 Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam --- source3/smbd/trans2.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index e093467..64960c1 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -237,6 +237,7 @@ NTSTATUS get_ea_names_from_file(TALLOC_CTX *mem_ctx, connection_struct *conn, char **names, **tmp; size_t num_names; ssize_t sizeret = -1; + NTSTATUS status; if (pnames) { *pnames = NULL; @@ -247,6 +248,14 @@ NTSTATUS get_ea_names_from_file(TALLOC_CTX *mem_ctx, connection_struct *conn, return NT_STATUS_OK; } + status = refuse_symlink(conn, fsp, fname); + if (!NT_STATUS_IS_OK(status)) { + /* + * Just return no EA's on a symlink. + */ + return NT_STATUS_OK; + } + /* * TALLOC the result early to get the talloc hierarchy right. */ -- 1.9.1 From 63ae57f412c5d1b6de20062cfc7c94dabdcae021 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 5 Jan 2016 11:33:48 -0800 Subject: [PATCH 08/30] CVE-2015-7560: s3: smbd: Refuse to set EA's on a symlink. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11648 Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam --- source3/smbd/trans2.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 64960c1..ff1ef15 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -659,6 +659,11 @@ NTSTATUS set_ea(connection_struct *conn, files_struct *fsp, return NT_STATUS_EAS_NOT_SUPPORTED; } + status = refuse_symlink(conn, fsp, smb_fname->base_name); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + status = check_access(conn, fsp, smb_fname, FILE_WRITE_EA); if (!NT_STATUS_IS_OK(status)) { return status; -- 1.9.1 From 25963b1be1e46e4f4d9eb0121c6db808fdfa7032 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 6 Jan 2016 17:17:24 -0800 Subject: [PATCH 09/30] CVE-2015-7560: s3: libsmb: Rename cli_posix_getfaclXX() functions to cli_posix_getacl() as they operate on pathnames. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11648 Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam --- source3/client/client.c | 2 +- source3/libsmb/clifile.c | 30 +++++++++++++++--------------- source3/libsmb/proto.h | 6 +++--- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/source3/client/client.c b/source3/client/client.c index ad56f26..831b9bc 100644 --- a/source3/client/client.c +++ b/source3/client/client.c @@ -3376,7 +3376,7 @@ static int cmd_getfacl(void) return 1; } - status = cli_posix_getfacl(targetcli, targetname, ctx, &rb_size, &retbuf); + status = cli_posix_getacl(targetcli, targetname, ctx, &rb_size, &retbuf); if (!NT_STATUS_IS_OK(status)) { d_printf("%s getfacl file %s\n", nt_errstr(status), src); diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c index 364376b..963634e 100644 --- a/source3/libsmb/clifile.c +++ b/source3/libsmb/clifile.c @@ -587,25 +587,25 @@ NTSTATUS cli_posix_hardlink(struct cli_state *cli, } /**************************************************************************** - Do a POSIX getfacl (UNIX extensions). + Do a POSIX getacl - pathname based ACL get (UNIX extensions). ****************************************************************************/ -struct getfacl_state { +struct getacl_state { uint32_t num_data; uint8_t *data; }; -static void cli_posix_getfacl_done(struct tevent_req *subreq); +static void cli_posix_getacl_done(struct tevent_req *subreq); -struct tevent_req *cli_posix_getfacl_send(TALLOC_CTX *mem_ctx, +struct tevent_req *cli_posix_getacl_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct cli_state *cli, const char *fname) { struct tevent_req *req = NULL, *subreq = NULL; - struct getfacl_state *state = NULL; + struct getacl_state *state = NULL; - req = tevent_req_create(mem_ctx, &state, struct getfacl_state); + req = tevent_req_create(mem_ctx, &state, struct getacl_state); if (req == NULL) { return NULL; } @@ -614,16 +614,16 @@ struct tevent_req *cli_posix_getfacl_send(TALLOC_CTX *mem_ctx, if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } - tevent_req_set_callback(subreq, cli_posix_getfacl_done, req); + tevent_req_set_callback(subreq, cli_posix_getacl_done, req); return req; } -static void cli_posix_getfacl_done(struct tevent_req *subreq) +static void cli_posix_getacl_done(struct tevent_req *subreq) { struct tevent_req *req = tevent_req_callback_data( subreq, struct tevent_req); - struct getfacl_state *state = tevent_req_data( - req, struct getfacl_state); + struct getacl_state *state = tevent_req_data( + req, struct getacl_state); NTSTATUS status; status = cli_qpathinfo_recv(subreq, state, &state->data, @@ -635,12 +635,12 @@ static void cli_posix_getfacl_done(struct tevent_req *subreq) tevent_req_done(req); } -NTSTATUS cli_posix_getfacl_recv(struct tevent_req *req, +NTSTATUS cli_posix_getacl_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, size_t *prb_size, char **retbuf) { - struct getfacl_state *state = tevent_req_data(req, struct getfacl_state); + struct getacl_state *state = tevent_req_data(req, struct getacl_state); NTSTATUS status; if (tevent_req_is_nterror(req, &status)) { @@ -651,7 +651,7 @@ NTSTATUS cli_posix_getfacl_recv(struct tevent_req *req, return NT_STATUS_OK; } -NTSTATUS cli_posix_getfacl(struct cli_state *cli, +NTSTATUS cli_posix_getacl(struct cli_state *cli, const char *fname, TALLOC_CTX *mem_ctx, size_t *prb_size, @@ -676,7 +676,7 @@ NTSTATUS cli_posix_getfacl(struct cli_state *cli, goto fail; } - req = cli_posix_getfacl_send(frame, + req = cli_posix_getacl_send(frame, ev, cli, fname); @@ -689,7 +689,7 @@ NTSTATUS cli_posix_getfacl(struct cli_state *cli, goto fail; } - status = cli_posix_getfacl_recv(req, mem_ctx, prb_size, retbuf); + status = cli_posix_getacl_recv(req, mem_ctx, prb_size, retbuf); fail: TALLOC_FREE(frame); diff --git a/source3/libsmb/proto.h b/source3/libsmb/proto.h index c32bba7..ab5fd82 100644 --- a/source3/libsmb/proto.h +++ b/source3/libsmb/proto.h @@ -252,15 +252,15 @@ NTSTATUS cli_posix_hardlink(struct cli_state *cli, const char *newname); uint32_t unix_perms_to_wire(mode_t perms); mode_t wire_perms_to_unix(uint32_t perms); -struct tevent_req *cli_posix_getfacl_send(TALLOC_CTX *mem_ctx, +struct tevent_req *cli_posix_getacl_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct cli_state *cli, const char *fname); -NTSTATUS cli_posix_getfacl_recv(struct tevent_req *req, +NTSTATUS cli_posix_getacl_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, size_t *prb_size, char **retbuf); -NTSTATUS cli_posix_getfacl(struct cli_state *cli, +NTSTATUS cli_posix_getacl(struct cli_state *cli, const char *fname, TALLOC_CTX *mem_ctx, size_t *prb_size, -- 1.9.1 From 444ba8f42003622fb64e9c0ae2e2deda158a1c2a Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 6 Jan 2016 17:02:52 -0800 Subject: [PATCH 10/30] CVE-2015-7560: s3: libsmb: Add SMB1-only POSIX cli_posix_setacl() functions. Needed for tests. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11648 Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam --- source3/libsmb/clifile.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++ source3/libsmb/proto.h | 11 ++++++ 2 files changed, 111 insertions(+) diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c index 963634e..0e790cd 100644 --- a/source3/libsmb/clifile.c +++ b/source3/libsmb/clifile.c @@ -697,6 +697,106 @@ NTSTATUS cli_posix_getacl(struct cli_state *cli, } /**************************************************************************** + Do a POSIX setacl - pathname based ACL set (UNIX extensions). +****************************************************************************/ + +struct setacl_state { + uint8_t *data; +}; + +static void cli_posix_setacl_done(struct tevent_req *subreq); + +struct tevent_req *cli_posix_setacl_send(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct cli_state *cli, + const char *fname, + const void *data, + size_t num_data) +{ + struct tevent_req *req = NULL, *subreq = NULL; + struct setacl_state *state = NULL; + + req = tevent_req_create(mem_ctx, &state, struct setacl_state); + if (req == NULL) { + return NULL; + } + state->data = talloc_memdup(state, data, num_data); + if (tevent_req_nomem(state->data, req)) { + return tevent_req_post(req, ev); + } + + subreq = cli_setpathinfo_send(state, + ev, + cli, + SMB_SET_POSIX_ACL, + fname, + state->data, + num_data); + if (tevent_req_nomem(subreq, req)) { + return tevent_req_post(req, ev); + } + tevent_req_set_callback(subreq, cli_posix_setacl_done, req); + return req; +} + +static void cli_posix_setacl_done(struct tevent_req *subreq) +{ + NTSTATUS status = cli_setpathinfo_recv(subreq); + tevent_req_simple_finish_ntstatus(subreq, status); +} + +NTSTATUS cli_posix_setacl_recv(struct tevent_req *req) +{ + return tevent_req_simple_recv_ntstatus(req); +} + +NTSTATUS cli_posix_setacl(struct cli_state *cli, + const char *fname, + const void *acl_buf, + size_t acl_buf_size) +{ + TALLOC_CTX *frame = talloc_stackframe(); + struct tevent_context *ev = NULL; + struct tevent_req *req = NULL; + NTSTATUS status = NT_STATUS_OK; + + if (smbXcli_conn_has_async_calls(cli->conn)) { + /* + * Can't use sync call while an async call is in flight + */ + status = NT_STATUS_INVALID_PARAMETER; + goto fail; + } + + ev = samba_tevent_context_init(frame); + if (ev == NULL) { + status = NT_STATUS_NO_MEMORY; + goto fail; + } + + req = cli_posix_setacl_send(frame, + ev, + cli, + fname, + acl_buf, + acl_buf_size); + if (req == NULL) { + status = NT_STATUS_NO_MEMORY; + goto fail; + } + + if (!tevent_req_poll_ntstatus(req, ev, &status)) { + goto fail; + } + + status = cli_posix_setacl_recv(req); + + fail: + TALLOC_FREE(frame); + return status; +} + +/**************************************************************************** Stat a file (UNIX extensions). ****************************************************************************/ diff --git a/source3/libsmb/proto.h b/source3/libsmb/proto.h index ab5fd82..dc9aa17 100644 --- a/source3/libsmb/proto.h +++ b/source3/libsmb/proto.h @@ -265,6 +265,17 @@ NTSTATUS cli_posix_getacl(struct cli_state *cli, TALLOC_CTX *mem_ctx, size_t *prb_size, char **retbuf); +struct tevent_req *cli_posix_setacl_send(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct cli_state *cli, + const char *fname, + const void *acl_buf, + size_t acl_buf_size); +NTSTATUS cli_posix_setacl_recv(struct tevent_req *req); +NTSTATUS cli_posix_setacl(struct cli_state *cli, + const char *fname, + const void *acl_buf, + size_t acl_buf_size); struct tevent_req *cli_posix_stat_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct cli_state *cli, -- 1.9.1 From ceb6dcc5df067354c5617b32d9c2ed860c0805e8 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 7 Jan 2016 12:58:34 -0800 Subject: [PATCH 11/30] CVE-2015-7560: s3: torture3: Add new POSIX-SYMLINK-ACL test. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11648 Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam --- selftest/knownfail | 1 + source3/selftest/tests.py | 2 +- source3/torture/torture.c | 198 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 200 insertions(+), 1 deletion(-) diff --git a/selftest/knownfail b/selftest/knownfail index 0d74933..0eda192 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -16,6 +16,7 @@ ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).UID-REGRESSION-TEST # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).SHORTNAME-TEST # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-APPEND # Fails against the s4 ntvfs server +^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-SYMLINK-ACL # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).NTTRANS-FSCTL # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).SMB2-NEGPROT # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).BAD-NBT-SESSION # Fails against the s4 ntvfs server diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 4a26c25..7d95b1e 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -78,7 +78,7 @@ tests = ["RW1", "RW2", "RW3"] for t in tests: plantestsuite("samba3.smbtorture_s3.vfs_aio_fork(simpleserver).%s" % t, "simpleserver", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/vfs_aio_fork', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"]) -posix_tests = ["POSIX", "POSIX-APPEND"] +posix_tests = ["POSIX", "POSIX-APPEND", "POSIX-SYMLINK-ACL"] for t in posix_tests: plantestsuite("samba3.smbtorture_s3.plain(nt4_dc).%s" % t, "nt4_dc", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/posix_share', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"]) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 505920f..f1ea7b0 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -5816,6 +5816,203 @@ static bool run_simple_posix_open_test(int dummy) return correct; } +/* + Test POSIX and Windows ACLs are rejected on symlinks. + */ +static bool run_acl_symlink_test(int dummy) +{ + static struct cli_state *cli; + const char *fname = "posix_file"; + const char *sname = "posix_symlink"; + uint16_t fnum = (uint16_t)-1; + bool correct = false; + NTSTATUS status; + char *posix_acl = NULL; + size_t posix_acl_len = 0; + char *posix_acl_sym = NULL; + size_t posix_acl_len_sym = 0; + struct security_descriptor *sd = NULL; + struct security_descriptor *sd_sym = NULL; + TALLOC_CTX *frame = NULL; + + frame = talloc_stackframe(); + + printf("Starting acl symlink test\n"); + + if (!torture_open_connection(&cli, 0)) { + TALLOC_FREE(frame); + return false; + } + + smbXcli_conn_set_sockopt(cli->conn, sockops); + + status = torture_setup_unix_extensions(cli); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(frame); + return false; + } + + cli_setatr(cli, fname, 0, 0); + cli_posix_unlink(cli, fname); + cli_setatr(cli, sname, 0, 0); + cli_posix_unlink(cli, sname); + + status = cli_ntcreate(cli, + fname, + 0, + READ_CONTROL_ACCESS, + 0, + FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, + FILE_CREATE, + 0x0, + 0x0, + &fnum, + NULL); + + if (!NT_STATUS_IS_OK(status)) { + printf("cli_ntcreate of %s failed (%s)\n", + fname, + nt_errstr(status)); + goto out; + } + + /* Get the Windows ACL on the file. */ + status = cli_query_secdesc(cli, + fnum, + frame, + &sd); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_query_secdesc failed (%s)\n", + nt_errstr(status)); + goto out; + } + + /* Get the POSIX ACL on the file. */ + status = cli_posix_getacl(cli, + fname, + frame, + &posix_acl_len, + &posix_acl); + + if (!NT_STATUS_IS_OK(status)) { + printf("cli_posix_getacl failed (%s)\n", + nt_errstr(status)); + goto out; + } + + status = cli_close(cli, fnum); + if (!NT_STATUS_IS_OK(status)) { + printf("close failed (%s)\n", nt_errstr(status)); + goto out; + } + fnum = (uint16_t)-1; + + /* Now create a symlink. */ + status = cli_posix_symlink(cli, fname, sname); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_posix_symlink of %s -> %s failed (%s)\n", + sname, + fname, + nt_errstr(status)); + goto out; + } + + /* Open a handle on the symlink. */ + status = cli_ntcreate(cli, + sname, + 0, + READ_CONTROL_ACCESS|SEC_STD_WRITE_DAC, + 0, + FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, + FILE_OPEN, + 0x0, + 0x0, + &fnum, + NULL); + + if (!NT_STATUS_IS_OK(status)) { + printf("cli_posix_open of %s failed (%s)\n", + sname, + nt_errstr(status)); + goto out; + } + + /* Get the Windows ACL on the symlink handle. Should fail */ + status = cli_query_secdesc(cli, + fnum, + frame, + &sd_sym); + + if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { + printf("cli_query_secdesc on a symlink gave %s. " + "Should be NT_STATUS_ACCESS_DENIED.\n", + nt_errstr(status)); + goto out; + } + + /* Get the POSIX ACL on the symlink pathname. Should fail. */ + status = cli_posix_getacl(cli, + sname, + frame, + &posix_acl_len_sym, + &posix_acl_sym); + + if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { + printf("cli_posix_getacl on a symlink gave %s. " + "Should be NT_STATUS_ACCESS_DENIED.\n", + nt_errstr(status)); + goto out; + } + + /* Set the Windows ACL on the symlink handle. Should fail */ + status = cli_set_security_descriptor(cli, + fnum, + SECINFO_DACL, + sd); + + if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { + printf("cli_query_secdesc on a symlink gave %s. " + "Should be NT_STATUS_ACCESS_DENIED.\n", + nt_errstr(status)); + goto out; + } + + /* Set the POSIX ACL on the symlink pathname. Should fail. */ + status = cli_posix_setacl(cli, + sname, + posix_acl, + posix_acl_len); + + if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { + printf("cli_posix_getacl on a symlink gave %s. " + "Should be NT_STATUS_ACCESS_DENIED.\n", + nt_errstr(status)); + goto out; + } + + printf("ACL symlink test passed\n"); + correct = true; + + out: + + if (fnum != (uint16_t)-1) { + cli_close(cli, fnum); + fnum = (uint16_t)-1; + } + + cli_setatr(cli, sname, 0, 0); + cli_posix_unlink(cli, sname); + cli_setatr(cli, fname, 0, 0); + cli_posix_unlink(cli, fname); + + if (!torture_close_connection(cli)) { + correct = false; + } + + TALLOC_FREE(frame); + return correct; +} + static uint32_t open_attrs_table[] = { FILE_ATTRIBUTE_NORMAL, @@ -9643,6 +9840,7 @@ static struct { {"OPEN", run_opentest, 0}, {"POSIX", run_simple_posix_open_test, 0}, {"POSIX-APPEND", run_posix_append, 0}, + {"POSIX-SYMLINK-ACL", run_acl_symlink_test, 0}, {"CASE-INSENSITIVE-CREATE", run_case_insensitive_create, 0}, {"ASYNC-ECHO", run_async_echo, 0}, { "UID-REGRESSION-TEST", run_uid_regression_test, 0}, -- 1.9.1 From c68280d930d658b40085d442004284ef73d288f0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 7 Jan 2016 14:26:35 -0800 Subject: [PATCH 12/30] CVE-2015-7560: s3: torture3: Add new POSIX-SYMLINK-EA test. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11648 Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam --- selftest/knownfail | 1 + source3/selftest/tests.py | 2 +- source3/torture/torture.c | 178 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 180 insertions(+), 1 deletion(-) diff --git a/selftest/knownfail b/selftest/knownfail index 0eda192..d9e2823 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -17,6 +17,7 @@ ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).SHORTNAME-TEST # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-APPEND # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-SYMLINK-ACL # Fails against the s4 ntvfs server +^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-SYMLINK-EA # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).NTTRANS-FSCTL # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).SMB2-NEGPROT # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).BAD-NBT-SESSION # Fails against the s4 ntvfs server diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 7d95b1e..79db4b5 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -78,7 +78,7 @@ tests = ["RW1", "RW2", "RW3"] for t in tests: plantestsuite("samba3.smbtorture_s3.vfs_aio_fork(simpleserver).%s" % t, "simpleserver", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/vfs_aio_fork', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"]) -posix_tests = ["POSIX", "POSIX-APPEND", "POSIX-SYMLINK-ACL"] +posix_tests = ["POSIX", "POSIX-APPEND", "POSIX-SYMLINK-ACL", "POSIX-SYMLINK-EA"] for t in posix_tests: plantestsuite("samba3.smbtorture_s3.plain(nt4_dc).%s" % t, "nt4_dc", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/posix_share', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"]) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index f1ea7b0..e75f2aa 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -6013,6 +6013,183 @@ static bool run_acl_symlink_test(int dummy) return correct; } +/* + Test setting EA's are rejected on symlinks. + */ +static bool run_ea_symlink_test(int dummy) +{ + static struct cli_state *cli; + const char *fname = "posix_file_ea"; + const char *sname = "posix_symlink_ea"; + const char *ea_name = "testea_name"; + const char *ea_value = "testea_value"; + uint16_t fnum = (uint16_t)-1; + bool correct = false; + NTSTATUS status; + size_t i, num_eas; + struct ea_struct *eas = NULL; + TALLOC_CTX *frame = NULL; + + frame = talloc_stackframe(); + + printf("Starting EA symlink test\n"); + + if (!torture_open_connection(&cli, 0)) { + TALLOC_FREE(frame); + return false; + } + + smbXcli_conn_set_sockopt(cli->conn, sockops); + + status = torture_setup_unix_extensions(cli); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(frame); + return false; + } + + cli_setatr(cli, fname, 0, 0); + cli_posix_unlink(cli, fname); + cli_setatr(cli, sname, 0, 0); + cli_posix_unlink(cli, sname); + + status = cli_ntcreate(cli, + fname, + 0, + READ_CONTROL_ACCESS, + 0, + FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, + FILE_CREATE, + 0x0, + 0x0, + &fnum, + NULL); + + if (!NT_STATUS_IS_OK(status)) { + printf("cli_ntcreate of %s failed (%s)\n", + fname, + nt_errstr(status)); + goto out; + } + + status = cli_close(cli, fnum); + if (!NT_STATUS_IS_OK(status)) { + printf("close failed (%s)\n", + nt_errstr(status)); + goto out; + } + fnum = (uint16_t)-1; + + /* Set an EA on the path. */ + status = cli_set_ea_path(cli, + fname, + ea_name, + ea_value, + strlen(ea_value)+1); + + if (!NT_STATUS_IS_OK(status)) { + printf("cli_set_ea_path failed (%s)\n", + nt_errstr(status)); + goto out; + } + + /* Now create a symlink. */ + status = cli_posix_symlink(cli, fname, sname); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_posix_symlink of %s -> %s failed (%s)\n", + sname, + fname, + nt_errstr(status)); + goto out; + } + + /* Get the EA list on the path. Should return value set. */ + status = cli_get_ea_list_path(cli, + fname, + frame, + &num_eas, + &eas); + + if (!NT_STATUS_IS_OK(status)) { + printf("cli_get_ea_list_path failed (%s)\n", + nt_errstr(status)); + goto out; + } + + /* Ensure the EA we set is there. */ + for (i=0; i Date: Fri, 7 Aug 2015 11:36:47 +0200 Subject: [PATCH 13/30] CVE-2016-0771: s4:librpc: python_dns and python_dcerpc_dnsp doesn't require client bindings BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 Signed-off-by: Stefan Metzmacher Reviewed-by: Garming Sam Reviewed-by: Andrew Bartlett --- librpc/idl/dns.idl | 2 +- librpc/idl/dnsp.idl | 4 ++-- source4/librpc/wscript_build | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/librpc/idl/dns.idl b/librpc/idl/dns.idl index d247e0e..1183bd1 100644 --- a/librpc/idl/dns.idl +++ b/librpc/idl/dns.idl @@ -270,7 +270,7 @@ interface dns /* this is a convenience hook for ndrdump */ - void decode_dns_name_packet( + [nopython] void decode_dns_name_packet( [in] dns_name_packet packet ); } diff --git a/librpc/idl/dnsp.idl b/librpc/idl/dnsp.idl index 4c49001..d705cfc 100644 --- a/librpc/idl/dnsp.idl +++ b/librpc/idl/dnsp.idl @@ -263,11 +263,11 @@ interface dnsp /* these are convenience hooks for ndrdump */ - void decode_DnssrvRpcRecord( + [nopython] void decode_DnssrvRpcRecord( [in] dnsp_DnssrvRpcRecord blob ); - void decode_DnsProperty( + [nopython] void decode_DnsProperty( [in] dnsp_DnsProperty blob ); } diff --git a/source4/librpc/wscript_build b/source4/librpc/wscript_build index 781611e..4da3eaa 100755 --- a/source4/librpc/wscript_build +++ b/source4/librpc/wscript_build @@ -167,7 +167,7 @@ bld.SAMBA_PYTHON('python_echo', bld.SAMBA_PYTHON('python_dns', source='../../librpc/gen_ndr/py_dns.c', - deps='RPC_NDR_DNS pytalloc-util pyrpc_util', + deps='NDR_DNS pytalloc-util pyrpc_util', realname='samba/dcerpc/dns.so' ) @@ -324,7 +324,7 @@ bld.SAMBA_PYTHON('python_dcerpc_drsblobs', bld.SAMBA_PYTHON('python_dcerpc_dnsp', source='../../librpc/gen_ndr/py_dnsp.c', - deps='pytalloc-util pyrpc_util NDR_SECURITY RPC_NDR_DNSP', + deps='pytalloc-util pyrpc_util NDR_SECURITY NDR_DNSP', realname='samba/dcerpc/dnsp.so' ) -- 1.9.1 From efaf50945f9d6bcb21b4e55568e84e085d9b525f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Aug 2015 11:36:47 +0200 Subject: [PATCH 14/30] CVE-2016-0771: librpc: add RPC_NDR_DNSSERVER to dcerpc-samba library RPC_NDR_DNSSERVER is the client interface NDR_DNSP contains just marshalling helpers. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Stefan Metzmacher Reviewed-by: Garming Sam Reviewed-by: Andrew Bartlett --- librpc/wscript_build | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/librpc/wscript_build b/librpc/wscript_build index 1594e72..244da21 100644 --- a/librpc/wscript_build +++ b/librpc/wscript_build @@ -27,7 +27,7 @@ bld.SAMBA_SUBSYSTEM('NDR_NAMED_PIPE_AUTH', bld.SAMBA_SUBSYSTEM('NDR_DNSSERVER', source='gen_ndr/ndr_dnsserver.c ndr/ndr_dnsserver.c', - public_deps='ndr' + public_deps='ndr NDR_DNSP' ) bld.SAMBA_SUBSYSTEM('NDR_DNS', @@ -371,7 +371,7 @@ bld.SAMBA_LIBRARY('ndr-standard', pc_files='ndr_standard.pc', deps='''NDR_SECURITY NDR_LSA NDR_SAMR NDR_NETLOGON NDR_EVENTLOG NDR_DFS NDR_NTSVCS NDR_SVCCTL NDR_INITSHUTDOWN NDR_WKSSVC NDR_SRVSVC NDR_WINREG - NDR_ECHO security NDR_DNS NDR_ATSVC NDR_SPOOLSS NDR_DSSETUP + NDR_ECHO security NDR_DNS NDR_DNSP NDR_ATSVC NDR_SPOOLSS NDR_DSSETUP NDR_SERVER_ID NDR_NOTIFY''', public_deps='ndr', public_headers='gen_ndr/samr.h gen_ndr/ndr_samr.h gen_ndr/lsa.h gen_ndr/netlogon.h gen_ndr/atsvc.h gen_ndr/ndr_atsvc.h gen_ndr/ndr_svcctl.h gen_ndr/svcctl.h', @@ -453,11 +453,6 @@ bld.SAMBA_SUBSYSTEM('RPC_NDR_AUDIOSRV', public_deps='NDR_AUDIOSRV dcerpc-binding' ) -bld.SAMBA_SUBSYSTEM('RPC_NDR_DNS', - source='gen_ndr/ndr_dns_c.c', - public_deps='dcerpc-binding NDR_DNS' - ) - bld.SAMBA_SUBSYSTEM('RPC_NDR_ECHO', source='gen_ndr/ndr_echo_c.c', public_deps='dcerpc-binding NDR_ECHO' @@ -640,11 +635,6 @@ bld.SAMBA_SUBSYSTEM('RPC_NDR_BACKUPKEY', public_deps='dcerpc-binding NDR_BACKUPKEY' ) -bld.SAMBA_SUBSYSTEM('RPC_NDR_DNSP', - source='gen_ndr/ndr_dnsp_c.c', - public_deps='dcerpc-binding NDR_DNSP' - ) - bld.SAMBA_SUBSYSTEM('RPC_NDR_DNSSERVER', source='gen_ndr/ndr_dnsserver_c.c', public_deps='dcerpc-binding ndr-standard' @@ -679,7 +669,7 @@ bld.SAMBA_SUBSYSTEM('RPC_NDR_MDSSVC', bld.SAMBA_LIBRARY('ndr-samba', source=[], deps='''NDR_DRSBLOBS NDR_DRSUAPI NDR_IDMAP NDR_NTLMSSP NDR_SCHANNEL NDR_MGMT - NDR_DNSP NDR_EPMAPPER NDR_XATTR NDR_UNIXINFO NDR_NAMED_PIPE_AUTH NDR_DCOM + NDR_DNSSERVER NDR_EPMAPPER NDR_XATTR NDR_UNIXINFO NDR_NAMED_PIPE_AUTH NDR_DCOM NDR_NTPRINTING NDR_FSRVP NDR_WITNESS NDR_MDSSVC NDR_OPEN_FILES NDR_SMBXSRV''', private_library=True, grouping_library=True @@ -691,7 +681,7 @@ bld.SAMBA_LIBRARY('dcerpc-samba', deps='''RPC_NDR_LSA RPC_NDR_SAMR RPC_NDR_NETLOGON RPC_NDR_EVENTLOG RPC_NDR_DFS RPC_NDR_NTSVCS RPC_NDR_SVCCTL RPC_NDR_INITSHUTDOWN RPC_NDR_WKSSVC RPC_NDR_SRVSVC RPC_NDR_WINREG RPC_NDR_ECHO RPC_NDR_EPMAPPER - RPC_NDR_ATSVC RPC_NDR_SPOOLSS RPC_NDR_DNS''', + RPC_NDR_ATSVC RPC_NDR_SPOOLSS RPC_NDR_DNSSERVER''', public_deps='ndr-standard', private_library=True, grouping_library=True -- 1.9.1 From 7693d683b5fa3ee3e311f4ad21a0237b0d61e3ca Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Aug 2015 11:36:47 +0200 Subject: [PATCH 15/30] CVE-2016-0771: librpc: add ndr_dnsp_string_list_copy() helper function BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Stefan Metzmacher Reviewed-by: Garming Sam Reviewed-by: Andrew Bartlett --- librpc/ndr/ndr_dnsp.c | 24 ++++++++++++++++++++++++ librpc/ndr/ndr_dnsp.h | 4 ++++ 2 files changed, 28 insertions(+) diff --git a/librpc/ndr/ndr_dnsp.c b/librpc/ndr/ndr_dnsp.c index 46141c1..3cb96f9 100644 --- a/librpc/ndr/ndr_dnsp.c +++ b/librpc/ndr/ndr_dnsp.c @@ -222,3 +222,27 @@ enum ndr_err_code ndr_push_dnsp_string_list(struct ndr_push *ndr, int ndr_flags, } return NDR_ERR_SUCCESS; } + +enum ndr_err_code ndr_dnsp_string_list_copy(TALLOC_CTX *mem_ctx, + const struct dnsp_string_list *src, + struct dnsp_string_list *dst) +{ + size_t i; + + dst->count = 0; + dst->str = talloc_zero_array(mem_ctx, const char *, src->count); + if (dst->str == NULL) { + return NDR_ERR_ALLOC; + } + + for (i = 0; i < src->count; i++) { + dst->str[i] = talloc_strdup(dst->str, src->str[i]); + if (dst->str[i] == NULL) { + TALLOC_FREE(dst->str); + return NDR_ERR_ALLOC; + } + } + + dst->count = src->count; + return NDR_ERR_SUCCESS; +} diff --git a/librpc/ndr/ndr_dnsp.h b/librpc/ndr/ndr_dnsp.h index 67f952c..0d56633 100644 --- a/librpc/ndr/ndr_dnsp.h +++ b/librpc/ndr/ndr_dnsp.h @@ -27,3 +27,7 @@ void ndr_print_dnsp_string(struct ndr_print *ndr, const char *name, const char *dns_string); enum ndr_err_code ndr_pull_dnsp_string(struct ndr_pull *ndr, int ndr_flags, const char **string); enum ndr_err_code ndr_push_dnsp_string(struct ndr_push *ndr, int ndr_flags, const char *string); + +enum ndr_err_code ndr_dnsp_string_list_copy(TALLOC_CTX *mem_ctx, + const struct dnsp_string_list *src, + struct dnsp_string_list *dst); -- 1.9.1 From df431a39e4781cf84d119cfbf52aacf29e1dd802 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Aug 2015 11:36:47 +0200 Subject: [PATCH 16/30] CVE-2016-0771: s4:dns_server: fix idl for dns_txt_record From RFC 1035: 3.3.14. TXT RDATA format +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ / TXT-DATA / +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ where: TXT-DATA One or more s. TXT RRs are used to hold descriptive text. The semantics of the text depends on the domain where it is found. Each record contains an array of strings instead of just one string. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Stefan Metzmacher Reviewed-by: Garming Sam Reviewed-by: Andrew Bartlett --- librpc/idl/dns.idl | 7 +++---- librpc/ndr/ndr_dns.c | 27 +++++++++++++++++++++++++++ librpc/wscript_build | 2 +- source4/dns_server/dns_query.c | 15 ++++++--------- source4/dns_server/dns_update.c | 31 ++++++------------------------- 5 files changed, 43 insertions(+), 39 deletions(-) diff --git a/librpc/idl/dns.idl b/librpc/idl/dns.idl index 1183bd1..918073c 100644 --- a/librpc/idl/dns.idl +++ b/librpc/idl/dns.idl @@ -8,7 +8,7 @@ encoding if it doesn't work out */ -import "misc.idl"; +import "misc.idl", "dnsp.idl"; [ helper("librpc/ndr/ndr_dns.h"), helpstring("DNS records"), @@ -163,9 +163,8 @@ interface dns dns_string exchange; } dns_mx_record; - typedef [public] struct { - [value(strlen(txt))] uint8 length; - [charset(DOS)] uint8 txt[length]; + typedef [public,nopull] struct { + dnsp_string_list txt; } dns_txt_record; typedef [public] struct { diff --git a/librpc/ndr/ndr_dns.c b/librpc/ndr/ndr_dns.c index 0b9e3b0..065d992 100644 --- a/librpc/ndr/ndr_dns.c +++ b/librpc/ndr/ndr_dns.c @@ -30,6 +30,7 @@ #include "includes.h" #include "librpc/gen_ndr/ndr_dns.h" #include "librpc/gen_ndr/ndr_misc.h" +#include "librpc/gen_ndr/ndr_dnsp.h" #include "system/locale.h" #include "lib/util/util_net.h" @@ -230,6 +231,29 @@ _PUBLIC_ enum ndr_err_code ndr_push_dns_string(struct ndr_push *ndr, return ndr_push_bytes(ndr, (const uint8_t *)"", 1); } +_PUBLIC_ enum ndr_err_code ndr_pull_dns_txt_record(struct ndr_pull *ndr, int ndr_flags, struct dns_txt_record *r) +{ + NDR_PULL_CHECK_FLAGS(ndr, ndr_flags); + if (ndr_flags & NDR_SCALARS) { + enum ndr_err_code ndr_err; + uint32_t data_size = ndr->data_size; + uint32_t record_size = 0; + ndr_err = ndr_token_retrieve(&ndr->array_size_list, r, + &record_size); + if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + NDR_PULL_NEED_BYTES(ndr, record_size); + ndr->data_size = ndr->offset + record_size; + } + NDR_CHECK(ndr_pull_align(ndr, 1)); + NDR_CHECK(ndr_pull_dnsp_string_list(ndr, NDR_SCALARS, &r->txt)); + NDR_CHECK(ndr_pull_trailer_align(ndr, 1)); + ndr->data_size = data_size; + } + if (ndr_flags & NDR_BUFFERS) { + } + return NDR_ERR_SUCCESS; +} + _PUBLIC_ enum ndr_err_code ndr_push_dns_res_rec(struct ndr_push *ndr, int ndr_flags, const struct dns_res_rec *r) @@ -302,6 +326,9 @@ _PUBLIC_ enum ndr_err_code ndr_pull_dns_res_rec(struct ndr_pull *ndr, NDR_CHECK(ndr_pull_uint16(ndr, NDR_SCALARS, &r->length)); _saved_offset1 = ndr->offset; if (r->length > 0) { + NDR_CHECK(ndr_token_store(ndr, &ndr->array_size_list, + &r->rdata, + r->length)); NDR_CHECK(ndr_pull_set_switch_value(ndr, &r->rdata, r->rr_type)); NDR_CHECK(ndr_pull_dns_rdata(ndr, NDR_SCALARS, diff --git a/librpc/wscript_build b/librpc/wscript_build index 244da21..d376bec 100644 --- a/librpc/wscript_build +++ b/librpc/wscript_build @@ -32,7 +32,7 @@ bld.SAMBA_SUBSYSTEM('NDR_DNSSERVER', bld.SAMBA_SUBSYSTEM('NDR_DNS', source='gen_ndr/ndr_dns.c ndr/ndr_dns.c', - public_deps='ndr' + public_deps='ndr NDR_DNSP' ) bld.SAMBA_SUBSYSTEM('NDR_DSBACKUP', diff --git a/source4/dns_server/dns_query.c b/source4/dns_server/dns_query.c index 4e3c6cc..9e30b71 100644 --- a/source4/dns_server/dns_query.c +++ b/source4/dns_server/dns_query.c @@ -46,8 +46,7 @@ static WERROR create_response_rr(const struct dns_name_question *question, { struct dns_res_rec *ans = *answers; uint16_t ai = *ancount; - char *tmp; - uint32_t i; + enum ndr_err_code ndr_err; ZERO_STRUCT(ans[ai]); @@ -101,14 +100,12 @@ static WERROR create_response_rr(const struct dns_name_question *question, } break; case DNS_QTYPE_TXT: - tmp = talloc_asprintf(ans, "\"%s\"", rec->data.txt.str[0]); - W_ERROR_HAVE_NO_MEMORY(tmp); - for (i=1; idata.txt.count; i++) { - tmp = talloc_asprintf_append_buffer( - tmp, " \"%s\"", rec->data.txt.str[i]); - W_ERROR_HAVE_NO_MEMORY(tmp); + ndr_err = ndr_dnsp_string_list_copy(ans, + &rec->data.txt, + &ans[ai].rdata.txt_record.txt); + if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + return WERR_NOMEM; } - ans[ai].rdata.txt_record.txt = tmp; break; default: DEBUG(0, ("Got unhandled type %u query.\n", rec->wType)); diff --git a/source4/dns_server/dns_update.c b/source4/dns_server/dns_update.c index c002b4d..60a4b36 100644 --- a/source4/dns_server/dns_update.c +++ b/source4/dns_server/dns_update.c @@ -299,9 +299,7 @@ static WERROR dns_rr_to_dnsp(TALLOC_CTX *mem_ctx, const struct dns_res_rec *rrec, struct dnsp_DnssrvRpcRecord *r) { - char *tmp; - char *txt_record_txt; - char *saveptr = NULL; + enum ndr_err_code ndr_err; if (rrec->rr_type == DNS_QTYPE_ALL) { return DNS_ERR(FORMAT_ERROR); @@ -354,28 +352,11 @@ static WERROR dns_rr_to_dnsp(TALLOC_CTX *mem_ctx, W_ERROR_HAVE_NO_MEMORY(r->data.mx.nameTarget); break; case DNS_QTYPE_TXT: - r->data.txt.count = 0; - r->data.txt.str = talloc_array(mem_ctx, const char *, - r->data.txt.count); - W_ERROR_HAVE_NO_MEMORY(r->data.txt.str); - - txt_record_txt = talloc_strdup(r->data.txt.str, - rrec->rdata.txt_record.txt); - W_ERROR_HAVE_NO_MEMORY(txt_record_txt); - - tmp = strtok_r(txt_record_txt, "\"", &saveptr); - while (tmp) { - if (strcmp(tmp, " ") == 0) { - tmp = strtok_r(NULL, "\"", &saveptr); - continue; - } - r->data.txt.str = talloc_realloc(mem_ctx, r->data.txt.str, const char *, - r->data.txt.count+1); - r->data.txt.str[r->data.txt.count] = talloc_strdup(r->data.txt.str, tmp); - W_ERROR_HAVE_NO_MEMORY(r->data.txt.str[r->data.txt.count]); - - r->data.txt.count++; - tmp = strtok_r(NULL, "\"", &saveptr); + ndr_err = ndr_dnsp_string_list_copy(mem_ctx, + &rrec->rdata.txt_record.txt, + &r->data.txt); + if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + return WERR_NOMEM; } break; -- 1.9.1 From 1c69840ef5e50e8f05be1b615c52cbe4cc309c96 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Aug 2015 11:36:47 +0200 Subject: [PATCH 17/30] CVE-2016-0771: dns.idl: make use of dnsp_hinfo BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Stefan Metzmacher Reviewed-by: Garming Sam Reviewed-by: Andrew Bartlett --- librpc/idl/dns.idl | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/librpc/idl/dns.idl b/librpc/idl/dns.idl index 918073c..5435fcf 100644 --- a/librpc/idl/dns.idl +++ b/librpc/idl/dns.idl @@ -152,13 +152,6 @@ interface dns } dns_soa_record; typedef [public] struct { - [value(strlen(cpu))] uint8 cpu_length; - [charset(DOS)] uint8 cpu[cpu_length]; - [value(strlen(os))] uint8 os_length; - [charset(DOS)] uint8 os[os_length]; - } dns_hinfo_record; - - typedef [public] struct { uint16 preference; dns_string exchange; } dns_mx_record; @@ -231,7 +224,7 @@ interface dns [case(DNS_QTYPE_CNAME)] dns_string cname_record; [case(DNS_QTYPE_SOA)] dns_soa_record soa_record; [case(DNS_QTYPE_PTR)] dns_string ptr_record; - [case(DNS_QTYPE_HINFO)] dns_hinfo_record hinfo_record; + [case(DNS_QTYPE_HINFO)] dnsp_hinfo hinfo_record; [case(DNS_QTYPE_MX)] dns_mx_record mx_record; [case(DNS_QTYPE_TXT)] dns_txt_record txt_record; [case(DNS_QTYPE_RP)] dns_rp_record rp_record; -- 1.9.1 From 3196b9e51252f6e59768cebc71d707c596ffb82f Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Wed, 6 Jan 2016 14:12:35 +1300 Subject: [PATCH 18/30] CVE-2016-0771: tests/dns: Modify dns tests to match new IDL BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Garming Sam Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- python/samba/tests/dns.py | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py index 04ac356..6f2bbb5 100644 --- a/python/samba/tests/dns.py +++ b/python/samba/tests/dns.py @@ -20,12 +20,19 @@ import struct import random import socket import samba.ndr as ndr -import samba.dcerpc.dns as dns from samba import credentials, param from samba.tests import TestCase -from samba.dcerpc import dnsp, dnsserver +from samba.dcerpc import dns, dnsp, dnsserver +def make_txt_record(records): + rdata_txt = dns.txt_record() + s_list = dnsp.string_list() + s_list.count = len(records) + s_list.str = records + rdata_txt.txt = s_list + return rdata_txt + class DNSTest(TestCase): def errstr(self, errcode): @@ -411,8 +418,7 @@ class TestDNSUpdates(DNSTest): r.rr_class = dns.DNS_QCLASS_IN r.ttl = 900 r.length = 0xffff - rdata = dns.txt_record() - rdata.txt = '"This is a test"' + rdata = make_txt_record(['"This is a test"']) r.rdata = rdata updates.append(r) p.nscount = len(updates) @@ -432,7 +438,7 @@ class TestDNSUpdates(DNSTest): response = self.dns_transaction_udp(p) self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) self.assertEquals(response.ancount, 1) - self.assertEquals(response.answers[0].rdata.txt, '"This is a test"') + self.assertEquals(response.answers[0].rdata.txt.str[0], '"This is a test"') def test_update_add_two_txt_records(self): "test adding two txt records works" @@ -452,8 +458,8 @@ class TestDNSUpdates(DNSTest): r.rr_class = dns.DNS_QCLASS_IN r.ttl = 900 r.length = 0xffff - rdata = dns.txt_record() - rdata.txt = '"This is a test" "and this is a test, too"' + rdata = make_txt_record(['"This is a test"', + '"and this is a test, too"']) r.rdata = rdata updates.append(r) p.nscount = len(updates) @@ -473,7 +479,8 @@ class TestDNSUpdates(DNSTest): response = self.dns_transaction_udp(p) self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) self.assertEquals(response.ancount, 1) - self.assertEquals(response.answers[0].rdata.txt, '"This is a test" "and this is a test, too"') + self.assertEquals(response.answers[0].rdata.txt.str[0], '"This is a test"') + self.assertEquals(response.answers[0].rdata.txt.str[1], '"and this is a test, too"') def test_delete_record(self): "Test if deleting records works" @@ -497,8 +504,7 @@ class TestDNSUpdates(DNSTest): r.rr_class = dns.DNS_QCLASS_IN r.ttl = 900 r.length = 0xffff - rdata = dns.txt_record() - rdata.txt = '"This is a test"' + rdata = make_txt_record(['"This is a test"']) r.rdata = rdata updates.append(r) p.nscount = len(updates) @@ -534,8 +540,7 @@ class TestDNSUpdates(DNSTest): r.rr_class = dns.DNS_QCLASS_NONE r.ttl = 0 r.length = 0xffff - rdata = dns.txt_record() - rdata.txt = '"This is a test"' + rdata = make_txt_record(['"This is a test"']) r.rdata = rdata updates.append(r) p.nscount = len(updates) @@ -577,8 +582,7 @@ class TestDNSUpdates(DNSTest): r.rr_class = dns.DNS_QCLASS_IN r.ttl = 900 r.length = 0xffff - rdata = dns.txt_record() - rdata.txt = '"This is a test"' + rdata = make_txt_record(['"This is a test"']) r.rdata = rdata updates.append(r) p.nscount = len(updates) @@ -614,8 +618,7 @@ class TestDNSUpdates(DNSTest): r.rr_class = dns.DNS_QCLASS_NONE r.ttl = 0 r.length = 0xffff - rdata = dns.txt_record() - rdata.txt = '"This is a test"' + rdata = make_txt_record(['"This is a test"']) r.rdata = rdata updates.append(r) p.nscount = len(updates) @@ -652,8 +655,7 @@ class TestDNSUpdates(DNSTest): r.rr_class = dns.DNS_QCLASS_IN r.ttl = 900 r.length = 0xffff - rdata = dns.txt_record() - rdata.txt = '"This is a test"' + rdata = make_txt_record(['"This is a test"']) r.rdata = rdata updates.append(r) p.nscount = len(updates) -- 1.9.1 From 18faca0c946d51370e973726cc929c00814fc9f7 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Thu, 21 Jan 2016 16:58:40 +1300 Subject: [PATCH 19/30] CVE-2016-0771: tests/dns: prepare script for further testing BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Garming Sam Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- python/samba/tests/dns.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py index 6f2bbb5..c82020b 100644 --- a/python/samba/tests/dns.py +++ b/python/samba/tests/dns.py @@ -35,6 +35,11 @@ def make_txt_record(records): class DNSTest(TestCase): + def get_loadparm(self): + lp = param.LoadParm() + lp.load(os.getenv("SMB_CONF_PATH")) + return lp + def errstr(self, errcode): "Return a readable error code" string_codes = [ @@ -855,11 +860,6 @@ class TestInvalidQueries(DNSTest): s.close() class TestZones(DNSTest): - def get_loadparm(self): - lp = param.LoadParm() - lp.load(os.getenv("SMB_CONF_PATH")) - return lp - def get_credentials(self, lp): creds = credentials.Credentials() creds.guess(lp) @@ -873,7 +873,7 @@ class TestZones(DNSTest): self.creds = self.get_credentials(self.lp) self.server = os.getenv("SERVER_IP") self.zone = "test.lan" - self.rpc_conn = dnsserver.dnsserver("ncacn_ip_tcp:%s" % (self.server), + self.rpc_conn = dnsserver.dnsserver("ncacn_ip_tcp:%s[sign]" % (self.server), self.lp, self.creds) def tearDown(self): -- 1.9.1 From 51ac36e014cfbf5f7d0efdb65490d66e6edc3d6f Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Thu, 21 Jan 2016 15:43:55 +1300 Subject: [PATCH 20/30] CVE-2016-0771: tests/dns: FORMERR can simply timeout against Windows Two requests with identical parameters which are poorly formatted, can non-deterministically return FORMERR or simply fail to give a response. Setting the timeout to a number allows Windows to succeed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Garming Sam Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- python/samba/tests/dns.py | 63 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 53 insertions(+), 10 deletions(-) diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py index c82020b..3e8c2a3 100644 --- a/python/samba/tests/dns.py +++ b/python/samba/tests/dns.py @@ -24,6 +24,9 @@ from samba import credentials, param from samba.tests import TestCase from samba.dcerpc import dns, dnsp, dnsserver +# This timeout only has relevance when testing against Windows +# Format errors tend to return patchy responses, so a timeout is needed. +timeout = None def make_txt_record(records): rdata_txt = dns.txt_record() @@ -97,7 +100,8 @@ class DNSTest(TestCase): "Helper to get dns domain" return os.getenv('REALM', 'example.com').lower() - def dns_transaction_udp(self, packet, host=os.getenv('SERVER_IP'), dump=False): + def dns_transaction_udp(self, packet, host=os.getenv('SERVER_IP'), + dump=False, timeout=timeout): "send a DNS query and read the reply" s = None try: @@ -105,6 +109,7 @@ class DNSTest(TestCase): if dump: print self.hexdump(send_packet) s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0) + s.settimeout(timeout) s.connect((host, 53)) s.send(send_packet, 0) recv_packet = s.recv(2048, 0) @@ -115,7 +120,8 @@ class DNSTest(TestCase): if s is not None: s.close() - def dns_transaction_tcp(self, packet, host=os.getenv('SERVER_IP'), dump=False): + def dns_transaction_tcp(self, packet, host=os.getenv('SERVER_IP'), + dump=False, timeout=timeout): "send a DNS query and read the reply" s = None try: @@ -123,6 +129,7 @@ class DNSTest(TestCase): if dump: print self.hexdump(send_packet) s = socket.socket(socket.AF_INET, socket.SOCK_STREAM, 0) + s.settimeout(timeout) s.connect((host, 53)) tcp_packet = struct.pack('!H', len(send_packet)) tcp_packet += send_packet @@ -217,8 +224,15 @@ class TestSimpleQueries(DNSTest): questions.append(q) self.finish_name_packet(p, questions) - response = self.dns_transaction_udp(p) - self.assert_dns_rcode_equals(response, dns.DNS_RCODE_FORMERR) + try: + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_FORMERR) + except socket.timeout: + # Windows chooses not to respond to incorrectly formatted queries. + # Although this appears to be non-deterministic even for the same + # request twice, it also appears to be based on a how poorly the + # request is formatted. + pass def test_qtype_all_query(self): "create a QTYPE_ALL query" @@ -256,8 +270,15 @@ class TestSimpleQueries(DNSTest): questions.append(q) self.finish_name_packet(p, questions) - response = self.dns_transaction_udp(p) - self.assert_dns_rcode_equals(response, dns.DNS_RCODE_NOTIMP) + try: + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_NOTIMP) + except socket.timeout: + # Windows chooses not to respond to incorrectly formatted queries. + # Although this appears to be non-deterministic even for the same + # request twice, it also appears to be based on a how poorly the + # request is formatted. + pass # Only returns an authority section entry in BIND and Win DNS # FIXME: Enable one Samba implements this feature @@ -310,8 +331,15 @@ class TestDNSUpdates(DNSTest): updates.append(u) self.finish_name_packet(p, updates) - response = self.dns_transaction_udp(p) - self.assert_dns_rcode_equals(response, dns.DNS_RCODE_FORMERR) + try: + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_FORMERR) + except socket.timeout: + # Windows chooses not to respond to incorrectly formatted queries. + # Although this appears to be non-deterministic even for the same + # request twice, it also appears to be based on a how poorly the + # request is formatted. + pass def test_update_wrong_qclass(self): "create update with DNS_QCLASS_NONE" @@ -349,8 +377,15 @@ class TestDNSUpdates(DNSTest): p.ancount = len(prereqs) p.answers = prereqs - response = self.dns_transaction_udp(p) - self.assert_dns_rcode_equals(response, dns.DNS_RCODE_FORMERR) + try: + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_FORMERR) + except socket.timeout: + # Windows chooses not to respond to incorrectly formatted queries. + # Although this appears to be non-deterministic even for the same + # request twice, it also appears to be based on a how poorly the + # request is formatted. + pass # I'd love to test this one, but it segfaults. :) # def test_update_prereq_with_non_null_length(self): @@ -833,6 +868,7 @@ class TestInvalidQueries(DNSTest): def test_one_a_reply(self): "send a reply instead of a query" + global timeout p = self.make_name_packet(dns.DNS_OPCODE_QUERY) questions = [] @@ -848,6 +884,7 @@ class TestInvalidQueries(DNSTest): try: send_packet = ndr.ndr_pack(p) s = socket.socket(socket.AF_INET, socket.SOCK_STREAM, 0) + s.settimeout(timeout) host=os.getenv('SERVER_IP') s.connect((host, 53)) tcp_packet = struct.pack('!H', len(send_packet)) @@ -855,6 +892,12 @@ class TestInvalidQueries(DNSTest): s.send(tcp_packet, 0) recv_packet = s.recv(0xffff + 2, 0) self.assertEquals(0, len(recv_packet)) + except socket.timeout: + # Windows chooses not to respond to incorrectly formatted queries. + # Although this appears to be non-deterministic even for the same + # request twice, it also appears to be based on a how poorly the + # request is formatted. + pass finally: if s is not None: s.close() -- 1.9.1 From 9f7a2a1fef9735a15163b73e13f38e1666601666 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Thu, 21 Jan 2016 17:08:18 +1300 Subject: [PATCH 21/30] CVE-2016-0771: tests/dns: Add a comment regarding odd Windows behaviour BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Garming Sam Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- python/samba/tests/dns.py | 1 + 1 file changed, 1 insertion(+) diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py index 3e8c2a3..5f9f3e2 100644 --- a/python/samba/tests/dns.py +++ b/python/samba/tests/dns.py @@ -963,6 +963,7 @@ class TestZones(DNSTest): self.finish_name_packet(p, questions) response = self.dns_transaction_udp(p) + # Windows returns OK while BIND logically seems to return NXDOMAIN self.assert_dns_rcode_equals(response, dns.DNS_RCODE_NXDOMAIN) self.assert_dns_opcode_equals(response, dns.DNS_OPCODE_QUERY) self.assertEquals(response.ancount, 0) -- 1.9.1 From 4011a52e1645ef00c21ba27a0204de08b7ec2108 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Tue, 15 Dec 2015 17:22:32 +1300 Subject: [PATCH 22/30] CVE-2016-0771: tests/dns: restore formerly segfaulting test This was on the client side, due the a strlen(NULL) on the previously DOS-encoded TXT field. With a new IDL structure, this segfault no longer exists. Note that both Samba and Windows return NXRRSET instead of FORMERR. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Garming Sam Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- python/samba/tests/dns.py | 51 +++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py index 5f9f3e2..f05ab05 100644 --- a/python/samba/tests/dns.py +++ b/python/samba/tests/dns.py @@ -387,32 +387,31 @@ class TestDNSUpdates(DNSTest): # request is formatted. pass -# I'd love to test this one, but it segfaults. :) -# def test_update_prereq_with_non_null_length(self): -# "test update with a non-null length" -# p = self.make_name_packet(dns.DNS_OPCODE_UPDATE) -# updates = [] -# -# name = self.get_dns_domain() -# -# u = self.make_name_question(name, dns.DNS_QTYPE_SOA, dns.DNS_QCLASS_IN) -# updates.append(u) -# self.finish_name_packet(p, updates) -# -# prereqs = [] -# r = dns.res_rec() -# r.name = "%s.%s" % (os.getenv('SERVER'), self.get_dns_domain()) -# r.rr_type = dns.DNS_QTYPE_TXT -# r.rr_class = dns.DNS_QCLASS_ANY -# r.ttl = 0 -# r.length = 1 -# prereqs.append(r) -# -# p.ancount = len(prereqs) -# p.answers = prereqs -# -# response = self.dns_transaction_udp(p) -# self.assert_dns_rcode_equals(response, dns.DNS_RCODE_FORMERR) + def test_update_prereq_with_non_null_length(self): + "test update with a non-null length" + p = self.make_name_packet(dns.DNS_OPCODE_UPDATE) + updates = [] + + name = self.get_dns_domain() + + u = self.make_name_question(name, dns.DNS_QTYPE_SOA, dns.DNS_QCLASS_IN) + updates.append(u) + self.finish_name_packet(p, updates) + + prereqs = [] + r = dns.res_rec() + r.name = "%s.%s" % (os.getenv('SERVER'), self.get_dns_domain()) + r.rr_type = dns.DNS_QTYPE_TXT + r.rr_class = dns.DNS_QCLASS_ANY + r.ttl = 0 + r.length = 1 + prereqs.append(r) + + p.ancount = len(prereqs) + p.answers = prereqs + + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_FORMERR) def test_update_prereq_nonexisting_name(self): "test update with a nonexisting name" -- 1.9.1 From 3bca5fcee8d8847297d96795054c2c0696419846 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Thu, 21 Jan 2016 10:25:44 +1300 Subject: [PATCH 23/30] CVE-2016-0771: tests/dns: Correct error code for formerly unrun test Both Samba and Windows returned NXRRSET BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Garming Sam Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- python/samba/tests/dns.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py index f05ab05..d8c488e 100644 --- a/python/samba/tests/dns.py +++ b/python/samba/tests/dns.py @@ -411,7 +411,7 @@ class TestDNSUpdates(DNSTest): p.answers = prereqs response = self.dns_transaction_udp(p) - self.assert_dns_rcode_equals(response, dns.DNS_RCODE_FORMERR) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_NXRRSET) def test_update_prereq_nonexisting_name(self): "test update with a nonexisting name" -- 1.9.1 From 63103d1035ad8857e1cbb932a51f8c36dcdf9f34 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Mon, 18 Jan 2016 12:39:46 +1300 Subject: [PATCH 24/30] CVE-2016-0771: tests/dns: Add some more test cases for TXT records BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Garming Sam Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- python/samba/tests/dns.py | 110 ++++++++++++++++++++++++++++++---------------- 1 file changed, 71 insertions(+), 39 deletions(-) diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py index d8c488e..2033af8 100644 --- a/python/samba/tests/dns.py +++ b/python/samba/tests/dns.py @@ -439,37 +439,35 @@ class TestDNSUpdates(DNSTest): response = self.dns_transaction_udp(p) self.assert_dns_rcode_equals(response, dns.DNS_RCODE_NXRRSET) - def test_update_add_txt_record(self): - "test adding records works" + def make_txt_update(self, prefix, txt_array): p = self.make_name_packet(dns.DNS_OPCODE_UPDATE) updates = [] name = self.get_dns_domain() - u = self.make_name_question(name, dns.DNS_QTYPE_SOA, dns.DNS_QCLASS_IN) updates.append(u) self.finish_name_packet(p, updates) updates = [] r = dns.res_rec() - r.name = "textrec.%s" % self.get_dns_domain() + r.name = "%s.%s" % (prefix, self.get_dns_domain()) r.rr_type = dns.DNS_QTYPE_TXT r.rr_class = dns.DNS_QCLASS_IN r.ttl = 900 r.length = 0xffff - rdata = make_txt_record(['"This is a test"']) + rdata = make_txt_record(txt_array) r.rdata = rdata updates.append(r) p.nscount = len(updates) p.nsrecs = updates - response = self.dns_transaction_udp(p) - self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + return p + def check_query_txt(self, prefix, txt_array): + name = "%s.%s" % (prefix, self.get_dns_domain()) p = self.make_name_packet(dns.DNS_OPCODE_QUERY) questions = [] - name = "textrec.%s" % self.get_dns_domain() q = self.make_name_question(name, dns.DNS_QTYPE_TXT, dns.DNS_QCLASS_IN) questions.append(q) @@ -477,49 +475,83 @@ class TestDNSUpdates(DNSTest): response = self.dns_transaction_udp(p) self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) self.assertEquals(response.ancount, 1) - self.assertEquals(response.answers[0].rdata.txt.str[0], '"This is a test"') + self.assertEquals(response.answers[0].rdata.txt.str, txt_array) - def test_update_add_two_txt_records(self): - "test adding two txt records works" - p = self.make_name_packet(dns.DNS_OPCODE_UPDATE) - updates = [] + def test_update_add_txt_record(self): + "test adding records works" + prefix, txt = 'textrec', ['"This is a test"'] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, txt) - name = self.get_dns_domain() + def test_update_add_null_padded_txt_record(self): + "test adding records works" + prefix, txt = 'pad1textrec', ['"This is a test"', '', ''] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, txt) - u = self.make_name_question(name, dns.DNS_QTYPE_SOA, dns.DNS_QCLASS_IN) - updates.append(u) - self.finish_name_packet(p, updates) + prefix, txt = 'pad2textrec', ['"This is a test"', '', '', 'more text'] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, txt) - updates = [] - r = dns.res_rec() - r.name = "textrec2.%s" % self.get_dns_domain() - r.rr_type = dns.DNS_QTYPE_TXT - r.rr_class = dns.DNS_QCLASS_IN - r.ttl = 900 - r.length = 0xffff - rdata = make_txt_record(['"This is a test"', - '"and this is a test, too"']) - r.rdata = rdata - updates.append(r) - p.nscount = len(updates) - p.nsrecs = updates + prefix, txt = 'pad3textrec', ['', '', '"This is a test"'] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, txt) + # Test is incomplete due to strlen against txt records + def test_update_add_null_char_txt_record(self): + "test adding records works" + prefix, txt = 'nulltextrec', ['NULL\x00BYTE'] + p = self.make_txt_update(prefix, txt) response = self.dns_transaction_udp(p) self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, ['NULL']) - p = self.make_name_packet(dns.DNS_OPCODE_QUERY) - questions = [] + prefix, txt = 'nulltextrec2', ['NULL\x00BYTE', 'NULL\x00BYTE'] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, ['NULL', 'NULL']) - name = "textrec2.%s" % self.get_dns_domain() - q = self.make_name_question(name, dns.DNS_QTYPE_TXT, dns.DNS_QCLASS_IN) - questions.append(q) + def test_update_add_hex_char_txt_record(self): + "test adding records works" + prefix, txt = 'hextextrec', ['HIGH\xFFBYTE'] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, txt) - self.finish_name_packet(p, questions) + def test_update_add_slash_txt_record(self): + "test adding records works" + prefix, txt = 'slashtextrec', ['Th\\=is=is a test'] + p = self.make_txt_update(prefix, txt) response = self.dns_transaction_udp(p) self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) - self.assertEquals(response.ancount, 1) - self.assertEquals(response.answers[0].rdata.txt.str[0], '"This is a test"') - self.assertEquals(response.answers[0].rdata.txt.str[1], '"and this is a test, too"') + self.check_query_txt(prefix, txt) + + def test_update_add_two_txt_records(self): + "test adding two txt records works" + prefix, txt = 'textrec2', ['"This is a test"', + '"and this is a test, too"'] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, txt) + + def test_update_add_empty_txt_records(self): + "test adding two txt records works" + prefix, txt = 'emptytextrec', [] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, txt) def test_delete_record(self): "Test if deleting records works" -- 1.9.1 From eb4684895c347bfb6675e11ef8bf2e737db71657 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Wed, 27 Jan 2016 17:41:44 +1300 Subject: [PATCH 25/30] CVE-2016-0771: tests/dns: modify tests to check via RPC This checks that TXT records added over DNS, look the same over RPC. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Garming Sam Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- python/samba/tests/dns.py | 266 ++++++++++++++++++++++++++++------------------ 1 file changed, 160 insertions(+), 106 deletions(-) diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py index 2033af8..25897a7 100644 --- a/python/samba/tests/dns.py +++ b/python/samba/tests/dns.py @@ -142,6 +142,44 @@ class DNSTest(TestCase): if s is not None: s.close() + def make_txt_update(self, prefix, txt_array): + p = self.make_name_packet(dns.DNS_OPCODE_UPDATE) + updates = [] + + name = self.get_dns_domain() + u = self.make_name_question(name, dns.DNS_QTYPE_SOA, dns.DNS_QCLASS_IN) + updates.append(u) + self.finish_name_packet(p, updates) + + updates = [] + r = dns.res_rec() + r.name = "%s.%s" % (prefix, self.get_dns_domain()) + r.rr_type = dns.DNS_QTYPE_TXT + r.rr_class = dns.DNS_QCLASS_IN + r.ttl = 900 + r.length = 0xffff + rdata = make_txt_record(txt_array) + r.rdata = rdata + updates.append(r) + p.nscount = len(updates) + p.nsrecs = updates + + return p + + def check_query_txt(self, prefix, txt_array): + name = "%s.%s" % (prefix, self.get_dns_domain()) + p = self.make_name_packet(dns.DNS_OPCODE_QUERY) + questions = [] + + q = self.make_name_question(name, dns.DNS_QTYPE_TXT, dns.DNS_QCLASS_IN) + questions.append(q) + + self.finish_name_packet(p, questions) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.assertEquals(response.ancount, 1) + self.assertEquals(response.answers[0].rdata.txt.str, txt_array) + class TestSimpleQueries(DNSTest): def test_one_a_query(self): @@ -439,44 +477,6 @@ class TestDNSUpdates(DNSTest): response = self.dns_transaction_udp(p) self.assert_dns_rcode_equals(response, dns.DNS_RCODE_NXRRSET) - def make_txt_update(self, prefix, txt_array): - p = self.make_name_packet(dns.DNS_OPCODE_UPDATE) - updates = [] - - name = self.get_dns_domain() - u = self.make_name_question(name, dns.DNS_QTYPE_SOA, dns.DNS_QCLASS_IN) - updates.append(u) - self.finish_name_packet(p, updates) - - updates = [] - r = dns.res_rec() - r.name = "%s.%s" % (prefix, self.get_dns_domain()) - r.rr_type = dns.DNS_QTYPE_TXT - r.rr_class = dns.DNS_QCLASS_IN - r.ttl = 900 - r.length = 0xffff - rdata = make_txt_record(txt_array) - r.rdata = rdata - updates.append(r) - p.nscount = len(updates) - p.nsrecs = updates - - return p - - def check_query_txt(self, prefix, txt_array): - name = "%s.%s" % (prefix, self.get_dns_domain()) - p = self.make_name_packet(dns.DNS_OPCODE_QUERY) - questions = [] - - q = self.make_name_question(name, dns.DNS_QTYPE_TXT, dns.DNS_QCLASS_IN) - questions.append(q) - - self.finish_name_packet(p, questions) - response = self.dns_transaction_udp(p) - self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) - self.assertEquals(response.ancount, 1) - self.assertEquals(response.answers[0].rdata.txt.str, txt_array) - def test_update_add_txt_record(self): "test adding records works" prefix, txt = 'textrec', ['"This is a test"'] @@ -485,74 +485,6 @@ class TestDNSUpdates(DNSTest): self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) self.check_query_txt(prefix, txt) - def test_update_add_null_padded_txt_record(self): - "test adding records works" - prefix, txt = 'pad1textrec', ['"This is a test"', '', ''] - p = self.make_txt_update(prefix, txt) - response = self.dns_transaction_udp(p) - self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) - self.check_query_txt(prefix, txt) - - prefix, txt = 'pad2textrec', ['"This is a test"', '', '', 'more text'] - p = self.make_txt_update(prefix, txt) - response = self.dns_transaction_udp(p) - self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) - self.check_query_txt(prefix, txt) - - prefix, txt = 'pad3textrec', ['', '', '"This is a test"'] - p = self.make_txt_update(prefix, txt) - response = self.dns_transaction_udp(p) - self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) - self.check_query_txt(prefix, txt) - - # Test is incomplete due to strlen against txt records - def test_update_add_null_char_txt_record(self): - "test adding records works" - prefix, txt = 'nulltextrec', ['NULL\x00BYTE'] - p = self.make_txt_update(prefix, txt) - response = self.dns_transaction_udp(p) - self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) - self.check_query_txt(prefix, ['NULL']) - - prefix, txt = 'nulltextrec2', ['NULL\x00BYTE', 'NULL\x00BYTE'] - p = self.make_txt_update(prefix, txt) - response = self.dns_transaction_udp(p) - self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) - self.check_query_txt(prefix, ['NULL', 'NULL']) - - def test_update_add_hex_char_txt_record(self): - "test adding records works" - prefix, txt = 'hextextrec', ['HIGH\xFFBYTE'] - p = self.make_txt_update(prefix, txt) - response = self.dns_transaction_udp(p) - self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) - self.check_query_txt(prefix, txt) - - def test_update_add_slash_txt_record(self): - "test adding records works" - prefix, txt = 'slashtextrec', ['Th\\=is=is a test'] - p = self.make_txt_update(prefix, txt) - response = self.dns_transaction_udp(p) - self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) - self.check_query_txt(prefix, txt) - - def test_update_add_two_txt_records(self): - "test adding two txt records works" - prefix, txt = 'textrec2', ['"This is a test"', - '"and this is a test, too"'] - p = self.make_txt_update(prefix, txt) - response = self.dns_transaction_udp(p) - self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) - self.check_query_txt(prefix, txt) - - def test_update_add_empty_txt_records(self): - "test adding two txt records works" - prefix, txt = 'emptytextrec', [] - p = self.make_txt_update(prefix, txt) - response = self.dns_transaction_udp(p) - self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) - self.check_query_txt(prefix, txt) - def test_delete_record(self): "Test if deleting records works" @@ -1012,7 +944,129 @@ class TestZones(DNSTest): self.assert_dns_opcode_equals(response, dns.DNS_OPCODE_QUERY) self.assertEquals(response.ancount, 0) +class TestRPCRoundtrip(DNSTest): + def get_credentials(self, lp): + creds = credentials.Credentials() + creds.guess(lp) + creds.set_machine_account(lp) + creds.set_krb_forwardable(credentials.NO_KRB_FORWARDABLE) + return creds + + def setUp(self): + super(TestRPCRoundtrip, self).setUp() + self.lp = self.get_loadparm() + self.creds = self.get_credentials(self.lp) + self.server = os.getenv("SERVER_IP") + self.rpc_conn = dnsserver.dnsserver("ncacn_ip_tcp:%s[sign]" % (self.server), + self.lp, self.creds) + + def tearDown(self): + super(TestRPCRoundtrip, self).tearDown() + + def test_update_add_null_padded_txt_record(self): + "test adding records works" + prefix, txt = 'pad1textrec', ['"This is a test"', '', ''] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, txt) + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.get_dns_domain(), + "%s.%s" % (prefix, self.get_dns_domain()), + dnsp.DNS_TYPE_TXT, '"\\"This is a test\\"" "" ""')) + prefix, txt = 'pad2textrec', ['"This is a test"', '', '', 'more text'] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, txt) + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.get_dns_domain(), + "%s.%s" % (prefix, self.get_dns_domain()), + dnsp.DNS_TYPE_TXT, '"\\"This is a test\\"" "" "" "more text"')) + + prefix, txt = 'pad3textrec', ['', '', '"This is a test"'] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, txt) + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.get_dns_domain(), + "%s.%s" % (prefix, self.get_dns_domain()), + dnsp.DNS_TYPE_TXT, '"" "" "\\"This is a test\\""')) + + # Test is incomplete due to strlen against txt records + def test_update_add_null_char_txt_record(self): + "test adding records works" + prefix, txt = 'nulltextrec', ['NULL\x00BYTE'] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, ['NULL']) + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.get_dns_domain(), + "%s.%s" % (prefix, self.get_dns_domain()), + dnsp.DNS_TYPE_TXT, '"NULL"')) + + prefix, txt = 'nulltextrec2', ['NULL\x00BYTE', 'NULL\x00BYTE'] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, ['NULL', 'NULL']) + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.get_dns_domain(), + "%s.%s" % (prefix, self.get_dns_domain()), + dnsp.DNS_TYPE_TXT, '"NULL" "NULL"')) + + def test_update_add_hex_char_txt_record(self): + "test adding records works" + prefix, txt = 'hextextrec', ['HIGH\xFFBYTE'] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, txt) + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.get_dns_domain(), + "%s.%s" % (prefix, self.get_dns_domain()), + dnsp.DNS_TYPE_TXT, '"HIGH\xFFBYTE"')) + + def test_update_add_slash_txt_record(self): + "test adding records works" + prefix, txt = 'slashtextrec', ['Th\\=is=is a test'] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, txt) + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.get_dns_domain(), + "%s.%s" % (prefix, self.get_dns_domain()), + dnsp.DNS_TYPE_TXT, '"Th\\\\=is=is a test"')) + + def test_update_add_two_txt_records(self): + "test adding two txt records works" + prefix, txt = 'textrec2', ['"This is a test"', + '"and this is a test, too"'] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, txt) + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.get_dns_domain(), + "%s.%s" % (prefix, self.get_dns_domain()), + dnsp.DNS_TYPE_TXT, '"\\"This is a test\\""' + + ' "\\"and this is a test, too\\""')) + + def test_update_add_empty_txt_records(self): + "test adding two txt records works" + prefix, txt = 'emptytextrec', [] + p = self.make_txt_update(prefix, txt) + response = self.dns_transaction_udp(p) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) + self.check_query_txt(prefix, txt) + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.get_dns_domain(), + "%s.%s" % (prefix, self.get_dns_domain()), + dnsp.DNS_TYPE_TXT, '')) if __name__ == "__main__": import unittest -- 1.9.1 From 2b4c7dbc15465911be35a1bdb1f10e396118fb1c Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Thu, 28 Jan 2016 12:36:43 +1300 Subject: [PATCH 26/30] CVE-2016-0771: dnsserver: don't force UTF-8 for TXT While using a charset is not entirely logical, it allows testing of non UTF-8 data (like inserting 0xFF into the TXT string). BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Garming Sam Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- librpc/idl/dnsserver.idl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/librpc/idl/dnsserver.idl b/librpc/idl/dnsserver.idl index ca9c371..c7742e7 100644 --- a/librpc/idl/dnsserver.idl +++ b/librpc/idl/dnsserver.idl @@ -73,7 +73,7 @@ import "misc.idl", "dnsp.idl"; typedef [public,gensize] struct { [value(strlen(str))] uint8 len; - [charset(UTF8)] uint8 str[len]; + [charset(UNIX)] uint8 str[len]; } DNS_RPC_NAME; -- 1.9.1 From ad5e885c2b0f58888237b409076113d4b06686db Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Thu, 28 Jan 2016 12:54:58 +1300 Subject: [PATCH 27/30] CVE-2016-0771: tests/dns: RPC => DNS roundtrip test Make sure that TXT entries stored via RPC come out the same in DNS. This has one caveat in that adding over RPC in Windows eats slashes, and so fails there. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Garming Sam Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- python/samba/tests/dns.py | 202 +++++++++++++++++++++++++++++++++++++++++++--- source4/selftest/tests.py | 1 + 2 files changed, 190 insertions(+), 13 deletions(-) diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py index 25897a7..3e655c3 100644 --- a/python/samba/tests/dns.py +++ b/python/samba/tests/dns.py @@ -23,6 +23,7 @@ import samba.ndr as ndr from samba import credentials, param from samba.tests import TestCase from samba.dcerpc import dns, dnsp, dnsserver +from samba.netcmd.dns import TXTRecord, dns_record_match, data_to_dns_record # This timeout only has relevance when testing against Windows # Format errors tend to return patchy responses, so a timeout is needed. @@ -879,7 +880,7 @@ class TestZones(DNSTest): self.creds = self.get_credentials(self.lp) self.server = os.getenv("SERVER_IP") self.zone = "test.lan" - self.rpc_conn = dnsserver.dnsserver("ncacn_ip_tcp:%s[sign]" % (self.server), + self.rpc_conn = dnsserver.dnsserver("ncacn_ip_tcp:%s[sign]" % (self.server_ip), self.lp, self.creds) def tearDown(self): @@ -899,7 +900,7 @@ class TestZones(DNSTest): zone_create.dwDpFlags = dnsserver.DNS_DP_DOMAIN_DEFAULT self.rpc_conn.DnssrvOperation2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, 0, - self.server, + self.server_ip, None, 0, 'ZoneCreate', @@ -909,7 +910,7 @@ class TestZones(DNSTest): def delete_zone(self, zone): self.rpc_conn.DnssrvOperation2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, 0, - self.server, + self.server_ip, zone, 0, 'DeleteZoneFromDs', @@ -957,12 +958,31 @@ class TestRPCRoundtrip(DNSTest): self.lp = self.get_loadparm() self.creds = self.get_credentials(self.lp) self.server = os.getenv("SERVER_IP") - self.rpc_conn = dnsserver.dnsserver("ncacn_ip_tcp:%s[sign]" % (self.server), + self.rpc_conn = dnsserver.dnsserver("ncacn_ip_tcp:%s[sign]" % (self.server_ip), self.lp, self.creds) def tearDown(self): super(TestRPCRoundtrip, self).tearDown() + def test_update_add_txt_rpc_to_dns(self): + prefix, txt = 'rpctextrec', ['"This is a test"'] + + name = "%s.%s" % (prefix, self.get_dns_domain()) + + rec = data_to_dns_record(dnsp.DNS_TYPE_TXT, '"\\"This is a test\\""') + add_rec_buf = dnsserver.DNS_RPC_RECORD_BUF() + add_rec_buf.rec = rec + try: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, add_rec_buf, None) + + self.check_query_txt(prefix, txt) + finally: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, None, add_rec_buf) + def test_update_add_null_padded_txt_record(self): "test adding records works" prefix, txt = 'pad1textrec', ['"This is a test"', '', ''] @@ -970,7 +990,7 @@ class TestRPCRoundtrip(DNSTest): response = self.dns_transaction_udp(p) self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) self.check_query_txt(prefix, txt) - self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server_ip, self.get_dns_domain(), "%s.%s" % (prefix, self.get_dns_domain()), dnsp.DNS_TYPE_TXT, '"\\"This is a test\\"" "" ""')) @@ -980,7 +1000,7 @@ class TestRPCRoundtrip(DNSTest): response = self.dns_transaction_udp(p) self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) self.check_query_txt(prefix, txt) - self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server_ip, self.get_dns_domain(), "%s.%s" % (prefix, self.get_dns_domain()), dnsp.DNS_TYPE_TXT, '"\\"This is a test\\"" "" "" "more text"')) @@ -990,11 +1010,66 @@ class TestRPCRoundtrip(DNSTest): response = self.dns_transaction_udp(p) self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) self.check_query_txt(prefix, txt) - self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server_ip, self.get_dns_domain(), "%s.%s" % (prefix, self.get_dns_domain()), dnsp.DNS_TYPE_TXT, '"" "" "\\"This is a test\\""')) + def test_update_add_padding_rpc_to_dns(self): + prefix, txt = 'pad1textrec', ['"This is a test"', '', ''] + prefix = 'rpc' + prefix + name = "%s.%s" % (prefix, self.get_dns_domain()) + + rec = data_to_dns_record(dnsp.DNS_TYPE_TXT, '"\\"This is a test\\"" "" ""') + add_rec_buf = dnsserver.DNS_RPC_RECORD_BUF() + add_rec_buf.rec = rec + try: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, add_rec_buf, None) + + self.check_query_txt(prefix, txt) + finally: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, None, add_rec_buf) + + prefix, txt = 'pad2textrec', ['"This is a test"', '', '', 'more text'] + prefix = 'rpc' + prefix + name = "%s.%s" % (prefix, self.get_dns_domain()) + + rec = data_to_dns_record(dnsp.DNS_TYPE_TXT, '"\\"This is a test\\"" "" "" "more text"') + add_rec_buf = dnsserver.DNS_RPC_RECORD_BUF() + add_rec_buf.rec = rec + try: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, add_rec_buf, None) + + self.check_query_txt(prefix, txt) + finally: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, None, add_rec_buf) + + prefix, txt = 'pad3textrec', ['', '', '"This is a test"'] + prefix = 'rpc' + prefix + name = "%s.%s" % (prefix, self.get_dns_domain()) + + rec = data_to_dns_record(dnsp.DNS_TYPE_TXT, '"" "" "\\"This is a test\\""') + add_rec_buf = dnsserver.DNS_RPC_RECORD_BUF() + add_rec_buf.rec = rec + try: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, add_rec_buf, None) + + self.check_query_txt(prefix, txt) + finally: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, None, add_rec_buf) + # Test is incomplete due to strlen against txt records def test_update_add_null_char_txt_record(self): "test adding records works" @@ -1003,7 +1078,7 @@ class TestRPCRoundtrip(DNSTest): response = self.dns_transaction_udp(p) self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) self.check_query_txt(prefix, ['NULL']) - self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server_ip, self.get_dns_domain(), "%s.%s" % (prefix, self.get_dns_domain()), dnsp.DNS_TYPE_TXT, '"NULL"')) @@ -1013,11 +1088,30 @@ class TestRPCRoundtrip(DNSTest): response = self.dns_transaction_udp(p) self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) self.check_query_txt(prefix, ['NULL', 'NULL']) - self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server_ip, self.get_dns_domain(), "%s.%s" % (prefix, self.get_dns_domain()), dnsp.DNS_TYPE_TXT, '"NULL" "NULL"')) + def test_update_add_null_char_rpc_to_dns(self): + prefix, txt = 'nulltextrec', ['NULL\x00BYTE'] + prefix = 'rpc' + prefix + name = "%s.%s" % (prefix, self.get_dns_domain()) + + rec = data_to_dns_record(dnsp.DNS_TYPE_TXT, '"NULL"') + add_rec_buf = dnsserver.DNS_RPC_RECORD_BUF() + add_rec_buf.rec = rec + try: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, add_rec_buf, None) + + self.check_query_txt(prefix, ['NULL']) + finally: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, None, add_rec_buf) + def test_update_add_hex_char_txt_record(self): "test adding records works" prefix, txt = 'hextextrec', ['HIGH\xFFBYTE'] @@ -1025,11 +1119,30 @@ class TestRPCRoundtrip(DNSTest): response = self.dns_transaction_udp(p) self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) self.check_query_txt(prefix, txt) - self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server_ip, self.get_dns_domain(), "%s.%s" % (prefix, self.get_dns_domain()), dnsp.DNS_TYPE_TXT, '"HIGH\xFFBYTE"')) + def test_update_add_hex_rpc_to_dns(self): + prefix, txt = 'hextextrec', ['HIGH\xFFBYTE'] + prefix = 'rpc' + prefix + name = "%s.%s" % (prefix, self.get_dns_domain()) + + rec = data_to_dns_record(dnsp.DNS_TYPE_TXT, '"HIGH\xFFBYTE"') + add_rec_buf = dnsserver.DNS_RPC_RECORD_BUF() + add_rec_buf.rec = rec + try: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, add_rec_buf, None) + + self.check_query_txt(prefix, txt) + finally: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, None, add_rec_buf) + def test_update_add_slash_txt_record(self): "test adding records works" prefix, txt = 'slashtextrec', ['Th\\=is=is a test'] @@ -1037,11 +1150,33 @@ class TestRPCRoundtrip(DNSTest): response = self.dns_transaction_udp(p) self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) self.check_query_txt(prefix, txt) - self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server_ip, self.get_dns_domain(), "%s.%s" % (prefix, self.get_dns_domain()), dnsp.DNS_TYPE_TXT, '"Th\\\\=is=is a test"')) + # This test fails against Windows as it eliminates slashes in RPC + # One typical use for a slash is in records like 'var=value' to + # escape '=' characters. + def test_update_add_slash_rpc_to_dns(self): + prefix, txt = 'slashtextrec', ['Th\\=is=is a test'] + prefix = 'rpc' + prefix + name = "%s.%s" % (prefix, self.get_dns_domain()) + + rec = data_to_dns_record(dnsp.DNS_TYPE_TXT, '"Th\\\\=is=is a test"') + add_rec_buf = dnsserver.DNS_RPC_RECORD_BUF() + add_rec_buf.rec = rec + try: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, add_rec_buf, None) + + self.check_query_txt(prefix, txt) + finally: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, None, add_rec_buf) + def test_update_add_two_txt_records(self): "test adding two txt records works" prefix, txt = 'textrec2', ['"This is a test"', @@ -1050,12 +1185,34 @@ class TestRPCRoundtrip(DNSTest): response = self.dns_transaction_udp(p) self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) self.check_query_txt(prefix, txt) - self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server_ip, self.get_dns_domain(), "%s.%s" % (prefix, self.get_dns_domain()), dnsp.DNS_TYPE_TXT, '"\\"This is a test\\""' + ' "\\"and this is a test, too\\""')) + def test_update_add_two_rpc_to_dns(self): + prefix, txt = 'textrec2', ['"This is a test"', + '"and this is a test, too"'] + prefix = 'rpc' + prefix + name = "%s.%s" % (prefix, self.get_dns_domain()) + + rec = data_to_dns_record(dnsp.DNS_TYPE_TXT, + '"\\"This is a test\\""' + + ' "\\"and this is a test, too\\""') + add_rec_buf = dnsserver.DNS_RPC_RECORD_BUF() + add_rec_buf.rec = rec + try: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, add_rec_buf, None) + + self.check_query_txt(prefix, txt) + finally: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, None, add_rec_buf) + def test_update_add_empty_txt_records(self): "test adding two txt records works" prefix, txt = 'emptytextrec', [] @@ -1063,11 +1220,30 @@ class TestRPCRoundtrip(DNSTest): response = self.dns_transaction_udp(p) self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK) self.check_query_txt(prefix, txt) - self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server, + self.assertIsNotNone(dns_record_match(self.rpc_conn, self.server_ip, self.get_dns_domain(), "%s.%s" % (prefix, self.get_dns_domain()), dnsp.DNS_TYPE_TXT, '')) + def test_update_add_empty_rpc_to_dns(self): + prefix, txt = 'rpcemptytextrec', [] + + name = "%s.%s" % (prefix, self.get_dns_domain()) + + rec = data_to_dns_record(dnsp.DNS_TYPE_TXT, '') + add_rec_buf = dnsserver.DNS_RPC_RECORD_BUF() + add_rec_buf.rec = rec + try: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, add_rec_buf, None) + + self.check_query_txt(prefix, txt) + finally: + self.rpc_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN, + 0, self.server_ip, self.get_dns_domain(), + name, None, add_rec_buf) + if __name__ == "__main__": import unittest unittest.main() diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index e284c2c..8f4976c 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -291,6 +291,7 @@ for f in sorted(os.listdir(os.path.join(samba4srcdir, "../pidl/tests"))): # DNS tests planpythontestsuite("fl2003dc:local", "samba.tests.dns") + for t in smbtorture4_testsuites("dns_internal."): plansmbtorture4testsuite(t, "ad_dc_ntvfs:local", '//$SERVER/whavever') -- 1.9.1 From 0dea999de7eb30db1162e467e93daa15839696ca Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Fri, 22 Jan 2016 11:35:03 +1300 Subject: [PATCH 28/30] CVE-2016-0771: tests: rename test getopt to get_opt This avoids any conflicts in this directory with the original toplevel getopt. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Garming Sam Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- python/samba/tests/{getopt.py => get_opt.py} | 0 selftest/tests.py | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename python/samba/tests/{getopt.py => get_opt.py} (100%) diff --git a/python/samba/tests/getopt.py b/python/samba/tests/get_opt.py similarity index 100% rename from python/samba/tests/getopt.py rename to python/samba/tests/get_opt.py diff --git a/selftest/tests.py b/selftest/tests.py index 872fbaf..2a07f9e 100644 --- a/selftest/tests.py +++ b/selftest/tests.py @@ -48,7 +48,7 @@ planpythontestsuite("none", "api", name="ldb.python", extra_path=['lib/ldb/tests planpythontestsuite("none", "samba.tests.credentials") planpythontestsuite("none", "samba.tests.registry") planpythontestsuite("none", "samba.tests.auth") -planpythontestsuite("none", "samba.tests.getopt") +planpythontestsuite("none", "samba.tests.get_opt") planpythontestsuite("none", "samba.tests.security") planpythontestsuite("none", "samba.tests.dcerpc.misc") planpythontestsuite("none", "samba.tests.dcerpc.integer") -- 1.9.1 From 7a11d9990af8aa87c16fb593cc181468962984fd Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Fri, 29 Jan 2016 17:03:56 +1300 Subject: [PATCH 29/30] CVE-2016-0771: tests/dns: change samba.tests.dns from being a unittest This makes it easier to invoke, particularly against Windows. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Garming Sam Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- python/samba/tests/dns.py | 47 +++++++++++++++++++++++++++++++++++++++-------- source4/selftest/tests.py | 2 +- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py index 3e655c3..729fbc1 100644 --- a/python/samba/tests/dns.py +++ b/python/samba/tests/dns.py @@ -16,6 +16,7 @@ # import os +import sys import struct import random import socket @@ -24,10 +25,39 @@ from samba import credentials, param from samba.tests import TestCase from samba.dcerpc import dns, dnsp, dnsserver from samba.netcmd.dns import TXTRecord, dns_record_match, data_to_dns_record +from samba.tests.subunitrun import SubunitOptions, TestProgram +import samba.getopt as options +import optparse + +parser = optparse.OptionParser("dns.py [options]") +sambaopts = options.SambaOptions(parser) +parser.add_option_group(sambaopts) # This timeout only has relevance when testing against Windows # Format errors tend to return patchy responses, so a timeout is needed. -timeout = None +parser.add_option("--timeout", type="int", dest="timeout", + help="Specify timeout for DNS requests") + +# use command line creds if available +credopts = options.CredentialsOptions(parser) +parser.add_option_group(credopts) +subunitopts = SubunitOptions(parser) +parser.add_option_group(subunitopts) + +opts, args = parser.parse_args() + +lp = sambaopts.get_loadparm() +creds = credopts.get_credentials(lp) + +timeout = opts.timeout + +if len(args) < 2: + parser.print_usage() + sys.exit(1) + +server_name = args[0] +server_ip = args[1] +creds.set_krb_forwardable(credentials.NO_KRB_FORWARDABLE) def make_txt_record(records): rdata_txt = dns.txt_record() @@ -39,10 +69,13 @@ def make_txt_record(records): class DNSTest(TestCase): - def get_loadparm(self): - lp = param.LoadParm() - lp.load(os.getenv("SMB_CONF_PATH")) - return lp + def setUp(self): + global server, server_ip, lp, creds + super(DNSTest, self).setUp() + self.server = server_name + self.server_ip = server_ip + self.lp = lp + self.creds = creds def errstr(self, errcode): "Return a readable error code" @@ -1244,6 +1277,4 @@ class TestRPCRoundtrip(DNSTest): 0, self.server_ip, self.get_dns_domain(), name, None, add_rec_buf) -if __name__ == "__main__": - import unittest - unittest.main() +TestProgram(module=__name__, opts=subunitopts) diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 8f4976c..69330f2 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -290,7 +290,7 @@ for f in sorted(os.listdir(os.path.join(samba4srcdir, "../pidl/tests"))): planperltestsuite("pidl.%s" % f[:-3], os.path.normpath(os.path.join(samba4srcdir, "../pidl/tests", f))) # DNS tests -planpythontestsuite("fl2003dc:local", "samba.tests.dns") +plantestsuite_loadlist("samba.tests.dns", "fl2003dc:local", [python, os.path.join(srcdir(), "python/samba/tests/dns.py"), '$SERVER', '$SERVER_IP', '--machine-pass', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) for t in smbtorture4_testsuites("dns_internal."): plansmbtorture4testsuite(t, "ad_dc_ntvfs:local", '//$SERVER/whavever') -- 1.9.1 From b428ecb28cfef407a91d0048a3769f8421fa252a Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Fri, 29 Jan 2016 17:28:54 +1300 Subject: [PATCH 30/30] CVE-2016-0771: tests/dns: Remove dependencies on env variables Now that it is invoked as a normal script, there should be less of them. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11128 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11686 Signed-off-by: Garming Sam Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- python/samba/tests/dns.py | 68 +++++++++++++++++------------------------------ 1 file changed, 24 insertions(+), 44 deletions(-) diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py index 729fbc1..e0739d0 100644 --- a/python/samba/tests/dns.py +++ b/python/samba/tests/dns.py @@ -132,9 +132,9 @@ class DNSTest(TestCase): def get_dns_domain(self): "Helper to get dns domain" - return os.getenv('REALM', 'example.com').lower() + return self.creds.get_realm().lower() - def dns_transaction_udp(self, packet, host=os.getenv('SERVER_IP'), + def dns_transaction_udp(self, packet, host=server_ip, dump=False, timeout=timeout): "send a DNS query and read the reply" s = None @@ -154,7 +154,7 @@ class DNSTest(TestCase): if s is not None: s.close() - def dns_transaction_tcp(self, packet, host=os.getenv('SERVER_IP'), + def dns_transaction_tcp(self, packet, host=server_ip, dump=False, timeout=timeout): "send a DNS query and read the reply" s = None @@ -221,7 +221,7 @@ class TestSimpleQueries(DNSTest): p = self.make_name_packet(dns.DNS_OPCODE_QUERY) questions = [] - name = "%s.%s" % (os.getenv('SERVER'), self.get_dns_domain()) + name = "%s.%s" % (self.server, self.get_dns_domain()) q = self.make_name_question(name, dns.DNS_QTYPE_A, dns.DNS_QCLASS_IN) print "asking for ", q.name questions.append(q) @@ -232,14 +232,14 @@ class TestSimpleQueries(DNSTest): self.assert_dns_opcode_equals(response, dns.DNS_OPCODE_QUERY) self.assertEquals(response.ancount, 1) self.assertEquals(response.answers[0].rdata, - os.getenv('SERVER_IP')) + self.server_ip) def test_one_a_query_tcp(self): "create a query packet containing one query record via TCP" p = self.make_name_packet(dns.DNS_OPCODE_QUERY) questions = [] - name = "%s.%s" % (os.getenv('SERVER'), self.get_dns_domain()) + name = "%s.%s" % (self.server, self.get_dns_domain()) q = self.make_name_question(name, dns.DNS_QTYPE_A, dns.DNS_QCLASS_IN) print "asking for ", q.name questions.append(q) @@ -250,14 +250,14 @@ class TestSimpleQueries(DNSTest): self.assert_dns_opcode_equals(response, dns.DNS_OPCODE_QUERY) self.assertEquals(response.ancount, 1) self.assertEquals(response.answers[0].rdata, - os.getenv('SERVER_IP')) + self.server_ip) def test_one_mx_query(self): "create a query packet causing an empty RCODE_OK answer" p = self.make_name_packet(dns.DNS_OPCODE_QUERY) questions = [] - name = "%s.%s" % (os.getenv('SERVER'), self.get_dns_domain()) + name = "%s.%s" % (self.server, self.get_dns_domain()) q = self.make_name_question(name, dns.DNS_QTYPE_MX, dns.DNS_QCLASS_IN) print "asking for ", q.name questions.append(q) @@ -271,7 +271,7 @@ class TestSimpleQueries(DNSTest): p = self.make_name_packet(dns.DNS_OPCODE_QUERY) questions = [] - name = "invalid-%s.%s" % (os.getenv('SERVER'), self.get_dns_domain()) + name = "invalid-%s.%s" % (self.server, self.get_dns_domain()) q = self.make_name_question(name, dns.DNS_QTYPE_MX, dns.DNS_QCLASS_IN) print "asking for ", q.name questions.append(q) @@ -287,7 +287,7 @@ class TestSimpleQueries(DNSTest): p = self.make_name_packet(dns.DNS_OPCODE_QUERY) questions = [] - name = "%s.%s" % (os.getenv('SERVER'), self.get_dns_domain()) + name = "%s.%s" % (self.server, self.get_dns_domain()) q = self.make_name_question(name, dns.DNS_QTYPE_A, dns.DNS_QCLASS_IN) questions.append(q) @@ -311,7 +311,7 @@ class TestSimpleQueries(DNSTest): p = self.make_name_packet(dns.DNS_OPCODE_QUERY) questions = [] - name = "%s.%s" % (os.getenv('SERVER'), self.get_dns_domain()) + name = "%s.%s" % (self.server, self.get_dns_domain()) q = self.make_name_question(name, dns.DNS_QTYPE_ALL, dns.DNS_QCLASS_IN) print "asking for ", q.name questions.append(q) @@ -328,7 +328,7 @@ class TestSimpleQueries(DNSTest): self.assert_dns_opcode_equals(response, dns.DNS_OPCODE_QUERY) self.assertEquals(response.ancount, num_answers) self.assertEquals(response.answers[0].rdata, - os.getenv('SERVER_IP')) + self.server_ip) if dc_ipv6 is not None: self.assertEquals(response.answers[1].rdata, dc_ipv6) @@ -337,7 +337,7 @@ class TestSimpleQueries(DNSTest): p = self.make_name_packet(dns.DNS_OPCODE_QUERY) questions = [] - name = "%s.%s" % (os.getenv('SERVER'), self.get_dns_domain()) + name = "%s.%s" % (self.server, self.get_dns_domain()) q = self.make_name_question(name, dns.DNS_QTYPE_ALL, dns.DNS_QCLASS_NONE) questions.append(q) @@ -394,7 +394,7 @@ class TestDNSUpdates(DNSTest): p = self.make_name_packet(dns.DNS_OPCODE_UPDATE) updates = [] - name = "%s.%s" % (os.getenv('SERVER'), self.get_dns_domain()) + name = "%s.%s" % (self.server, self.get_dns_domain()) u = self.make_name_question(name, dns.DNS_QTYPE_A, dns.DNS_QCLASS_IN) updates.append(u) @@ -439,7 +439,7 @@ class TestDNSUpdates(DNSTest): prereqs = [] r = dns.res_rec() - r.name = "%s.%s" % (os.getenv('SERVER'), self.get_dns_domain()) + r.name = "%s.%s" % (self.server, self.get_dns_domain()) r.rr_type = dns.DNS_QTYPE_TXT r.rr_class = dns.DNS_QCLASS_NONE r.ttl = 1 @@ -472,7 +472,7 @@ class TestDNSUpdates(DNSTest): prereqs = [] r = dns.res_rec() - r.name = "%s.%s" % (os.getenv('SERVER'), self.get_dns_domain()) + r.name = "%s.%s" % (self.server, self.get_dns_domain()) r.rr_type = dns.DNS_QTYPE_TXT r.rr_class = dns.DNS_QCLASS_ANY r.ttl = 0 @@ -777,7 +777,7 @@ class TestComplexQueries(DNSTest): r.rr_class = dns.DNS_QCLASS_IN r.ttl = 900 r.length = 0xffff - r.rdata = "%s.%s" % (os.getenv('SERVER'), self.get_dns_domain()) + r.rdata = "%s.%s" % (self.server, self.get_dns_domain()) updates.append(r) p.nscount = len(updates) p.nsrecs = updates @@ -803,7 +803,7 @@ class TestComplexQueries(DNSTest): r.rr_class = dns.DNS_QCLASS_NONE r.ttl = 0 r.length = 0xffff - r.rdata = "%s.%s" % (os.getenv('SERVER'), self.get_dns_domain()) + r.rdata = "%s.%s" % (self.server, self.get_dns_domain()) updates.append(r) p.nscount = len(updates) p.nsrecs = updates @@ -828,10 +828,10 @@ class TestComplexQueries(DNSTest): self.assertEquals(response.ancount, 2) self.assertEquals(response.answers[0].rr_type, dns.DNS_QTYPE_CNAME) self.assertEquals(response.answers[0].rdata, "%s.%s" % - (os.getenv('SERVER'), self.get_dns_domain())) + (self.server, self.get_dns_domain())) self.assertEquals(response.answers[1].rr_type, dns.DNS_QTYPE_A) self.assertEquals(response.answers[1].rdata, - os.getenv('SERVER_IP')) + self.server_ip) class TestInvalidQueries(DNSTest): @@ -841,7 +841,7 @@ class TestInvalidQueries(DNSTest): s = None try: s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0) - s.connect((os.getenv('SERVER_IP'), 53)) + s.connect((self.server_ip, 53)) s.send("", 0) finally: if s is not None: @@ -850,7 +850,7 @@ class TestInvalidQueries(DNSTest): p = self.make_name_packet(dns.DNS_OPCODE_QUERY) questions = [] - name = "%s.%s" % (os.getenv('SERVER'), self.get_dns_domain()) + name = "%s.%s" % (self.server, self.get_dns_domain()) q = self.make_name_question(name, dns.DNS_QTYPE_A, dns.DNS_QCLASS_IN) print "asking for ", q.name questions.append(q) @@ -861,7 +861,7 @@ class TestInvalidQueries(DNSTest): self.assert_dns_opcode_equals(response, dns.DNS_OPCODE_QUERY) self.assertEquals(response.ancount, 1) self.assertEquals(response.answers[0].rdata, - os.getenv('SERVER_IP')) + self.server_ip) def test_one_a_reply(self): "send a reply instead of a query" @@ -882,7 +882,7 @@ class TestInvalidQueries(DNSTest): send_packet = ndr.ndr_pack(p) s = socket.socket(socket.AF_INET, socket.SOCK_STREAM, 0) s.settimeout(timeout) - host=os.getenv('SERVER_IP') + host=self.server_ip s.connect((host, 53)) tcp_packet = struct.pack('!H', len(send_packet)) tcp_packet += send_packet @@ -900,18 +900,8 @@ class TestInvalidQueries(DNSTest): s.close() class TestZones(DNSTest): - def get_credentials(self, lp): - creds = credentials.Credentials() - creds.guess(lp) - creds.set_machine_account(lp) - creds.set_krb_forwardable(credentials.NO_KRB_FORWARDABLE) - return creds - def setUp(self): super(TestZones, self).setUp() - self.lp = self.get_loadparm() - self.creds = self.get_credentials(self.lp) - self.server = os.getenv("SERVER_IP") self.zone = "test.lan" self.rpc_conn = dnsserver.dnsserver("ncacn_ip_tcp:%s[sign]" % (self.server_ip), self.lp, self.creds) @@ -979,18 +969,8 @@ class TestZones(DNSTest): self.assertEquals(response.ancount, 0) class TestRPCRoundtrip(DNSTest): - def get_credentials(self, lp): - creds = credentials.Credentials() - creds.guess(lp) - creds.set_machine_account(lp) - creds.set_krb_forwardable(credentials.NO_KRB_FORWARDABLE) - return creds - def setUp(self): super(TestRPCRoundtrip, self).setUp() - self.lp = self.get_loadparm() - self.creds = self.get_credentials(self.lp) - self.server = os.getenv("SERVER_IP") self.rpc_conn = dnsserver.dnsserver("ncacn_ip_tcp:%s[sign]" % (self.server_ip), self.lp, self.creds) -- 1.9.1