[PATCH] Port of samba.security Python module

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

[PATCH] Port of samba.security Python module

Samba - samba-technical mailing list
Hello.

I've ported another Python C extension to Python 3 compatible form.
AFAIK there is only one usage of samba.security - found in
python/samba/netcmd/gpo.py on line 407. Usage of this module looks very
specific and complicated because it needs token from user session and
unpacked NDR objects as arguments and samba.security is not used widely
so I am not sure how to write tests for this function. Any pointers
would be appreciated..

Have a nice day.

Lumír


0001-python-Port-samba.security-to-Python-3-compatible-fo.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Port of samba.security Python module

Samba - samba-technical mailing list
Hello.

Is there any problem with this patch? I've tried to rebase it to the
latest master and there is no conflict.

If the problem is that the module is unstested, could you please help me
to write some test?

Thank you and have a nice day.

Lumír


On 05/22/2017 04:24 PM, Lumir Balhar via samba-technical wrote:

> Hello.
>
> I've ported another Python C extension to Python 3 compatible form.
> AFAIK there is only one usage of samba.security - found in
> python/samba/netcmd/gpo.py on line 407. Usage of this module looks
> very specific and complicated because it needs token from user session
> and unpacked NDR objects as arguments and samba.security is not used
> widely so I am not sure how to write tests for this function. Any
> pointers would be appreciated..
>
> Have a nice day.
>
> Lumír
>


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

Re: [PATCH] Port of samba.security Python module

Samba - samba-technical mailing list
On Mon, 2017-08-07 at 16:48 +0200, Lumir Balhar via samba-technical
wrote:
> Hello.
>
> Is there any problem with this patch? I've tried to rebase it to the 
> latest master and there is no conflict.
>
> If the problem is that the module is unstested, could you please help
> me 
> to write some test?

We do always need more tests :-)

It doesn't look like a complex conversion however.  How about creating
an SD from SSDL, then testing it against a token (even an empty one).
Add that to the python/samba/tests/security.py testsuite.

That should confirm the C code still gets hit by the python code.

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: [PATCH] Port of samba.security Python module

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

> On Mon, 2017-08-07 at 16:48 +0200, Lumir Balhar via samba-technical
> wrote:
>> Hello.
>>
>> Is there any problem with this patch? I've tried to rebase it to the
>> latest master and there is no conflict.
>>
>> If the problem is that the module is unstested, could you please help
>> me
>> to write some test?
> We do always need more tests :-)
>
> It doesn't look like a complex conversion however.  How about creating
> an SD from SSDL, then testing it against a token (even an empty one).
> Add that to the python/samba/tests/security.py testsuite.
>
> That should confirm the C code still gets hit by the python code.
>
> Thanks,
>
> Andrew Bartlett
Hello.

Thank you for the hint. I've created tests as you suggested with an
empty token and same descriptor as other tests are using in
samba.test.security.

Unfortunately, I cannot use assertRaises() as a context manager for
testing exceptions because it is not supported in Python 2.6.

Please, let me know if this is ok.

Thank you and have a nice day.

Lumír

0003-python-Enable-execution-of-samba.tests.security-with.patch (1K) Download Attachment
0002-python-Add-tests-for-check_access-function-from-samb.patch (1K) Download Attachment
0001-python-Port-samba.security-to-Python-3-compatible-fo.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Port of samba.security Python module

Samba - samba-technical mailing list
Hi Lumir,

> +
> +class CheckAccessTests(samba.tests.TestCase):
> +
> +    def test_check_access(self):
> +        desc = security.descriptor.from_sddl("O:AOG:DAD:(A;;RPWPCCDCLCSWRCWDWOGA;;;S-1-0-0)", security.dom_sid("S-2-0-0"))
> +        token = security.token()
> +
> +        self.assertEqual(access_check(desc, token, 0), 0)
> +
> +        params = (
> +            (-1, -1073741727, 'A required privilege is not held by the client.'),
> +            (1, -1073741790, '{Access Denied} A process has requested access to an object but has not been granted those access rights.')
> +        )
Can you use string constants for the integer values?

I guess you can use
security.SEC_FLAG_SYSTEM_SECURITY/ntstatus.NT_STATUS_PRIVILEGE_NOT_HELD
and
security.SEC_STD_READ_CONTROL/ntstatus.NT_STATUS_ACCESS_DENIED

And I guess checking the status code is enough, we don't
need to assert on the error message.

metze




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

Re: [PATCH] Port of samba.security Python module

Samba - samba-technical mailing list
On 08/08/2017 10:47 AM, Stefan Metzmacher wrote:

> Hi Lumir,
>
>> +
>> +class CheckAccessTests(samba.tests.TestCase):
>> +
>> +    def test_check_access(self):
>> +        desc = security.descriptor.from_sddl("O:AOG:DAD:(A;;RPWPCCDCLCSWRCWDWOGA;;;S-1-0-0)", security.dom_sid("S-2-0-0"))
>> +        token = security.token()
>> +
>> +        self.assertEqual(access_check(desc, token, 0), 0)
>> +
>> +        params = (
>> +            (-1, -1073741727, 'A required privilege is not held by the client.'),
>> +            (1, -1073741790, '{Access Denied} A process has requested access to an object but has not been granted those access rights.')
>> +        )
> Can you use string constants for the integer values?
>
> I guess you can use
> security.SEC_FLAG_SYSTEM_SECURITY/ntstatus.NT_STATUS_PRIVILEGE_NOT_HELD
> and
> security.SEC_STD_READ_CONTROL/ntstatus.NT_STATUS_ACCESS_DENIED
>
> And I guess checking the status code is enough, we don't
> need to assert on the error message.
>
> metze
>
>
>
Thank you for the review. Yes, I can use constants instead of integers -
good idea - but samba.ntstatus module is not ready for Python 3 yet. It
is not a big problem because I am working on samba.ntstatus and
samba.werror modules right now so I am gonna send another patchset today
and when it will be merged I'll fix these test.

Thank you one more time and have a nice day.
Lumír

Loading...