[SCM] Samba Shared Repository - branch master updated
Andrew Bartlett
abartlet at samba.org
Mon Nov 27 23:39:01 UTC 2023
The branch, master has been updated
via a757a51a26f libcli/security: note suboptimality of conditional ACE Contains operators
via 2eb00c0bba5 libcli/security: comparability check: claim members are of one type
via 55999b7b7b2 libcli/security: shift comparability check to shortcut exits
via 6c6f25904ee libcli/security: add shortcuts for conditional ACE compare
via 8bad19c42e1 libcli/security: improve conditional ACE composite comparison
via 6a07d2fe44e libcli/security: separate out claim_v1_to_ace_composite_unchecked()
via e338625ebf1 libcli/security: avoid leak on SDDL encode failure
via 4f56c702834 libcli/security: claim_v1_to_ace_token(): avoid unnecessary re-sort
via e223ce4a930 libcli/security: add_claim_to_token() re-sorts/checks claims
via 843fd4d15f7 libcli/security: resource attribute claims use claim_v1_check_and_sort()
via 8074257c3ae libcli/security: wire claim conversion uses claim_v1_check_and_sort()
via 4b032d25584 libcli/security: claim_v1_check_and_sort(): add all types
via a19f914fb9f libcli/security: begin claim_v1_check_and_sort with Boolean checks
via 4ebb488e512 libcli/security: don't allow two NULL string claims
via 1c88dfc6ac5 libcli/security: wire claims conversion: remove strings uniqueness check
via 08096fd5b40 libcli/security: int wire claims drop uniqueness check
via 10fd3e5836c libcli/security: simplify wire claim conversion mem, 3/3: rm tmp_ctx
via d7da41a9bef libcli/security: simplify wire claim conversion mem, 2/3: one tree
via 7656d133345 libcli/security: simplify wire claim conversion mem, 1/3: avoid NULL parent
via a836ad14422 pytest: conditional_ace_claims tests large composite comparisons
via cea44421ebc libcli/security/sddl: improve some SDDL error messages
via bc9da956822 pytest: conditional_ace_claims: write_c_test_on_failure() copes with claims
via 4cc91d28283 pytest: token_factory note that a flag is not set
via 51d9444baa0 pytest: token_factory copes with empty claims
via f9f87247188 pytest: token_factory claims can have case_sensitive flag
via adf695aa343 pytest: token_factory separate out list_to_claim() helper
via fc480144292 libcli/security: sddl_conditional_ace: check a talloc_new()
via 78506e1752a libcli/security: conditional ACE sddl writers take const tokens
via 33d2deec514 lib/security:CA: tokens_are_comparable() accepts NULL operator
via 6e15a20228c libcli/security: CA: tokens_are_comparable() considers the obvious
via fc890742ab4 libcli/security: add test_claims_conversion
via da077b84862 libcli/security: test_run_conditional_ace tests more comparisons
via 2f40583ab2f libcli/security: test_run_conditional_ace can set debug levels
via e81e0706388 librpc/idl:security: add claims flag indicating orderly and unique members
via a8f83fe8a2f librpc/idl:security: add a couple of claims flags
via f4ea27b84a5 librpc/idl:condtional_ace: shift CONDITIONAL_ACE_FLAG_TOKEN_FROM_ATTR to last bit
via 6aa6ef4b7c1 librpc/idl:conditional_ace: make a flags field 32 bit
via ca572691622 libcli/security: remove redundant claim SID size check
via fa96bbbe816 libcli/security: avoid leak when converting SID claims
from 8f42b8431ef s3: smbd: Allow fchmod from the NFS-style mode ACL in set_nt_acl() for a SMB2 POSIX handle.
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit a757a51a26f664591ab776db99bf48acfa698591
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Sat Nov 25 12:55:09 2023 +1300
libcli/security: note suboptimality of conditional ACE Contains operators
The Contains and Any_of operators could use a sorted comparison like
compare_composites_via_sort(), rather than O(n²) nested loops. But
that would involve amount of quite fiddly work that I am not starting
on now.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
Autobuild-Date(master): Mon Nov 27 23:38:13 UTC 2023 on atb-devel-224
commit 2eb00c0bba5ed1abaa15c1511c6012da56a78604
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu Nov 23 13:03:15 2023 +1300
libcli/security: comparability check: claim members are of one type
We know from the way claims are defined, and from the code that checks
sortedness and sets the flag.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 55999b7b7b2b423eea3c26425c09130059bb4fd9
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu Nov 23 13:01:49 2023 +1300
libcli/security: shift comparability check to shortcut exits
The ordinary comparison path, using the sorted arrays, already implicitly
checks for comparability. We only need this when we're leaving early.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 6c6f25904ee34fc34ce760311197ead6fa64d8ab
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu Nov 23 12:47:45 2023 +1300
libcli/security: add shortcuts for conditional ACE compare
If the number of members does not match in certain ways we can
say the sets are not equal without comparing the members.
We first need to check for comparability, though, so that we can return
an error if things aren't comparable.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 8bad19c42e14f0f7d459db41577a950f97306bcc
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Nov 10 16:27:45 2023 +1300
libcli/security: improve conditional ACE composite comparison
We had the comparison method wrong. Composites are compared as sets or
flabby sets, depending on their origin. Until now we compared them as
something a bit like sets, but not quite, in a maximally inefficient way.
Claims are always sets, and the left hand side is always a claim, but
literal composites on the right hand side can be multi-sets
(containing duplicate values). When it comes to comparison, composites
are reduced down to sets. To do the comparison we sort each side and
compare in order.
The fact that either side might ask for case-sensitive comparison (if
it is a claim) is an interesting complication.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 6a07d2fe44e0b699b4485963d3d47510e2ef275d
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Nov 17 13:58:12 2023 +1300
libcli/security: separate out claim_v1_to_ace_composite_unchecked()
For SDDL Resource ACE conversions we don't want to check too much
claim validity so that a semi-invalid ACE can round-trip through
deserialisation and serialisation. This is because Windows allows it,
but also because if the check puts the values in a sorted order that
makes the round-trip less round (that is, the return string is
semantically the same but possibly different in byte order).
The validity we're talking about is mostly uniqueness. For example
`S:(RA;;;;;WD;("foo",TU,0,7,5,7))` has two 7s, and that would be
invalid as a claim, but this is not checked while in ACE form.
On the other hand `S:(RA;;;;;WD;("foo",TU,0,3,2))` is valid, but the
return string will have 3 and 2 reversed when the check is made. We
prefer the ACE to stay the same while it is just being an ACE.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit e338625ebf1d595e10bfb55a8e35b5b48d8f2c28
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Nov 17 12:01:15 2023 +1300
libcli/security: avoid leak on SDDL encode failure
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 4f56c702834072d0353072b28a317208eeabb886
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Nov 22 16:40:12 2023 +1300
libcli/security: claim_v1_to_ace_token(): avoid unnecessary re-sort
If it is a wire claim (which is probably most common), the checking
and sorting has already happened. We don't need to make a copy to
sort and check.
In either case, there is still a copy step to make the conditional ACE
token.
This shuffles around some knownfails because the claim_v1_copy()
function we were using is checking for duplicates, which we don't
always want. That will be fixed soon.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit e223ce4a930d5c90d9effe37ac324ec159b35c9b
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Nov 22 16:38:13 2023 +1300
libcli/security: add_claim_to_token() re-sorts/checks claims
This function is used in tests and fuzzing.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 843fd4d15f7bba28e517e6d5b7b922a834f72565
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Nov 15 16:51:19 2023 +1300
libcli/security: resource attribute claims use claim_v1_check_and_sort()
Because RA ACEs live a double life, sometimes being ACEs and sometimes
being claims, we make a copy of the claim strucutre for sorting and
further use in conditional ACEs.
We don't need to do that for wire claims, because they are not
persistent or forwarded on to somewhere else.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 8074257c3ae04f3a3c6a5e546d3c8267e1b2d05b
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu Nov 2 18:13:13 2023 +1300
libcli/security: wire claim conversion uses claim_v1_check_and_sort()
This roughly returns things to where they were a few commits ago, with
the claims being checked for uniqueness.
The difference is the claims will be sorted afterwards, and the
uniqueness check will be far more efficient on large claims.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 4b032d2558439cb8199934a29c8918bef9607eb0
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Nov 22 16:14:25 2023 +1300
libcli/security: claim_v1_check_and_sort(): add all types
To manage this sort we need a qsort_r-like sort context which holds:
a) the value type,
b) a case sensitive flag for the string compare, and
c) a return flag indicating a failure. Failures are not picked up until
after the sort finishes.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit a19f914fb9fd5321cf76b6daa399ef6918d577dc
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Nov 22 11:07:29 2023 +1300
libcli/security: begin claim_v1_check_and_sort with Boolean checks
claim_v1_check_and_sort() is meant to sort the claim values and check
that there are no duplicates, as well as making some value checks.
In order to ease into the idea, we look first at the case where the claim
has Boolean values. There are only two values allowed, which limits the
length of a valid claim set and means we only really need to "sort" in
the {1, 0} case, which we rewrite in place as {0, 1}.
That's what will happen with other types: we'll sort in-place, make
some checks on values, set flags, and return an error if there are
duplicates or value errors.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 4ebb488e512dcaeb1aa1866dd40d2515f9c5da96
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Nov 24 17:59:24 2023 +1300
libcli/security: don't allow two NULL string claims
This restores the behaviour with regard to duplicate NULL strings that
existed before the last commit. I'm putting it separately, because it
seems so strange, and I not entirely certain the behaviour is
intentional.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 1c88dfc6ac5021beb1ab92a394d38adf44eede62
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Nov 22 14:57:09 2023 +1300
libcli/security: wire claims conversion: remove strings uniqueness check
This changes the behaviour when one of the strings is NULL. Previously
a single NULL string would be ignored, and two would cause an error.
That will be restored in the next commit.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 08096fd5b40b0759ab086f2c657cca26ce3f9369
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Nov 22 14:54:25 2023 +1300
libcli/security: int wire claims drop uniqueness check
And we allocate all the values together as an array, because
we might as well.
This and the next couple of commits might look like steps backwards,
and they are, but they allow us to get a run-up to leap over a big
fence.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 10fd3e5836c002d8f2f4cc8d1a497c24c714493b
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Nov 22 14:48:31 2023 +1300
libcli/security: simplify wire claim conversion mem, 3/3: rm tmp_ctx
The interstitial tmp_ctx now does nothing but be interstitial, so
let's get rid of it.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit d7da41a9bef66a428f6f11a37bad4adfb6f89278
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Nov 22 14:44:30 2023 +1300
libcli/security: simplify wire claim conversion mem, 2/3: one tree
These values would have leaked in the event of failure (but only onto
the caller mem_ctx, which might be fleeting -- especially as its
security token is now failing).
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 7656d1333459e2ebd3d9983283629609d0db6d96
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Nov 22 14:39:49 2023 +1300
libcli/security: simplify wire claim conversion mem, 1/3: avoid NULL parent
The reason for this, apart from weighing up possible over-allocations
vs realloc costs, is in the first iteration of the loop,
claim_values = talloc_array(claims,
would allocate onto NULL, which leaks.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit a836ad14422e55ec07fb51d6a6e0f5e5a211bf5a
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Tue Nov 14 12:53:24 2023 +1300
pytest: conditional_ace_claims tests large composite comparisons
Our composite comparisons are currently all wrong.
Soon they will be fixed, but we are going to have an inflection point
where we switch from the naive compare-everything approach to a sort
based comparison, and we want to test both sides. Also, we use these
tests for a little bit of timing, which reveals it is all fast enough.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit cea44421ebc1242485dbf614e891c1786a800594
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Mon Nov 13 12:56:13 2023 +1300
libcli/security/sddl: improve some SDDL error messages
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit bc9da9568223ddfc8b70384f895c41b7cb6a9b13
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Mon Nov 13 13:34:57 2023 +1300
pytest: conditional_ace_claims: write_c_test_on_failure() copes with claims
*copes badly, but better than crashing.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 4cc91d28283bcded7d2d736d39345e9c4bec6f74
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Tue Nov 14 12:46:21 2023 +1300
pytest: token_factory note that a flag is not set
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 51d9444baa083e7e2c92e422227df681d466cd7b
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Mon Nov 13 13:36:00 2023 +1300
pytest: token_factory copes with empty claims
We don't have a good story yet with regard to empty claims, but we at
least want to be able to create them in tests.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit f9f87247188418bd020678937d1da07a22cf9e1c
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Tue Nov 14 12:51:10 2023 +1300
pytest: token_factory claims can have case_sensitive flag
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit adf695aa343e8519347ad434558029a546850501
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Mon Nov 13 12:57:13 2023 +1300
pytest: token_factory separate out list_to_claim() helper
This is so conditional_ace_claims test can create claim objects which
can e.g. have the case sensitive flag set.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit fc4801442925f1fd78eaa4b88c562be98e806436
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Nov 10 16:19:40 2023 +1300
libcli/security: sddl_conditional_ace: check a talloc_new()
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 78506e1752aea9a68638e492e0ea73e8726f4990
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Nov 10 16:18:32 2023 +1300
libcli/security: conditional ACE sddl writers take const tokens
We don't change these when writing the SDDL.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 33d2deec514048f200bfb57a7dd64d6f9876f42a
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Nov 10 16:15:25 2023 +1300
lib/security:CA: tokens_are_comparable() accepts NULL operator
In some circumstances we are going to know general comparability
without having an operator around to use.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 6e15a20228c78e0f7b74da2fc4bc7edd78b6507f
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Nov 10 16:13:40 2023 +1300
libcli/security: CA: tokens_are_comparable() considers the obvious
Existing callers already make this check, but we are soon going to use
it in contexts that don't.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit fc890742ab47a11ce68d47c3eaaa92413a3df125
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Nov 24 13:37:42 2023 +1300
libcli/security: add test_claims_conversion
These are unit tests for converting wire claims into sorted claims v1
structures.
These are based from packets derived from the krb5.conditional_ace
tests, and currently don't test more than they do, but they work about
a hundred thousand times quicker.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit da077b8486251de97e4920dbdd481e7f8d0e4428
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Nov 17 11:30:03 2023 +1300
libcli/security: test_run_conditional_ace tests more comparisons
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 2f40583ab2f0123736dbd71f4d1d0711672458e7
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Nov 17 11:29:06 2023 +1300
libcli/security: test_run_conditional_ace can set debug levels
No -d, just `bin/test_run_conditional_ace 3`.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit e81e0706388c86bb3261443d422a910fb5fb4d3c
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Nov 10 16:56:04 2023 +1300
librpc/idl:security: add claims flag indicating orderly and unique members
The same flag will be used in conditional ACE composites, and on
CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 structures derived from wire
claims and resource attribute ACEs, when we know we have checked the
claim has no duplicate values.
Resource Attribute ACEs contain CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1
at rest, but we are not going to set the flag there on the off chance
that the ACE could fly off to another application and have another
application specific meaning there. We will only check for uniqueness
and set the flag on ephemeral copies of resource claims during access
check operations.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit a8f83fe8a2ff2f5ab5f36538df26b097810c417a
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Nov 10 14:53:44 2023 +1300
librpc/idl:security: add a couple of claims flags
We don't use these.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit f4ea27b84a5d6b06576e13798835fcb7fcb44d5e
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Nov 10 14:31:59 2023 +1300
librpc/idl:condtional_ace: shift CONDITIONAL_ACE_FLAG_TOKEN_FROM_ATTR to last bit
This region is "available for application-specific data" in the
CLAIM_SECURITY_ATTRIBUTE_ space, according to [MS-DTYP] 2.4.10.1,
so it nicer to use that, even though we are not actually setting the
flag on the V1 claims.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 6aa6ef4b7c14bcafeff288c9b83abef92a9f78df
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Nov 10 14:24:48 2023 +1300
librpc/idl:conditional_ace: make a flags field 32 bit
This allows it to align with
CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1.flags, with which it shares
values and will soon share more.
It was 16 bit because we needed few flags, and at one point .type was
8 bit, so 16 bits packed nicely into a smaller struct.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit ca572691622ca30097d172fe2843dca34eb3764f
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Tue Nov 28 10:46:40 2023 +1300
libcli/security: remove redundant claim SID size check
blob_string_sid_to_sid() immediately checks the size is within 5-191, so the 1-10000
just gives you a different message in chircumstances you'll never see.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit fa96bbbe8160fa5bfdbae21baf77103ffeaf1995
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Tue Nov 28 10:35:43 2023 +1300
libcli/security: avoid leak when converting SID claims
Apart from the leak fix, this is faster and stricter, not accepting
SID string buffers with trailing garbage ("S-1-2-3qwerty" would have
been accepted, but not now).
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
-----------------------------------------------------------------------
Summary of changes:
libcli/security/claims-conversions.c | 569 +++++++++++++++++------
libcli/security/claims-conversions.h | 9 +
libcli/security/conditional_ace.c | 451 ++++++++++++++++--
libcli/security/sddl.c | 11 +-
libcli/security/sddl_conditional_ace.c | 17 +-
libcli/security/tests/data/ndr_dumps/fileb5iJt4 | Bin 0 -> 118 bytes
libcli/security/tests/data/ndr_dumps/fileb8cNVS | Bin 0 -> 360 bytes
libcli/security/tests/data/ndr_dumps/filebI7h5H | Bin 0 -> 112 bytes
libcli/security/tests/data/ndr_dumps/filebNdBgt | Bin 0 -> 344 bytes
libcli/security/tests/data/ndr_dumps/filebOjK4H | Bin 0 -> 124 bytes
libcli/security/tests/data/ndr_dumps/filebzCPTH | Bin 0 -> 480 bytes
libcli/security/tests/test_claim_conversion.c | 171 +++++++
libcli/security/tests/test_run_conditional_ace.c | 66 ++-
libcli/security/wscript_build | 18 +-
librpc/idl/conditional_ace.idl | 23 +-
librpc/idl/security.idl | 48 +-
python/samba/tests/conditional_ace_claims.py | 512 +++++++++++++++++++-
python/samba/tests/token_factory.py | 59 ++-
selftest/knownfail.d/conditional_ace_claims | 1 +
selftest/tests.py | 2 +
20 files changed, 1733 insertions(+), 224 deletions(-)
create mode 100644 libcli/security/tests/data/ndr_dumps/fileb5iJt4
create mode 100644 libcli/security/tests/data/ndr_dumps/fileb8cNVS
create mode 100644 libcli/security/tests/data/ndr_dumps/filebI7h5H
create mode 100644 libcli/security/tests/data/ndr_dumps/filebNdBgt
create mode 100644 libcli/security/tests/data/ndr_dumps/filebOjK4H
create mode 100644 libcli/security/tests/data/ndr_dumps/filebzCPTH
create mode 100644 libcli/security/tests/test_claim_conversion.c
create mode 100644 selftest/knownfail.d/conditional_ace_claims
Changeset truncated at 500 lines:
diff --git a/libcli/security/claims-conversions.c b/libcli/security/claims-conversions.c
index 968ba1efdb3..22ab252173a 100644
--- a/libcli/security/claims-conversions.c
+++ b/libcli/security/claims-conversions.c
@@ -25,6 +25,7 @@
#include "lib/util/tsort.h"
#include "lib/util/debug.h"
#include "lib/util/bytearray.h"
+#include "lib/util/stable_sort.h"
#include "librpc/gen_ndr/conditional_ace.h"
#include "librpc/gen_ndr/claims.h"
@@ -99,8 +100,52 @@ static bool claim_v1_octet_string_to_ace_octet_string(
}
+static bool blob_string_sid_to_sid(DATA_BLOB *blob,
+ struct dom_sid *sid)
+{
+ /*
+ * Resource ACE claim SIDs are stored as SID strings in
+ * CLAIM_SECURITY_ATTRIBUTE_OCTET_STRING_RELATIVE blobs. These are in
+ * ACEs, which means we don't quite know who wrote them, and it is
+ * unspecified whether the blob should contain a terminating NUL byte.
+ * Therefore we accept either form, copying into a temporary buffer if
+ * there is no '\0'. Apart from this special case, we don't accept
+ * SIDs that are shorter than the blob.
+ *
+ * It doesn't seem like SDDL short SIDs ("WD") are accepted here. This
+ * isn't SDDL.
+ */
+ bool ok;
+ size_t len = blob->length;
+ char buf[DOM_SID_STR_BUFLEN + 1]; /* 191 + 1 */
+ const char *end = NULL;
+ char *str = NULL;
+
+ if (len < 5 || len >= DOM_SID_STR_BUFLEN) {
+ return false;
+ }
+ if (blob->data[len - 1] == '\0') {
+ str = (char *)blob->data;
+ len--;
+ } else {
+ memcpy(buf, blob->data, len);
+ buf[len] = 0;
+ str = buf;
+ }
+
+ ok = dom_sid_parse_endp(str, sid, &end);
+ if (!ok) {
+ return false;
+ }
+
+ if (end - str != len) {
+ return false;
+ }
+ return true;
+}
+
+
static bool claim_v1_sid_to_ace_sid(
- TALLOC_CTX *mem_ctx,
const struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *claim,
size_t offset,
struct ace_condition_token *result)
@@ -114,27 +159,19 @@ static bool claim_v1_sid_to_ace_sid(
* There are no SIDs in ADTS claims, but there can be in
* resource ACEs.
*/
- struct dom_sid *sid = NULL;
DATA_BLOB *v = NULL;
+ bool ok;
v = claim->values[offset].sid_value;
- if (v->length == 0 || v->length > CONDITIONAL_ACE_MAX_LENGTH) {
- DBG_WARNING("claim has SID string of unexpected length %zu, "
- "(expected range 1 - %u)\n",
- v->length, CONDITIONAL_ACE_MAX_LENGTH);
- return false;
- }
-
- sid = dom_sid_parse_length(mem_ctx, v);
- if (sid == NULL) {
+ ok = blob_string_sid_to_sid(v, &result->data.sid.sid);
+ if (! ok) {
DBG_WARNING("claim has invalid SID string of length %zu.\n",
v->length);
return false;
}
result->type = CONDITIONAL_ACE_TOKEN_SID;
- result->data.sid.sid = *sid;
return true;
}
@@ -238,7 +275,7 @@ static bool claim_v1_offset_to_ace_token(
return claim_v1_string_to_ace_string(mem_ctx, claim, offset,
result);
case CLAIM_SECURITY_ATTRIBUTE_TYPE_SID:
- return claim_v1_sid_to_ace_sid(mem_ctx, claim, offset, result);
+ return claim_v1_sid_to_ace_sid(claim, offset, result);
case CLAIM_SECURITY_ATTRIBUTE_TYPE_BOOLEAN:
return claim_v1_bool_to_ace_int(claim, offset, result);
case CLAIM_SECURITY_ATTRIBUTE_TYPE_OCTET_STRING:
@@ -252,14 +289,76 @@ static bool claim_v1_offset_to_ace_token(
}
+static bool claim_v1_copy(
+ TALLOC_CTX *mem_ctx,
+ struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *dest,
+ const struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *src);
+
+
+
+bool claim_v1_to_ace_composite_unchecked(
+ TALLOC_CTX *mem_ctx,
+ const struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *claim,
+ struct ace_condition_token *result)
+{
+ /*
+ * This converts a claim object into a conditional ACE
+ * composite without checking whether it is a valid and sorted
+ * claim. It is called in two places:
+ *
+ * 1. claim_v1_to_ace_token() below (which does do those
+ * checks, and is the function you want).
+ *
+ * 2. sddl_resource_attr_from_claim() in which a resource
+ * attribute claim needs to pass through a conditional ACE
+ * composite structure on its way to becoming SDDL. In that
+ * case we don't want to check validity.
+ */
+ size_t i;
+ struct ace_condition_token *tokens = NULL;
+ bool ok;
+
+ tokens = talloc_array(mem_ctx,
+ struct ace_condition_token,
+ claim->value_count);
+ if (tokens == NULL) {
+ return false;
+ }
+
+ for (i = 0; i < claim->value_count; i++) {
+ ok = claim_v1_offset_to_ace_token(tokens,
+ claim,
+ i,
+ &tokens[i]);
+ if (! ok) {
+ TALLOC_FREE(tokens);
+ return false;
+ }
+ }
+
+ result->type = CONDITIONAL_ACE_TOKEN_COMPOSITE;
+ result->data.composite.tokens = tokens;
+ result->data.composite.n_members = claim->value_count;
+ result->flags = claim->flags & CLAIM_SECURITY_ATTRIBUTE_VALUE_CASE_SENSITIVE;
+ return true;
+}
+
+
bool claim_v1_to_ace_token(TALLOC_CTX *mem_ctx,
const struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *claim,
struct ace_condition_token *result)
{
- size_t i;
- struct ace_condition_token *tokens = NULL;
+ struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *claim_copy = NULL;
+ const struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *sorted_claim = NULL;
+ NTSTATUS status;
+ bool ok;
+ bool case_sensitive = claim->flags & \
+ CLAIM_SECURITY_ATTRIBUTE_VALUE_CASE_SENSITIVE;
+
if (claim->value_count < 1 ||
claim->value_count >= CONDITIONAL_ACE_MAX_TOKENS) {
+ DBG_WARNING("rejecting claim with %"PRIu32" tokens\n",
+ claim->value_count);
return false;
}
/*
@@ -273,6 +372,53 @@ bool claim_v1_to_ace_token(TALLOC_CTX *mem_ctx,
0,
result);
}
+
+ if (claim->flags & CLAIM_SECURITY_ATTRIBUTE_UNIQUE_AND_SORTED) {
+ /*
+ * We can avoid making a sorted copy.
+ *
+ * This is normal case for wire claims, where the
+ * sorting and duplicate checking happens earlier in
+ * token_claims_to_claims_v1().
+ */
+ sorted_claim = claim;
+ } else {
+ /*
+ * This is presumably a resource attribute ACE, which
+ * is stored in the ACE as struct
+ * CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1, and we don't
+ * really want to mutate that copy -- even if there
+ * aren't currently realistic pathways that read an
+ * ACE, trigger this, and write it back (outside of
+ * tests).
+ */
+ claim_copy = talloc(mem_ctx, struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1);
+ if (claim_copy == NULL) {
+ return false;
+ }
+
+ ok = claim_v1_copy(claim_copy, claim_copy, claim);
+ if (!ok) {
+ TALLOC_FREE(claim_copy);
+ return false;
+ }
+
+ status = claim_v1_check_and_sort(claim_copy, claim_copy,
+ case_sensitive);
+ if (!NT_STATUS_IS_OK(status)) {
+ DBG_WARNING("resource attribute claim sort failed with %s\n",
+ nt_errstr(status));
+ TALLOC_FREE(claim_copy);
+ return false;
+ }
+ sorted_claim = claim_copy;
+ }
+ ok = claim_v1_to_ace_composite_unchecked(mem_ctx, sorted_claim, result);
+ if (! ok) {
+ TALLOC_FREE(claim_copy);
+ return false;
+ }
+
/*
* The multiple values will get turned into a composite
* literal in the conditional ACE. Each element of the
@@ -281,32 +427,9 @@ bool claim_v1_to_ace_token(TALLOC_CTX *mem_ctx,
* set here (at least the _FROM_ATTR flag) or the child values
* will not be reached.
*/
-
- result->flags = (
+ result->flags |= (
CONDITIONAL_ACE_FLAG_TOKEN_FROM_ATTR |
- (claim->flags & CLAIM_SECURITY_ATTRIBUTE_VALUE_CASE_SENSITIVE));
-
- tokens = talloc_array(mem_ctx,
- struct ace_condition_token,
- claim->value_count);
- if (tokens == NULL) {
- return false;
- }
-
- for (i = 0; i < claim->value_count; i++) {
- bool ok = claim_v1_offset_to_ace_token(tokens,
- claim,
- i,
- &tokens[i]);
- if (! ok) {
- TALLOC_FREE(tokens);
- return false;
- }
- }
-
- result->type = CONDITIONAL_ACE_TOKEN_COMPOSITE;
- result->data.composite.tokens = tokens;
- result->data.composite.n_members = claim->value_count;
+ CLAIM_SECURITY_ATTRIBUTE_UNIQUE_AND_SORTED);
return true;
}
@@ -631,6 +754,7 @@ bool add_claim_to_token(TALLOC_CTX *mem_ctx,
const char *claim_type)
{
struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *tmp = NULL;
+ NTSTATUS status;
uint32_t *n = NULL;
bool ok;
struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 **list = NULL;
@@ -660,22 +784,218 @@ bool add_claim_to_token(TALLOC_CTX *mem_ctx,
ok = claim_v1_copy(mem_ctx, &tmp[*n], claim);
if (! ok ) {
+ TALLOC_FREE(tmp);
return false;
}
+
+ status = claim_v1_check_and_sort(tmp, &tmp[*n],
+ claim->flags & CLAIM_SECURITY_ATTRIBUTE_VALUE_CASE_SENSITIVE);
+ if (!NT_STATUS_IS_OK(status)) {
+ DBG_WARNING("resource attribute claim sort failed with %s\n",
+ nt_errstr(status));
+ TALLOC_FREE(tmp);
+ return false;
+ }
+
(*n)++;
*list = tmp;
return true;
}
+
+static NTSTATUS claim_v1_check_and_sort_boolean(
+ TALLOC_CTX *mem_ctx,
+ struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *claim)
+{
+ /*
+ * There are so few valid orders in a boolean claim that we can
+ * enumerate them all.
+ */
+ switch (claim->value_count) {
+ case 0:
+ return NT_STATUS_OK;
+ case 1:
+ if (*claim->values[0].uint_value == 0 ||
+ *claim->values[0].uint_value == 1) {
+ return NT_STATUS_OK;
+ }
+ break;
+ case 2:
+ if (*claim->values[0].uint_value == 1) {
+ /* switch the order. */
+ *claim->values[0].uint_value = *claim->values[1].uint_value;
+ *claim->values[1].uint_value = 1;
+ }
+ if (*claim->values[0].uint_value == 0 &&
+ *claim->values[1].uint_value == 1) {
+ return NT_STATUS_OK;
+ }
+ break;
+ default:
+ /* 3 or more must have duplicates. */
+ break;
+ }
+ return NT_STATUS_INVALID_PARAMETER;
+}
+
+
+struct claim_sort_context {
+ uint16_t value_type;
+ bool failed;
+ bool case_sensitive;
+};
+
+static int claim_sort_cmp(const union claim_values *lhs,
+ const union claim_values *rhs,
+ struct claim_sort_context *ctx)
+{
+ /*
+ * These comparisons have to match those used in
+ * conditional_ace.c.
+ */
+ int cmp;
+
+ switch (ctx->value_type) {
+ case CLAIM_SECURITY_ATTRIBUTE_TYPE_INT64:
+ case CLAIM_SECURITY_ATTRIBUTE_TYPE_UINT64:
+ {
+ /*
+ * We sort as signed integers, even for uint64,
+ * because a) we don't actually care about the true
+ * order, just uniqueness, and b) the conditional ACEs
+ * only know of signed values.
+ */
+ int64_t a, b;
+ if (ctx->value_type == CLAIM_SECURITY_ATTRIBUTE_TYPE_INT64) {
+ a = *lhs->int_value;
+ b = *rhs->int_value;
+ } else {
+ a = (int64_t)*lhs->uint_value;
+ b = (int64_t)*rhs->uint_value;
+ }
+ if (a < b) {
+ return -1;
+ }
+ if (a == b) {
+ return 0;
+ }
+ return 1;
+ }
+ case CLAIM_SECURITY_ATTRIBUTE_TYPE_STRING:
+ {
+ const char *a = lhs->string_value;
+ const char *b = rhs->string_value;
+ if (ctx->case_sensitive) {
+ return strcmp(a, b);
+ }
+ return strcasecmp_m(a, b);
+ }
+
+ case CLAIM_SECURITY_ATTRIBUTE_TYPE_SID:
+ {
+ /*
+ * The blobs in a claim are "S-1-.." strings, not struct
+ * dom_sid as used in conditional ACEs, and to sort them the
+ * same as ACEs we need to make temporary structs.
+ *
+ * We don't accept SID claims over the wire -- these
+ * are resource attribute ACEs only.
+ */
+ struct dom_sid a, b;
+ bool lhs_ok, rhs_ok;
+
+ lhs_ok = blob_string_sid_to_sid(lhs->sid_value, &a);
+ rhs_ok = blob_string_sid_to_sid(rhs->sid_value, &b);
+ if (!(lhs_ok && rhs_ok)) {
+ ctx->failed = true;
+ return -1;
+ }
+ cmp = dom_sid_compare(&a, &b);
+ return cmp;
+ }
+ case CLAIM_SECURITY_ATTRIBUTE_TYPE_OCTET_STRING:
+ {
+ const DATA_BLOB *a = lhs->octet_value;
+ const DATA_BLOB *b = rhs->octet_value;
+ return data_blob_cmp(a, b);
+ }
+ default:
+ ctx->failed = true;
+ break;
+ }
+ return -1;
+}
+
+
+NTSTATUS claim_v1_check_and_sort(TALLOC_CTX *mem_ctx,
+ struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *claim,
+ bool case_sensitive)
+{
+ bool ok;
+ uint32_t i;
+ struct claim_sort_context sort_ctx = {
+ .failed = false,
+ .value_type = claim->value_type,
+ .case_sensitive = case_sensitive
+ };
+
+ if (claim->value_type == CLAIM_SECURITY_ATTRIBUTE_TYPE_BOOLEAN) {
+ NTSTATUS status = claim_v1_check_and_sort_boolean(mem_ctx, claim);
+ if (NT_STATUS_IS_OK(status)) {
+ claim->flags |= CLAIM_SECURITY_ATTRIBUTE_UNIQUE_AND_SORTED;
+ }
+ return status;
+ }
+
+ ok = stable_sort_talloc_r(mem_ctx,
+ claim->values,
+ claim->value_count,
+ sizeof(union claim_values),
+ (samba_compare_with_context_fn_t)claim_sort_cmp,
+ &sort_ctx);
+ if (!ok) {
+ return NT_STATUS_NO_MEMORY;
+ }
+
+ if (sort_ctx.failed) {
+ /* this failure probably means a bad SID string */
+ DBG_WARNING("claim sort of %"PRIu32" members, type %"PRIu16" failed\n",
+ claim->value_count,
+ claim->value_type);
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
+ for (i = 1; i < claim->value_count; i++) {
+ int cmp = claim_sort_cmp(&claim->values[i - 1],
+ &claim->values[i],
+ &sort_ctx);
+ if (cmp == 0) {
+ DBG_WARNING("duplicate values in claim\n");
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+ if (cmp > 0) {
+ DBG_ERR("claim sort failed!\n");
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+ }
+ if (case_sensitive) {
+ claim->flags |= CLAIM_SECURITY_ATTRIBUTE_VALUE_CASE_SENSITIVE;
+ }
+ claim->flags |= CLAIM_SECURITY_ATTRIBUTE_UNIQUE_AND_SORTED;
+ return NT_STATUS_OK;
+}
+
+
NTSTATUS token_claims_to_claims_v1(TALLOC_CTX *mem_ctx,
const struct CLAIMS_SET *claims_set,
struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 **out_claims,
uint32_t *out_n_claims)
{
- TALLOC_CTX *tmp_ctx = NULL;
struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *claims = NULL;
uint32_t n_claims = 0;
+ uint32_t expected_n_claims = 0;
uint32_t i;
+ NTSTATUS status;
--
Samba Shared Repository
More information about the samba-cvs
mailing list