The branch, v4-6-test has been updated
via 8635c4f VERSION: Bump version up to 4.6.17.
via fd60772 Merge tag 'samba-4.6.16' into v4-6-test
via 18df99b VERSION: Disable GIT_SNAPSHOT for the 4.6.16 release.
via cd2839a WHATSNEW: Add release notes for Samba 4.6.16.
via 9f166c0 CVE-2018-10919 tests: Add extra test for dirsync deleted
object corner-case
via 246e79f CVE-2018-10919 acl_read: Fix unauthorized attribute access
via searches
via 9605ecc CVE-2018-10919 acl_read: Flip the logic in the dirsync check
via 533106a CVE-2018-10919 acl_read: Small refactor to
aclread_callback()
via fa7bcea CVE-2018-10919 acl_read: Split access_mask logic out into
helper function
via f6cbad5 CVE-2018-10919 security: Fix checking of object-specific
CONTROL_ACCESS rights
via 873ccd0 CVE-2018-10919 tests: test ldap searches for non-existent
attributes.
via 924f87c CVE-2018-10919 tests: Add test case for object visibility
with limited rights
via 3388706 CVE-2018-10919 tests: Add tests for guessing confidential
attributes
via 010d1f1 CVE-2018-10919 security: Add more comments to the
object-specific access checks
via 2878c22 CVE-2018-10919 security: Move object-specific access checks
into separate function
via 2711b66 CVE-2018-10858: libsmb: Harden smbc_readdir_internal()
against returns from malicious servers.
via 6936d3e CVE-2018-10858: libsmb: Ensure smbc_urlencode() can't
overwrite passed in buffer.
via 30428f3 VERSION: Bump version up to 4.6.16...
from 7705a4d VERSION: Bump version up to 4.6.16...
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-6-test
- Log -----------------------------------------------------------------
commit 8635c4fb1b8dcf842480173b27b8352df484fb88
Author: Karolin Seeger <[email protected]>
Date: Tue Aug 14 12:21:06 2018 +0200
VERSION: Bump version up to 4.6.17.
Signed-off-by: Karolin Seeger <[email protected]>
commit fd60772cd61d295e788c68d8d87b6685bde546dd
Merge: 7705a4d 18df99b
Author: Karolin Seeger <[email protected]>
Date: Tue Aug 14 12:20:50 2018 +0200
Merge tag 'samba-4.6.16' into v4-6-test
samba: tag release samba-4.6.16
-----------------------------------------------------------------------
Summary of changes:
VERSION | 2 +-
WHATSNEW.txt | 66 +-
libcli/security/access_check.c | 110 ++-
source3/libsmb/libsmb_dir.c | 57 +-
source3/libsmb/libsmb_path.c | 9 +-
source4/dsdb/samdb/ldb_modules/acl_read.c | 333 +++++++-
source4/dsdb/tests/python/acl.py | 68 ++
source4/dsdb/tests/python/confidential_attr.py | 1025 ++++++++++++++++++++++++
source4/dsdb/tests/python/ldap.py | 9 +
source4/selftest/tests.py | 3 +
10 files changed, 1608 insertions(+), 74 deletions(-)
create mode 100755 source4/dsdb/tests/python/confidential_attr.py
Changeset truncated at 500 lines:
diff --git a/VERSION b/VERSION
index 466bd23..88d6548 100644
--- a/VERSION
+++ b/VERSION
@@ -25,7 +25,7 @@
########################################################
SAMBA_VERSION_MAJOR=4
SAMBA_VERSION_MINOR=6
-SAMBA_VERSION_RELEASE=16
+SAMBA_VERSION_RELEASE=17
########################################################
# If a official release has a serious bug #
diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index fa673c3..d0c0533 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -1,4 +1,66 @@
==============================
+ Release Notes for Samba 4.6.16
+ August 14, 2018
+ ==============================
+
+
+This is a security release in order to address the following defects:
+
+o CVE-2018-10858 (Insufficient input validation on client directory
+ listing in libsmbclient.)
+o CVE-2018-10919 (Confidential attribute disclosure from the AD LDAP
+ server.)
+
+
+=======
+Details
+=======
+
+o CVE-2018-10858:
+ A malicious server could return a directory entry that could corrupt
+ libsmbclient memory.
+
+o CVE-2018-10919:
+ Missing access control checks allow discovery of confidential attribute
+ values via authenticated LDAP search expressions.
+
+
+Changes since 4.6.15:
+--------------------
+
+o Jeremy Allison <[email protected]>
+ * BUG 13453: CVE-2018-10858: libsmb: Harden smbc_readdir_internal() against
+ returns from malicious servers.
+
+o Tim Beale <[email protected]>
+ * BUG 13434: CVE-2018-10919: acl_read: Fix unauthorized attribute access via
+ searches.
+
+
+#######################################
+Reporting bugs & Development Discussion
+#######################################
+
+Please discuss this release on the samba-technical mailing list or by
+joining the #samba-technical IRC channel on irc.freenode.net.
+
+If you do report problems then please try to send high quality
+feedback. If you don't provide vital information to help us track down
+the problem then you will probably be ignored. All bug reports should
+be filed under the "Samba 4.1 and newer" product in the project's Bugzilla
+database (https://bugzilla.samba.org/).
+
+
+======================================================================
+== Our Code, Our Bugs, Our Responsibility.
+== The Samba Team
+======================================================================
+
+
+Release notes for older releases follow:
+----------------------------------------
+
+ ==============================
Release Notes for Samba 4.6.15
April 13, 2018
==============================
@@ -70,8 +132,8 @@ database (https://bugzilla.samba.org/).
======================================================================
-Release notes for older releases follow:
-----------------------------------------
+----------------------------------------------------------------------
+
==============================
Release Notes for Samba 4.6.14
diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c
index b4c850b..03a7dca 100644
--- a/libcli/security/access_check.c
+++ b/libcli/security/access_check.c
@@ -375,6 +375,81 @@ static const struct GUID *get_ace_object_type(struct
security_ace *ace)
}
/**
+ * Evaluates access rights specified in a object-specific ACE for an AD object.
+ * This logic corresponds to MS-ADTS 5.1.3.3.3 Checking Object-Specific Access.
+ * @param[in] ace - the ACE being processed
+ * @param[in/out] tree - remaining_access gets updated for the tree
+ * @param[out] grant_access - set to true if the ACE grants sufficient access
+ * rights to the object/attribute
+ * @returns NT_STATUS_OK, unless access was denied
+ */
+static NTSTATUS check_object_specific_access(struct security_ace *ace,
+ struct object_tree *tree,
+ bool *grant_access)
+{
+ struct object_tree *node = NULL;
+ const struct GUID *type = NULL;
+
+ *grant_access = false;
+
+ /* if no tree was supplied, we can't do object-specific access checks */
+ if (!tree) {
+ return NT_STATUS_OK;
+ }
+
+ /* Get the ObjectType GUID this ACE applies to */
+ type = get_ace_object_type(ace);
+
+ /*
+ * If the ACE doesn't have a type, then apply it to the whole tree, i.e.
+ * treat 'OA' ACEs as 'A' and 'OD' as 'D'
+ */
+ if (!type) {
+ node = tree;
+ } else {
+
+ /* skip it if the ACE's ObjectType GUID is not in the tree */
+ node = get_object_tree_by_GUID(tree, type);
+ if (!node) {
+ return NT_STATUS_OK;
+ }
+ }
+
+ if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT) {
+
+ /* apply the access rights to this node, and any children */
+ object_tree_modify_access(node, ace->access_mask);
+
+ /*
+ * Currently all nodes in the tree request the same access mask,
+ * so we can use any node to check if processing this ACE now
+ * means the requested access has been granted
+ */
+ if (node->remaining_access == 0) {
+ *grant_access = true;
+ return NT_STATUS_OK;
+ }
+
+ /*
+ * As per 5.1.3.3.4 Checking Control Access Right-Based Access,
+ * if the CONTROL_ACCESS right is present, then we can grant
+ * access and stop any further access checks
+ */
+ if (ace->access_mask & SEC_ADS_CONTROL_ACCESS) {
+ *grant_access = true;
+ return NT_STATUS_OK;
+ }
+ } else {
+
+ /* this ACE denies access to the requested object/attribute */
+ if (node->remaining_access & ace->access_mask){
+ return NT_STATUS_ACCESS_DENIED;
+ }
+ }
+ return NT_STATUS_OK;
+}
+
+/**
* @brief Perform directoryservice (DS) related access checks for a given user
*
* Perform DS access checks for the user represented by its security_token, on
@@ -405,8 +480,6 @@ NTSTATUS sec_access_check_ds(const struct
security_descriptor *sd,
{
uint32_t i;
uint32_t bits_remaining;
- struct object_tree *node;
- const struct GUID *type;
struct dom_sid self_sid;
dom_sid_parse(SID_NT_SELF, &self_sid);
@@ -456,6 +529,8 @@ NTSTATUS sec_access_check_ds(const struct
security_descriptor *sd,
for (i=0; bits_remaining && i < sd->dacl->num_aces; i++) {
struct dom_sid *trustee;
struct security_ace *ace = &sd->dacl->aces[i];
+ NTSTATUS status;
+ bool grant_access = false;
if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY) {
continue;
@@ -486,34 +561,15 @@ NTSTATUS sec_access_check_ds(const struct
security_descriptor *sd,
break;
case SEC_ACE_TYPE_ACCESS_DENIED_OBJECT:
case SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT:
- /*
- * check only in case we have provided a tree,
- * the ACE has an object type and that type
- * is in the tree
- */
- type = get_ace_object_type(ace);
-
- if (!tree) {
- continue;
- }
+ status = check_object_specific_access(ace, tree,
+ &grant_access);
- if (!type) {
- node = tree;
- } else {
- if (!(node = get_object_tree_by_GUID(tree,
type))) {
- continue;
- }
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
}
- if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT) {
- object_tree_modify_access(node,
ace->access_mask);
- if (node->remaining_access == 0) {
- return NT_STATUS_OK;
- }
- } else {
- if (node->remaining_access & ace->access_mask){
- return NT_STATUS_ACCESS_DENIED;
- }
+ if (grant_access) {
+ return NT_STATUS_OK;
}
break;
default: /* Other ACE types not handled/supported */
diff --git a/source3/libsmb/libsmb_dir.c b/source3/libsmb/libsmb_dir.c
index 6314591..96eb163 100644
--- a/source3/libsmb/libsmb_dir.c
+++ b/source3/libsmb/libsmb_dir.c
@@ -930,27 +930,47 @@ SMBC_closedir_ctx(SMBCCTX *context,
}
-static void
+static int
smbc_readdir_internal(SMBCCTX * context,
struct smbc_dirent *dest,
struct smbc_dirent *src,
int max_namebuf_len)
{
if (smbc_getOptionUrlEncodeReaddirEntries(context)) {
+ int remaining_len;
/* url-encode the name. get back remaining buffer space */
- max_namebuf_len =
+ remaining_len =
smbc_urlencode(dest->name, src->name, max_namebuf_len);
+ /* -1 means no null termination. */
+ if (remaining_len < 0) {
+ return -1;
+ }
+
/* We now know the name length */
dest->namelen = strlen(dest->name);
+ if (dest->namelen + 1 < 1) {
+ /* Integer wrap. */
+ return -1;
+ }
+
+ if (dest->namelen + 1 >= max_namebuf_len) {
+ /* Out of space for comment. */
+ return -1;
+ }
+
/* Save the pointer to the beginning of the comment */
dest->comment = dest->name + dest->namelen + 1;
+ if (remaining_len < 1) {
+ /* No room for comment null termination. */
+ return -1;
+ }
+
/* Copy the comment */
- strncpy(dest->comment, src->comment, max_namebuf_len - 1);
- dest->comment[max_namebuf_len - 1] = '\0';
+ strlcpy(dest->comment, src->comment, remaining_len);
/* Save other fields */
dest->smbc_type = src->smbc_type;
@@ -960,10 +980,21 @@ smbc_readdir_internal(SMBCCTX * context,
} else {
/* No encoding. Just copy the entry as is. */
+ if (src->dirlen > max_namebuf_len) {
+ return -1;
+ }
memcpy(dest, src, src->dirlen);
+ if (src->namelen + 1 < 1) {
+ /* Integer wrap */
+ return -1;
+ }
+ if (src->namelen + 1 >= max_namebuf_len) {
+ /* Comment off the end. */
+ return -1;
+ }
dest->comment = (char *)(&dest->name + src->namelen + 1);
}
-
+ return 0;
}
/*
@@ -975,6 +1006,7 @@ SMBC_readdir_ctx(SMBCCTX *context,
SMBCFILE *dir)
{
int maxlen;
+ int ret;
struct smbc_dirent *dirp, *dirent;
TALLOC_CTX *frame = talloc_stackframe();
@@ -1024,7 +1056,12 @@ SMBC_readdir_ctx(SMBCCTX *context,
dirp = &context->internal->dirent;
maxlen = sizeof(context->internal->_dirent_name);
- smbc_readdir_internal(context, dirp, dirent, maxlen);
+ ret = smbc_readdir_internal(context, dirp, dirent, maxlen);
+ if (ret == -1) {
+ errno = EINVAL;
+ TALLOC_FREE(frame);
+ return NULL;
+ }
dir->dir_next = dir->dir_next->next;
@@ -1082,6 +1119,7 @@ SMBC_getdents_ctx(SMBCCTX *context,
*/
while ((dirlist = dir->dir_next)) {
+ int ret;
struct smbc_dirent *dirent;
struct smbc_dirent *currentEntry = (struct smbc_dirent *)ndir;
@@ -1096,8 +1134,13 @@ SMBC_getdents_ctx(SMBCCTX *context,
/* Do urlencoding of next entry, if so selected */
dirent = &context->internal->dirent;
maxlen = sizeof(context->internal->_dirent_name);
- smbc_readdir_internal(context, dirent,
+ ret = smbc_readdir_internal(context, dirent,
dirlist->dirent, maxlen);
+ if (ret == -1) {
+ errno = EINVAL;
+ TALLOC_FREE(frame);
+ return -1;
+ }
reqd = dirent->dirlen;
diff --git a/source3/libsmb/libsmb_path.c b/source3/libsmb/libsmb_path.c
index 01b0a61..5b53b38 100644
--- a/source3/libsmb/libsmb_path.c
+++ b/source3/libsmb/libsmb_path.c
@@ -173,8 +173,13 @@ smbc_urlencode(char *dest,
}
}
- *dest++ = '\0';
- max_dest_len--;
+ if (max_dest_len <= 0) {
+ /* Ensure we return -1 if no null termination. */
+ return -1;
+ }
+
+ *dest++ = '\0';
+ max_dest_len--;
return max_dest_len;
}
diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c
b/source4/dsdb/samdb/ldb_modules/acl_read.c
index f15633f..eb8676d 100644
--- a/source4/dsdb/samdb/ldb_modules/acl_read.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c
@@ -38,6 +38,8 @@
#include "param/param.h"
#include "dsdb/samdb/ldb_modules/util.h"
+/* The attributeSecurityGuid for the Public Information Property-Set */
+#define PUBLIC_INFO_PROPERTY_SET "e48d0154-bcf8-11d1-8702-00c04fb96050"
struct aclread_context {
struct ldb_module *module;
@@ -64,6 +66,254 @@ static bool aclread_is_inaccessible(struct
ldb_message_element *el) {
return el->flags & LDB_FLAG_INTERNAL_INACCESSIBLE_ATTRIBUTE;
}
+/*
+ * Returns the access mask required to read a given attribute
+ */
+static uint32_t get_attr_access_mask(const struct dsdb_attribute *attr,
+ uint32_t sd_flags)
+{
+
+ uint32_t access_mask = 0;
+ bool is_sd;
+
+ /* nTSecurityDescriptor is a special case */
+ is_sd = (ldb_attr_cmp("nTSecurityDescriptor",
+ attr->lDAPDisplayName) == 0);
+
+ if (is_sd) {
+ if (sd_flags & (SECINFO_OWNER|SECINFO_GROUP)) {
+ access_mask |= SEC_STD_READ_CONTROL;
+ }
+ if (sd_flags & SECINFO_DACL) {
+ access_mask |= SEC_STD_READ_CONTROL;
+ }
+ if (sd_flags & SECINFO_SACL) {
+ access_mask |= SEC_FLAG_SYSTEM_SECURITY;
+ }
+ } else {
+ access_mask = SEC_ADS_READ_PROP;
+ }
+
+ if (attr->searchFlags & SEARCH_FLAG_CONFIDENTIAL) {
+ access_mask |= SEC_ADS_CONTROL_ACCESS;
+ }
+
+ return access_mask;
+}
+
+/* helper struct for traversing the attributes in the search-tree */
+struct parse_tree_aclread_ctx {
+ struct aclread_context *ac;
+ TALLOC_CTX *mem_ctx;
+ struct dom_sid *sid;
+ struct ldb_dn *dn;
+ struct security_descriptor *sd;
+ const struct dsdb_class *objectclass;
+ bool suppress_result;
+};
+
+/*
+ * Checks that the user has sufficient access rights to view an attribute
+ */
+static int check_attr_access_rights(TALLOC_CTX *mem_ctx, const char *attr_name,
+ struct aclread_context *ac,
+ struct security_descriptor *sd,
+ const struct dsdb_class *objectclass,
+ struct dom_sid *sid, struct ldb_dn *dn,
+ bool *is_public_info)
+{
+ int ret;
+ const struct dsdb_attribute *attr = NULL;
+ uint32_t access_mask;
+ struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
+
+ *is_public_info = false;
+
+ attr = dsdb_attribute_by_lDAPDisplayName(ac->schema, attr_name);
+ if (!attr) {
+ ldb_debug_set(ldb,
+ LDB_DEBUG_TRACE,
+ "acl_read: %s cannot find attr[%s] in schema,"
+ "ignoring\n",
+ ldb_dn_get_linearized(dn), attr_name);
+ return LDB_SUCCESS;
+ }
+
+ /*
+ * If we have no Read Property (RP) rights for a child object, it should
+ * still appear as a visible object in 'objectClass=*' searches,
+ * as long as we have List Contents (LC) rights for it.
+ * This is needed for the acl.py tests (e.g. test_search1()).
+ * I couldn't find the Windows behaviour documented in the specs, so
+ * this is a guess, but it seems to only apply to attributes in the
+ * Public Information Property Set that have the systemOnly flag set to
+ * TRUE. (This makes sense in a way, as it's not disclosive to find out
+ * that a child object has a 'objectClass' or 'name' attribute, as every
+ * object has these attributes).
+ */
+ if (attr->systemOnly) {
+ struct GUID public_info_guid;
+ NTSTATUS status;
+
+ status = GUID_from_string(PUBLIC_INFO_PROPERTY_SET,
+ &public_info_guid);
+ if (!NT_STATUS_IS_OK(status)) {
+ ldb_set_errstring(ldb, "Public Info GUID parse error");
+ return LDB_ERR_OPERATIONS_ERROR;
+ }
+
+ if (GUID_compare(&attr->attributeSecurityGUID,
+ &public_info_guid) == 0) {
+ *is_public_info = true;
--
Samba Shared Repository
