Looks like we do not have self-tests for smbcacls

classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
Hi folks,

I was trying to set an ACL via smbcacls with the following command:

smbcacls //localhost/someshare some-dir --sddl -Uetc -S
'some-long-sddl-dumped-from-windows-with-smbcacls'

and I got this error:

../source3/rpc_client/cli_pipe.c:568: RPC fault code
DCERPC_NCA_S_OP_RNG_ERROR received from host localhost!

And when I look in the logfile I see:

              level                    : LSA_POLICY_INFO_DOMAIN (3)
[2017/07/12 00:27:29.877240,  4, pid=24646, effective(361800500,
361800513), real(361800500, 0), class=rpc_srv]
../source3/rpc_server/srv_pipe.c:1485(api_rpcTNP)
  api_rpcTNP: fault(469827586) return.
[2017/07/12 00:27:29.877257,  1, pid=24646, effective(361800500,
361800513), real(361800500, 0)]
../librpc/ndr/ndr.c:413(ndr_print_debug)
       &r: struct ncacn_packet
          rpc_vers                 : 0x05 (5)
          rpc_vers_minor           : 0x00 (0)
          ptype                    : DCERPC_PKT_FAULT (3)
          pfc_flags                : 0x23 (35)
                 1: DCERPC_PFC_FLAG_FIRST
                 1: DCERPC_PFC_FLAG_LAST
                 0: DCERPC_PFC_FLAG_PENDING_CANCEL_OR_HDR_SIGNING
                 0: DCERPC_PFC_FLAG_CONC_MPX
                 1: DCERPC_PFC_FLAG_DID_NOT_EXECUTE
                 0: DCERPC_PFC_FLAG_MAYBE
                 0: DCERPC_PFC_FLAG_OBJECT_UUID
          drep: ARRAY(4)
              [0]                      : 0x10 (16)
              [1]                      : 0x00 (0)
              [2]                      : 0x00 (0)
              [3]                      : 0x00 (0)
          frag_length              : 0x0020 (32)
          auth_length              : 0x0000 (0)
          call_id                  : 0x00000003 (3)
          u                        : union dcerpc_payload(case 3)
          fault: struct dcerpc_fault
              alloc_hint               : 0x00000000 (0)
              context_id               : 0x0000 (0)
              cancel_count             : 0x00 (0)
              status                   : DCERPC_NCA_S_OP_RNG_ERROR (469827586)
              _pad                     : DATA_BLOB length=4


--
Regards,
Richard Sharpe
(何以解憂?唯有杜康。--曹操)

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
On Tue, Jul 11, 2017 at 05:34:56PM -0700, Richard Sharpe via samba-technical wrote:

> Hi folks,
>
> I was trying to set an ACL via smbcacls with the following command:
>
> smbcacls //localhost/someshare some-dir --sddl -Uetc -S
> 'some-long-sddl-dumped-from-windows-with-smbcacls'
>
> and I got this error:
>
> ../source3/rpc_client/cli_pipe.c:568: RPC fault code
> DCERPC_NCA_S_OP_RNG_ERROR received from host localhost!
>
> And when I look in the logfile I see:
>
>               level                    : LSA_POLICY_INFO_DOMAIN (3)
> [2017/07/12 00:27:29.877240,  4, pid=24646, effective(361800500,
> 361800513), real(361800500, 0), class=rpc_srv]
> ../source3/rpc_server/srv_pipe.c:1485(api_rpcTNP)
>   api_rpcTNP: fault(469827586) return.
> [2017/07/12 00:27:29.877257,  1, pid=24646, effective(361800500,
> 361800513), real(361800500, 0)]
> ../librpc/ndr/ndr.c:413(ndr_print_debug)
>        &r: struct ncacn_packet
>           rpc_vers                 : 0x05 (5)
>           rpc_vers_minor           : 0x00 (0)
>           ptype                    : DCERPC_PKT_FAULT (3)
>           pfc_flags                : 0x23 (35)
>                  1: DCERPC_PFC_FLAG_FIRST
>                  1: DCERPC_PFC_FLAG_LAST
>                  0: DCERPC_PFC_FLAG_PENDING_CANCEL_OR_HDR_SIGNING
>                  0: DCERPC_PFC_FLAG_CONC_MPX
>                  1: DCERPC_PFC_FLAG_DID_NOT_EXECUTE
>                  0: DCERPC_PFC_FLAG_MAYBE
>                  0: DCERPC_PFC_FLAG_OBJECT_UUID
>           drep: ARRAY(4)
>               [0]                      : 0x10 (16)
>               [1]                      : 0x00 (0)
>               [2]                      : 0x00 (0)
>               [3]                      : 0x00 (0)
>           frag_length              : 0x0020 (32)
>           auth_length              : 0x0000 (0)
>           call_id                  : 0x00000003 (3)
>           u                        : union dcerpc_payload(case 3)
>           fault: struct dcerpc_fault
>               alloc_hint               : 0x00000000 (0)
>               context_id               : 0x0000 (0)
>               cancel_count             : 0x00 (0)
>               status                   : DCERPC_NCA_S_OP_RNG_ERROR (469827586)
>               _pad                     : DATA_BLOB length=4

Can you get more info on the failure - a capture trace maybe ?

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
Hi
On 12/07/17 22:25, Jeremy Allison via samba-technical wrote:

> On Tue, Jul 11, 2017 at 05:34:56PM -0700, Richard Sharpe via samba-technical wrote:
>> Hi folks,
>>
>> I was trying to set an ACL via smbcacls with the following command:
>>
>> smbcacls //localhost/someshare some-dir --sddl -Uetc -S
>> 'some-long-sddl-dumped-from-windows-with-smbcacls'
>>
>> and I got this error:
>>
>> ../source3/rpc_client/cli_pipe.c:568: RPC fault code
>> DCERPC_NCA_S_OP_RNG_ERROR received from host localhost!
>>
>> And when I look in the logfile I see:
>>
[...]

Let me shamelessly hijack this thread (sortof), with the attached
patchset (rebased for current master) for my propagate inheritance
related smbcacls changes includes selftests for smbcacls (that can be
fleshed out more). Note: The tests (& my patch) currently fail since
commit 1199907cbe2 "param: change the effective default for "client max
protocol" to the latest supported protocol" Please see the separate
message to the list with title "RFC: smbcacls fails with windows with
SMB2 (succeeds with SMB1 only)" for more details and a 'maybe' fix for
that. Dave, if you have a chance can you have a look at these patches
again :-))

Noel



smbcacls_review#6.patch (121K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
On Thu, 20 Jul 2017 14:32:18 +0100, Noel Power wrote:

> Let me shamelessly hijack this thread (sortof), with the attached
> patchset (rebased for current master) for my propagate inheritance
> related smbcacls changes includes selftests for smbcacls (that can be
> fleshed out more). Note: The tests (& my patch) currently fail since
> commit 1199907cbe2 "param: change the effective default for "client max
> protocol" to the latest supported protocol" Please see the separate
> message to the list with title "RFC: smbcacls fails with windows with
> SMB2 (succeeds with SMB1 only)" for more details and a 'maybe' fix for
> that. Dave, if you have a chance can you have a look at these patches
> again :-))

Thanks for bringing this back up, Noel. I'll take a look through the
comments I had last time around and see what's been fixed / needs to be
addressed.

Cheers, David

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
Hi Noel,

>>> I was trying to set an ACL via smbcacls with the following command:
>>>
>>> smbcacls //localhost/someshare some-dir --sddl -Uetc -S
>>> 'some-long-sddl-dumped-from-windows-with-smbcacls'
>>>
>>> and I got this error:
>>>
>>> ../source3/rpc_client/cli_pipe.c:568: RPC fault code
>>> DCERPC_NCA_S_OP_RNG_ERROR received from host localhost!
>>>
>>> And when I look in the logfile I see:
>>>
> [...]
>
> Let me shamelessly hijack this thread (sortof), with the attached
> patchset (rebased for current master) for my propagate inheritance
> related smbcacls changes includes selftests for smbcacls (that can be
> fleshed out more). Note: The tests (& my patch) currently fail since
> commit 1199907cbe2 "param: change the effective default for "client max
> protocol" to the latest supported protocol" Please see the separate
> message to the list with title "RFC: smbcacls fails with windows with
> SMB2 (succeeds with SMB1 only)" for more details and a 'maybe' fix for
> that. Dave, if you have a chance can you have a look at these patches
> again :-))
I'm sorry, but I'd really like to avoid start using perl scripts for
new tests, would it be possible to do it in python or shell?

metze



signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
Please find a new version of the patchset (with tests re-written in python)
Noel
On 20/07/17 21:19, Stefan Metzmacher wrote:

> Hi Noel,
>
>>>> I was trying to set an ACL via smbcacls with the following command:
>>>>
>>>> smbcacls //localhost/someshare some-dir --sddl -Uetc -S
>>>> 'some-long-sddl-dumped-from-windows-with-smbcacls'
>>>>
>>>> and I got this error:
>>>>
>>>> ../source3/rpc_client/cli_pipe.c:568: RPC fault code
>>>> DCERPC_NCA_S_OP_RNG_ERROR received from host localhost!
>>>>
>>>> And when I look in the logfile I see:
>>>>
>> [...]
>>
>> Let me shamelessly hijack this thread (sortof), with the attached
>> patchset (rebased for current master) for my propagate inheritance
>> related smbcacls changes includes selftests for smbcacls (that can be
>> fleshed out more). Note: The tests (& my patch) currently fail since
>> commit 1199907cbe2 "param: change the effective default for "client max
>> protocol" to the latest supported protocol" Please see the separate
>> message to the list with title "RFC: smbcacls fails with windows with
>> SMB2 (succeeds with SMB1 only)" for more details and a 'maybe' fix for
>> that. Dave, if you have a chance can you have a look at these patches
>> again :-))
> I'm sorry, but I'd really like to avoid start using perl scripts for
> new tests, would it be possible to do it in python or shell?
>
> metze
>
>


smbcacls_with_pytest.patch (103K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
On Mon, 2017-07-31 at 13:51 +0100, Noel Power via samba-technical
wrote:
> Please find a new version of the patchset (with tests re-written in python)
> Noel

Thanks for writing these tests.  It would have been better however if
the tests used the standard test classes like BlackboxTestCase and the
assertions that go with it.  

I know it will be a pile of pain to re-work the test (again!), but the
consistency really helps as others will copy and paste from the first
example they find.

See for example python/samba/tests/blackbox/ndrdump.py

(We used to have more of those, but SambaToolCmdTest took over for
Samba tests).

Sorry,

Andrew Bartlett
--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
On 07/31/2017 03:34 PM, Andrew Bartlett via samba-technical wrote:

> On Mon, 2017-07-31 at 13:51 +0100, Noel Power via samba-technical
> wrote:
>> Please find a new version of the patchset (with tests re-written in python)
>> Noel
>
> Thanks for writing these tests.  It would have been better however if
> the tests used the standard test classes like BlackboxTestCase and the
> assertions that go with it.  
>
> I know it will be a pile of pain to re-work the test (again!), but the
> consistency really helps as others will copy and paste from the first
> example they find.
>
> See for example python/samba/tests/blackbox/ndrdump.py
>
> (We used to have more of those, but SambaToolCmdTest took over for
> Samba tests).
Sorry, but this does not pass the any test of consistency as a review.
There is only one test in samba using it and _in the same directory_ as
the submitted test there is only one written in python, which itself
does not use that class.  The submitted test is modeled after that one.

If there is missing policy documentation, then fix it.  If the
documentation is there, point to it in the review.

Turning away test code where there was none calls for a higher level of
review and feedback than this.

Jim

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
On Tue, 2017-08-01 at 07:40 -0400, Jim McDonough wrote:

> On 07/31/2017 03:34 PM, Andrew Bartlett via samba-technical wrote:
> > On Mon, 2017-07-31 at 13:51 +0100, Noel Power via samba-technical
> > wrote:
> > > Please find a new version of the patchset (with tests re-written in python)
> > > Noel
> >
> > Thanks for writing these tests.  It would have been better however if
> > the tests used the standard test classes like BlackboxTestCase and the
> > assertions that go with it.  
> >
> > I know it will be a pile of pain to re-work the test (again!), but the
> > consistency really helps as others will copy and paste from the first
> > example they find.
> >
> > See for example python/samba/tests/blackbox/ndrdump.py
> >
> > (We used to have more of those, but SambaToolCmdTest took over for
> > Samba tests).
>
> Sorry, but this does not pass the any test of consistency as a review.
> There is only one test in samba using it and _in the same directory_ as
> the submitted test there is only one written in python, which itself
> does not use that class.  

Most Samba python tests use a test class derived from TestClass.

A large number of good examples can be found under:
python/samba/tests

While there are few blackbox tests written that way (most are written
in shell), there is already a good example that I pointed at, being
python/samba/tests/blackbox/ndrdump.py

The reason I raised my concerns is that the test looks nothing like the
majority of other python tests in Samba.  

Can you point me at the test it looks like?  

> The submitted test is modeled after that one.

Which one is that?  I couldn't find anything in that directory other
than the wbinfo test, which is a python driver inside a shell script as
a subunit wrapper, and it also looks nothing like this test.

unittest.TestCase is the standard Python unit testing framework, and we
use that in Samba extensively.

> If there is missing policy documentation, then fix it.  If the
> documentation is there, point to it in the review.
>
> Turning away test code where there was none calls for a higher level of
> review and feedback than this.

I hope the above is helpful.  I can't at this time fix our full
documentation deficit, but I have added:

https://wiki.samba.org/index.php/Writing_Python_Tests

I realise this proposed patch meets metze's request that the language
be changed to python (or shell), but sadly that is all it does.  That
is why I need to say:

Please model the python based test on our existing test framework and
usage patterns.  Unfortunately changing only the language is not
enough.

Thanks,

Andrew Bartlett
--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Expanded wiki pages on adding tests, preference for python.

Samba - samba-technical mailing list
On Wed, 2017-08-02 at 08:28 +1200, Andrew Bartlett via samba-technical
wrote:
>
> I hope the above is helpful.  I can't at this time fix our full
> documentation deficit, but I have added:
>
> https://wiki.samba.org/index.php/Writing_Python_Tests

I've expanded a number of pages under:

https://wiki.samba.org/index.php/Developer_Documentation#Debugging_.2F_Testing

I hope this helps clarify things for new developers.  I've explained
there that we don't want any more Perl tests, and suggested Python as
the best first choice for new tests.  

Established developers will often have good reasons why tests should be
in a particular language, but I want to give new developers a push into
a particular direction[1].

Thanks,

Andrew Bartlett

[1] My first task in Samba was to write pure posix SH for the build
farm.  I therefore can't suggest that to new developers as a default
option :-)

--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Expanded wiki pages on adding tests, preference for python.

Samba - samba-technical mailing list
On 08/02/2017 02:45 AM, Andrew Bartlett wrote:

> On Wed, 2017-08-02 at 08:28 +1200, Andrew Bartlett via samba-technical
> wrote:
>>
>> I hope the above is helpful.  I can't at this time fix our full
>> documentation deficit, but I have added:
>>
>> https://wiki.samba.org/index.php/Writing_Python_Tests
>
> I've expanded a number of pages under:
>
> https://wiki.samba.org/index.php/Developer_Documentation#Debugging_.2F_Testing
>
> I hope this helps clarify things for new developers.  I've explained
> there that we don't want any more Perl tests, and suggested Python as
> the best first choice for new tests.  
Thanks, Andrew.  This is definitely quite helpful.

Jim


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
And another version.....

On 31/07/17 20:34, Andrew Bartlett wrote:

> On Mon, 2017-07-31 at 13:51 +0100, Noel Power via samba-technical
> wrote:
>> Please find a new version of the patchset (with tests re-written in python)
>> Noel
> Thanks for writing these tests.  It would have been better however if
> the tests used the standard test classes like BlackboxTestCase and the
> assertions that go with it.  
>
> I know it will be a pile of pain to re-work the test (again!), but the
> consistency really helps as others will copy and paste from the first
> example they find.
>
> See for example python/samba/tests/blackbox/ndrdump.py
>
> (We used to have more of those, but SambaToolCmdTest took over for
> Samba tests).
>
> Sorry,
>
> Andrew Bartlett


smbcacls_with_pytest#2.patch (101K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
I have only made a shallow review of the code (will really try to make a
proper review but I have a lot on my plate now), and have some general
comments / suggestions:

1. The switch name. In principle, the --propagate-inheritance feature
propagates changes made to an ACL, so it doesn't propagate just
inheritance information, it propagates ACEs. Maybe just --propagate?

2. Usually we "do what Windows does(tm)". However in this case it seems
like "Windows" is not the Windows OS but rather Windows Explorer. I
think the documentation and / or commit message should make it clear
what the reference implementation (or other reference) is, in case there
are bug reports down the road.

3. Assuming this *is* a Windows Explorer look-alike, Windows
Explorer pops up a message if it fails to set the ACL of a file,
allowing the user to continue or abort. IMHO that would be useful here,
because changing a large tree without the option to continue would be
difficult. The program can output messages on files which failed.

4. In the docs, there's a modification saying the flags serve smbcacls -
well they serve the SMB server too when creating new files and folders
(I think they are an artifact of the NTFS file system, and Samba
emulates that behavior).

Thanks,
Uri.

On 08/03/2017 02:41 PM, Noel Power via samba-technical wrote:

> And another version.....
>
> On 31/07/17 20:34, Andrew Bartlett wrote:
>> On Mon, 2017-07-31 at 13:51 +0100, Noel Power via samba-technical
>> wrote:
>>> Please find a new version of the patchset (with tests re-written in python)
>>> Noel
>> Thanks for writing these tests.  It would have been better however if
>> the tests used the standard test classes like BlackboxTestCase and the
>> assertions that go with it.  
>>
>> I know it will be a pile of pain to re-work the test (again!), but the
>> consistency really helps as others will copy and paste from the first
>> example they find.
>>
>> See for example python/samba/tests/blackbox/ndrdump.py
>>
>> (We used to have more of those, but SambaToolCmdTest took over for
>> Samba tests).
>>
>> Sorry,
>>
>> Andrew Bartlett
>
>


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
Hi Uri
On 03/08/17 12:53, Uri Simchoni wrote:
> I have only made a shallow review of the code (will really try to make a
> proper review but I have a lot on my plate now), and have some general
> comments / suggestions:
thanks for looking!
>
> 1. The switch name. In principle, the --propagate-inheritance feature
> propagates changes made to an ACL, so it doesn't propagate just
> inheritance information, it propagates ACEs. Maybe just --propagate?
well it propagates ACES(s) that's true but only in the context of the
rules of inheritance, --propagate on it's own to me suggests something
like a recursive operation. I would have been happy with just
--inheritance (or something like that) but of course everyone has an
opinion :-) and I was convinced by someone else at some stage that
'--propagate-inheritance' If there is some sort of consensus I'll
happily change it to something else, meanwhile I'd be happy to leave it
as is
>
> 2. Usually we "do what Windows does(tm)". However in this case it seems
> like "Windows" is not the Windows OS but rather Windows Explorer. I
> think the documentation and / or commit message should make it clear
> what the reference implementation (or other reference) is, in case there
> are bug reports down the road.
ok, I can do that. But.. note that smbcacls isn't trying to behave like
Windows Explorer but rather obey what is described here
https://msdn.microsoft.com/en-us/library/windows/desktop/aa374924(v=vs.85).aspx
However if you are looking for something to compare against icacls.exe
is probably the closest thing to smbcacls that is around (and what I
mostly referenced where possible) icacls has 'grant', 'grant:r' & 'deny
that loosely perform similar operations to smbcacls --add, --set &
--delete respectively. Note: icacls.exe automatically propagates
inheritable aces according to the rules above
>
> 3. Assuming this *is* a Windows Explorer look-alike, Windows
> Explorer pops up a message if it fails to set the ACL of a file,
> allowing the user to continue or abort. IMHO that would be useful here,
> because changing a large tree without the option to continue would be
> difficult. The program can output messages on files which failed.
see above, smbcacls like icacls is a power tool, you can shoot yourself
in the foot royally, I can't recall whether an icacls failure is
reported and it continues on or not. My gut feeling is we should do as
icacls does (where possible)
I'll look into what it does
>
> 4. In the docs, there's a modification saying the flags serve smbcacls -
> well they serve the SMB server too when creating new files and folders
> (I think they are an artifact of the NTFS file system, and Samba
> emulates that behavior).
Not sure what modification you mean, can you elaborate

Thanks again
Noel

>
> Thanks,
> Uri.
>
> On 08/03/2017 02:41 PM, Noel Power via samba-technical wrote:
>> And another version.....
>>
>> On 31/07/17 20:34, Andrew Bartlett wrote:
>>> On Mon, 2017-07-31 at 13:51 +0100, Noel Power via samba-technical
>>> wrote:
>>>> Please find a new version of the patchset (with tests re-written in python)
>>>> Noel
>>> Thanks for writing these tests.  It would have been better however if
>>> the tests used the standard test classes like BlackboxTestCase and the
>>> assertions that go with it.  
>>>
>>> I know it will be a pile of pain to re-work the test (again!), but the
>>> consistency really helps as others will copy and paste from the first
>>> example they find.
>>>
>>> See for example python/samba/tests/blackbox/ndrdump.py
>>>
>>> (We used to have more of those, but SambaToolCmdTest took over for
>>> Samba tests).
>>>
>>> Sorry,
>>>
>>> Andrew Bartlett
>>
>


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
On 03/08/17 14:09, Noel Power wrote:
>> 3. Assuming this *is* a Windows Explorer look-alike, Windows >> Explorer pops up a message if it fails to set the ACL of a file, >>
allowing the user to continue or abort. IMHO that would be useful >>
here, because changing a large tree without the option to continue >>
would be difficult. The program can output messages on files which >>
failed. > see above, smbcacls like icacls is a power tool, you can shoot
> yourself in the foot royally, I can't recall whether an icacls >
failure is reported and it continues on or not. My gut feeling is we >
should do as icacls does (where possible) I'll look into what it > does
What appears to happen is that although inheritable aces are propagated
to all child containers and files icalcs only reports that it is
processing a single file. Any access errors (or failure to apply
inhertitable aces) appear to be squashed and icacls.exe continues best
effort. smbcacls doesn't behave like this, I believe it should (good
call to make me check this behavour)

Here is a sample run from windows.

icacls is operating on the top level 'oi_dir' directory, however there
is a lower level directory 'oi_dir/other' that is inaccessible

  +-test_dir/
    +-oi_dir/ <= (not accessible)
    | +-file-1
    | +-pfile.txt
    | +-nested/
    |   +-file-2
    | +-other/

Note: icacls.exe /T switch effectively operates recursively

  c:\Temp>icacls oi_dir/ /grant Administrator:(OI)(CI)(R)
  processed file: oi_dir/
  Successfully processed 1 files; Failed processing 0 files

  c:\Temp>icacls oi_dir /T
  oi_dir TESTDOMAIN1\Administrator:(OI)(CI)(R)
         TESTDOMAIN1\Administrator:(I)(OI)(CI)(F)
         BUILTIN\Administrators:(I)(OI)(CI)(F)
         Everyone:(I)(OI)(CI)(F)

  oi_dir\file-1 TESTDOMAIN1\Administrator:(I)(R)
                TESTDOMAIN1\Administrator:(I)(F)
                BUILTIN\Administrators:(I)(F)
                Everyone:(I)(F)

  oi_dir\nested TESTDOMAIN1\Administrator:(I)(OI)(CI)(R)
                TESTDOMAIN1\Administrator:(I)(OI)(CI)(F)
                BUILTIN\Administrators:(I)(OI)(CI)(F)
                Everyone:(I)(OI)(CI)(F)

  oi_dir\other: Access is denied.
  Successfully processed 3 files; Failed processing 1 files

Note:: Failure above prevented icacls from displaying the ACL for
pfile.txt (but it was modified with the propagated ace(s))

  c:\Temp>icacls oi_dir/pfile.txt /T
  oi_dir/pfile.txt TESTDOMAIN1\Administrator:(I)(R)
                   TESTDOMAIN1\Administrator:(I)(F)
                   BUILTIN\Administrators:(I)(F)
                   Everyone:(I)(F)

  oi_dir\other\*: Access is denied.
  Successfully processed 1 files; Failed processing 1 files

  c:\Temp>icacls oi_dir/other
  oi_dir/other: Access is denied.
  Successfully processed 0 files; Failed processing 1 files

  c:\Temp>icacls oi_dir/other /grant  Administrator:(OI)(F)
  oi_dir/other: Access is denied.
  Successfully processed 0 files; Failed processing 1 files


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
On 03/08/17 17:07, Noel Power wrote:

>
> icacls is operating on the top level 'oi_dir' directory, however there
> is a lower level directory 'oi_dir/other' that is inaccessible
>
>   +-test_dir/
>     +-oi_dir/ <= (not accessible)
>     | +-file-1
>     | +-pfile.txt
>     | +-nested/
>     |   +-file-2
>     | +-other/

diagram is wrong of course

  +-test_dir/
    +-oi_dir/
    | +-file-1
    | +-pfile.txt
    | +-nested/
    |   +-file-2
    | +-other/ <= (not accessible)


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On 08/03/2017 04:09 PM, Noel Power wrote:
> Hi Uri
> On 03/08/17 12:53, Uri Simchoni wrote:
>> 4. In the docs, there's a modification saying the flags serve smbcacls -
>> well they serve the SMB server too when creating new files and folders
>> (I think they are an artifact of the NTFS file system, and Samba
>> emulates that behavior).
> Not sure what modification you mean, can you elaborate
>

Sorry for not being specific enough. I was referring to patch 3/4:

> @@ -229,18 +234,22 @@ ACL:&lt;sid or name&gt;:&lt;type&gt;/&lt;flags&gt;/&lt;mask&gt;
>         determine the type of access granted to the SID. </para>
>
>         <para>The type can be either ALLOWED or DENIED to allow/deny access
> -       to the SID. The flags values are generally zero for file ACEs and
> -       either 9 or 2 for directory ACEs.  Some common flags are: </para>
> +       to the SID.</para>
> +
> +       <para>The flags field defines how the ACE should be considered when
> +       performing inheritance. <command>smbcacls</command> uses these flags
> +       when run with <parameter>--propagate-inheritance</parameter>.</para>

Everything written about the flags is true, and the previous text is
admittedly bad. However, IMHO, we should add the role those flags play
when later creating new files, not only when modifying an ACL. When
settings the ACL of an empty dir via smbcacls, one should carefully
consider the flags, even though there's nothing to propagate.

Thanks,
Uri.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Thu, 2017-08-03 at 12:41 +0100, Noel Power via samba-technical
wrote:
> And another version.....

Thanks.  This is getting much closer, and looks much more like our
other tests.

Can you please look at source4/dsdb/tests/python/ldap.py for an example
of how to do the command-line parsing and credentials handling?

It is important that we don't introduce new command-line syntax, the
rest of Samba uses -U, not -u for example.

Thanks,

Andrew Bartlett

--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT  
https://catalyst.net.nz/services/samba





Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
On 04/08/17 04:45, Andrew Bartlett wrote:

> On Thu, 2017-08-03 at 12:41 +0100, Noel Power via samba-technical
> wrote:
>> And another version.....
> Thanks.  This is getting much closer, and looks much more like our
> other tests.
>
> Can you please look at source4/dsdb/tests/python/ldap.py for an example
> of how to do the command-line parsing and credentials handling?
>
> It is important that we don't introduce new command-line syntax, the
> rest of Samba uses -U, not -u for example.
sorry that credential handling does not seem to work, it seems to expect
s4 code/environment as it call

raceback (most recent call last):
  File "/usr/lib64/python2.7/runpy.py", line 174, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib64/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/data/samba-back/bin/python/samba/subunit/run.py", line 697, in
<module>
    TestProgram(module=None, argv=sys.argv, stdout=sys.stdout)
  File "/data/samba-back/bin/python/samba/subunit/run.py", line 535, in
__init__
    self.parseArgs(argv)
  File "/data/samba-back/bin/python/samba/subunit/run.py", line 594, in
parseArgs
    self.createTests()
  File "/data/samba-back/bin/python/samba/subunit/run.py", line 603, in
createTests
    self.module)
  File "/usr/lib64/python2.7/unittest/loader.py", line 130, in
loadTestsFromNames
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/usr/lib64/python2.7/unittest/loader.py", line 91, in
loadTestsFromName
    module = __import__('.'.join(parts_copy))
  File "/data/samba-back/bin/python/samba/tests/blackbox/smbcacls.py",
line 49, in <module>
    creds = credopts.get_credentials(lp)
  File "/data/samba-back/bin/python/samba/getopt.py", line 209, in
get_credentials
    self.creds.set_machine_account(lp)
samba.NTSTATUSError: (-1073741606, 'Configuration information could not
be read from the domain controller, either because the machine is
unavailable or access has been denied.')

it seems get_credentials calls credentials_set_machine_account, the
associated required tdb(s) and configuration etc. seem too restrictive
for my requirements so I've removed the convenience main function to
avoid this mess

On 03/08/17 20:46, Uri Simchoni wrote:

> Sorry for not being specific enough. I was referring to patch 3/4:
>
>> @@ -229,18 +234,22 @@ ACL:&lt;sid or name&gt;:&lt;type&gt;/&lt;flags&gt;/&lt;mask&gt;
>>         determine the type of access granted to the SID. </para>
>>
>>         <para>The type can be either ALLOWED or DENIED to allow/deny access
>> -       to the SID. The flags values are generally zero for file ACEs and
>> -       either 9 or 2 for directory ACEs.  Some common flags are: </para>
>> +       to the SID.</para>
>> +
>> +       <para>The flags field defines how the ACE should be considered when
>> +       performing inheritance. <command>smbcacls</command> uses these flags
>> +       when run with <parameter>--propagate-inheritance</parameter>.</para>
> Everything written about the flags is true, and the previous text is
> admittedly bad. However, IMHO, we should add the role those flags play
> when later creating new files, not only when modifying an ACL. When
> settings the ACL of an empty dir via smbcacls, one should carefully
> consider the flags, even though there's nothing to propagate.
Uri I've added a small para to additionally mention file/directory
creation, not sure if it is enough, I'd happily take on board any more
suggestions re. wording and verbage for that. Thing to note though is
that just because there are inheritance related ACE(s) on the parent
directory doesn't mean that whatever creates the file has to honour it.
E.g. cygwin on windows happily breaks the inheritance and creates files
with explicit ACE(s). Breaking inheritance is perfectly legal, there is
even a button for doing it from the explorer file/dir properties dialog

Noel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Looks like we do not have self-tests for smbcacls

Samba - samba-technical mailing list
On Fri, 2017-08-04 at 17:49 +0100, Noel Power via samba-technical
wrote:
> sorry that credential handling does not seem to work, it seems to expect
> s4 code/environment as it call

I'm not sure exactly what is going on as I can't see the code, but that
looks like a fallback to the machine account.  Did you catch that this
code uses -U not -u?

Andrew Bartlett

--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Loading...