[PATCHES] util_sd: Also accept hex input for ALLOW/DENIED (was Re: bug in smbcacls)
Christof Schmitt
cs at samba.org
Thu Feb 11 23:13:31 UTC 2016
On Thu, Feb 04, 2016 at 10:47:04AM -0700, Christof Schmitt wrote:
> On Thu, Feb 04, 2016 at 09:31:37AM -0800, Herb Lewis wrote:
> > In source3/lib/util_sd.c function parse_ace there is the following
> > line that I think is incorrect
> >
> > if (sscanf(p, "%u/%u/%u", &atype, &aflags, &amask) == 3 &&
> >
> > in the old samba 3.6 code it used to read
> >
> > if (sscanf(p, "%i/%i/%i", &atype, &aflags, &amask) == 3 &&
> >
> > and according to the man page for sscanf the %u is only for decimal
> > integers while %i also checks for base 16 and base 8 as well. Was there
> > a reason this was changed so that hex numbers are no longer allowed?
>
> util_sd.c is the result of using common code between smbcacls and
> sharesec. There is no reason why we would disallow base 16 and base 8; i
> probably just missed that part. Feel free to send a patch, or i can
> address this when i have a minute.
What about this approach? I also added a test and fixed some issues in
the test script.
Christof
-------------- next part --------------
From f6983f66a4618fe7a9103c03b934c0ea2df79bef Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Mon, 8 Feb 2016 13:56:23 -0700
Subject: [PATCH 1/5] util_sd: Also accept hex input for ALLOW/DENIED
Implement this by explicitly checking for decimal or hexadecimal input.
This avoids using sscanf with %i and a signed integer type, and it also
matches the code paths for flags and mask that also have an explicit
check.
Signed-off-by: Christof Schmitt <cs at samba.org>
---
source3/lib/util_sd.c | 35 +++++++++++++++++++++++++++--------
1 file changed, 27 insertions(+), 8 deletions(-)
diff --git a/source3/lib/util_sd.c b/source3/lib/util_sd.c
index 8065a0f..80f4b58 100644
--- a/source3/lib/util_sd.c
+++ b/source3/lib/util_sd.c
@@ -418,14 +418,6 @@ bool parse_ace(struct cli_state *cli, struct security_ace *ace,
}
*p = '\0';
p++;
- /* Try to parse numeric form */
-
- if (sscanf(p, "%u/%u/%u", &atype, &aflags, &amask) == 3 &&
- StringToSid(cli, &sid, str)) {
- goto done;
- }
-
- /* Try to parse text form */
if (!StringToSid(cli, &sid, str)) {
printf("ACE '%s': failed to convert '%s' to SID\n",
@@ -448,6 +440,33 @@ bool parse_ace(struct cli_state *cli, struct security_ace *ace,
atype = SEC_ACE_TYPE_ACCESS_ALLOWED;
} else if (strncmp(tok, "DENIED", strlen("DENIED")) == 0) {
atype = SEC_ACE_TYPE_ACCESS_DENIED;
+
+ } else if (strnequal(tok, "0x", 2)) {
+ int result;
+
+ result = sscanf(tok, "%x", &atype);
+ if (result == 0 ||
+ (atype != SEC_ACE_TYPE_ACCESS_ALLOWED &&
+ atype != SEC_ACE_TYPE_ACCESS_DENIED)) {
+ printf("ACE '%s': bad hex value for type at '%s'\n",
+ orig_str, tok);
+ SAFE_FREE(str);
+ TALLOC_FREE(frame);
+ return false;
+ }
+ } else if(tok[0] >= '0' && tok[0] <= '9') {
+ int result;
+
+ result = sscanf(tok, "%u", &atype);
+ if (result == 0 ||
+ (atype != SEC_ACE_TYPE_ACCESS_ALLOWED &&
+ atype != SEC_ACE_TYPE_ACCESS_DENIED)) {
+ printf("ACE '%s': bad integer value for type at '%s'\n",
+ orig_str, tok);
+ SAFE_FREE(str);
+ TALLOC_FREE(frame);
+ return false;
+ }
} else {
printf("ACE '%s': missing 'ALLOWED' or 'DENIED' entry at '%s'\n",
orig_str, tok);
--
1.8.3.1
From 896dde1078dfce9a6d2f67442376e020ffb82742 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 4 Feb 2016 16:35:08 -0700
Subject: [PATCH 2/5] test_sharesec: Add new test for ACL entry from numerical
input
Signed-off-by: Christof Schmitt <cs at samba.org>
---
source3/script/tests/test_sharesec.sh | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/source3/script/tests/test_sharesec.sh b/source3/script/tests/test_sharesec.sh
index ef207ff..3781429 100755
--- a/source3/script/tests/test_sharesec.sh
+++ b/source3/script/tests/test_sharesec.sh
@@ -97,6 +97,20 @@ testit "Verify ACL count after removal" test $COUNT -eq 3 || \
ACL="$($CMD --view | grep S-1-5-32-546')"
testit "Verify removal" test -e "$ACL" || failed=$(expr $failed + 1)
+testit "Set ACL as hex value" $CMD --add S-1-5-32-547:0x1/0x0/0x001F01FF || \
+ failed=$(expr $failed + 1)
+ACL="$($CMD --view | grep S-1-5-32-547 | sed -e 's/^ACL://')"
+testit "Verify numerically set entry" \
+ test "$ACL" = S-1-5-32-547:DENIED/0x0/FULL || \
+ failed=$(expr $failed + 1)
+
+testit "Set ACL as dec value" $CMD --add S-1-5-32-548:1/0/0x001F01FF || \
+ failed=$(expr $failed + 1)
+ACL="$($CMD --view | grep S-1-5-32-548 | sed -e 's/^ACL://')"
+testit "Verify numerically set entry" \
+ test "$ACL" = S-1-5-32-548:DENIED/0x0/FULL || \
+ failed=$(expr $failed + 1)
+
testit "Set back to default ACL " $CMD --replace S-1-1-0:ALLOWED/0x0/FULL || \
failed=$(expr $failed + 1)
testit "Query standard ACL" $CMD --view || \
--
1.8.3.1
From 42b8746af2cea0ecbd02f82325cdc56d15ed0857 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 4 Feb 2016 16:35:25 -0700
Subject: [PATCH 3/5] test_sharesec: Fix usage message
Signed-off-by: Christof Schmitt <cs at samba.org>
---
source3/script/tests/test_sharesec.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/script/tests/test_sharesec.sh b/source3/script/tests/test_sharesec.sh
index 3781429..59dbe8f 100755
--- a/source3/script/tests/test_sharesec.sh
+++ b/source3/script/tests/test_sharesec.sh
@@ -10,7 +10,7 @@
# Copyright (C) 2015 Christof Schmitt
if [ $# -lt 3 ]; then
-Usage: test_sharesec.sh SERVERCONFFILE SHARESEC SHARE
+ echo Usage: test_sharesec.sh SERVERCONFFILE SHARESEC SHARE
exit 1
fi
--
1.8.3.1
From 711fc9e4701e2b4f082cce8c356c639720dbb500 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 4 Feb 2016 16:39:59 -0700
Subject: [PATCH 4/5] test_sharesec: Fix check for deleted ACL
Remove semicolon; without this change the test could not detect a
failure of removing the ACL.
Signed-off-by: Christof Schmitt <cs at samba.org>
---
source3/script/tests/test_sharesec.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/script/tests/test_sharesec.sh b/source3/script/tests/test_sharesec.sh
index 59dbe8f..8165a58 100755
--- a/source3/script/tests/test_sharesec.sh
+++ b/source3/script/tests/test_sharesec.sh
@@ -94,7 +94,7 @@ testit "Query ACL with three entries after removal" $CMD --view || \
COUNT=$($CMD --view | grep ACL: | sed -e 's/^ACL://' | wc -l)
testit "Verify ACL count after removal" test $COUNT -eq 3 || \
failed=$(expr $failed + 1)
-ACL="$($CMD --view | grep S-1-5-32-546')"
+ACL="$($CMD --view | grep S-1-5-32-546)"
testit "Verify removal" test -e "$ACL" || failed=$(expr $failed + 1)
testit "Set ACL as hex value" $CMD --add S-1-5-32-547:0x1/0x0/0x001F01FF || \
--
1.8.3.1
From 3f6665048ad66ed1529750144e2ef8c82cb90530 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Mon, 8 Feb 2016 14:20:56 -0700
Subject: [PATCH 5/5] testprogs/blackbox/subunit: Fix testok
The fail count is always in the second parameter. Omit the shift
operations, so that the value can be read correctly from $2.
Signed-off-by: Christof Schmitt <cs at samba.org>
---
testprogs/blackbox/subunit.sh | 2 --
1 file changed, 2 deletions(-)
diff --git a/testprogs/blackbox/subunit.sh b/testprogs/blackbox/subunit.sh
index 3ed0882..db7fb05 100755
--- a/testprogs/blackbox/subunit.sh
+++ b/testprogs/blackbox/subunit.sh
@@ -96,9 +96,7 @@ testit_expect_failure () {
testok () {
name=`basename $1`
- shift
failed=$2
- shift
exit $failed
}
--
1.8.3.1
More information about the samba-technical
mailing list