On (18/01/16 13:05), Pavel Březina wrote:
On 01/18/2016 12:23 PM, Pavel Březina wrote:
>On 01/18/2016 10:54 AM, Lukas Slebodnik wrote:
>>On (18/01/16 10:18), Jakub Hrozek wrote:
>>>On Mon, Jan 18, 2016 at 10:03:30AM +0100, Pavel Březina wrote:
>>>>On 01/15/2016 04:31 PM, Lukas Slebodnik wrote:
>>>>>On (15/01/16 15:06), Sumit Bose wrote:
>>>>>>On Fri, Jan 15, 2016 at 02:26:48PM +0100, Lukas Slebodnik wrote:
>>>>>>>On (15/01/16 10:21), Pavel Březina wrote:
>>>>>>>>On 01/14/2016 06:37 PM, Sumit Bose wrote:
>>>>>>>>>On Thu, Jan 14, 2016 at 01:55:39PM +0100, Pavel
Březina wrote:
>>>>>>>>>>On 01/13/2016 05:12 PM, Sumit Bose wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>@@ -1018,7 +1019,6 @@
rules_iterator(hash_entry_t *item,
>>>>>>>>>>>>
>>>>>>>>>>>> if (ctx == NULL) {
>>>>>>>>>>>> DEBUG(SSSDBG_CRIT_FAILURE,
"Bug: ctx is NULL\n");
>>>>>>>>>>>>- ctx->ret = ERR_INTERNAL;
>>>>>>>>>>>> return false;
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>>@@ -1066,7 +1066,6 @@
cmdgroups_iterator(hash_entry_t *item,
>>>>>>>>>>>>
>>>>>>>>>>>> if (ctx == NULL) {
>>>>>>>>>>>> DEBUG(SSSDBG_CRIT_FAILURE,
"Bug: ctx is NULL\n");
>>>>>>>>>>>>- ctx->ret = ERR_INTERNAL;
>>>>>>>>>>>> return false;
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>it looks like the last 2 hunks are not
available in the
>>>>>>>>>>>latest tar ball.
>>>>>>>>>>
>>>>>>>>>>Sorry about that. Added.
>>>>>>>>>>
>>>>>>>>>>>and there are some trailing whitespaces in
patch 9 at line 41.
>>>>>>>>>>
>>>>>>>>>>Fixed by git.
>>>>>>>>>>
>>>>>>>>>>I also attached two more patches then converts usn
into number
>>>>>>>>>>and then use
>>>>>>>>>>in filter entryUSN >= (current + 1) instead of
(entryUSN >=
>>>>>>>>>>cur) &&
>>>>>>>>>>(entryUSN != cur) as per Thierry request.
>>>>>>>>>>
>>>>>>>>>>It could probably be squashed and replace some
previous
>>>>>>>>>>patches but I think
>>>>>>>>>>sending it as new patches is easier for you to
review, given
>>>>>>>>>>the previous
>>>>>>>>>>patches are basically acked.
>>>>>>>>>
>>>>>>>>>>+ errno = 0;
>>>>>>>>>> usn_number = strtoul(usn, &endptr, 10);
>>>>>>>>>>- if ((endptr == NULL || (*endptr ==
'\0' && endptr != usn))
>>>>>>>>>>- && (usn_number >
srv_opts->last_usn)) {
>>>>>>>>>>- srv_opts->last_usn = usn_number;
>>>>>>>>>>+ if (endptr != NULL && *endptr !=
'\0') {
>>>>>>>>>
>>>>>>>>>Currently an empty string in usn would result in
usn_number==0,
>>>>>>>>>I wonder
>>>>>>>>>if this is valid or if it would be better to error out
in this
>>>>>>>>>case?
>>>>>>>>
>>>>>>>>Zero value will not change anything since we always
compare if
>>>>>>>>the current
>>>>>>>>usn value is lower than the new one, we switch them only
in such
>>>>>>>>case. So I
>>>>>>>>think it is not necessary to error out in this case since
>>>>>>>>strtoul succeeds.
>>>>>>>>
>>>>>>>>>
>>>>>>>>>Otherwise all looks good, CI and Coverity test are
currently
>>>>>>>>>running.
>>>>>>>>>
>>>>>>>>>bye,
>>>>>>>>>Sumit
>>>>>>>>>
>>>>>>>>>>+ DEBUG(SSSDBG_MINOR_FAILURE, "Unable
to convert USN
>>>>>>>>>>%s\n", usn);
>>>>>>>>>>+ return;
>>>>>>>>>>+ } else if (errno != 0) {
>>>>>>>>>>+ ret = errno;
>>>>>>>>>>+ DEBUG(SSSDBG_MINOR_FAILURE, "Unable
to convert USN %s
>>>>>>>>>>[%d]:
>>>>>>>>>>%s\n",
>>>>>>>>>>+ usn, ret, sss_strerror(ret));
>>>>>>>>>>+ return;
>>>>>>>>>> }
>>>>>>>
>>>>>>>There are 4 failed test from freeipa sudo test suite.
>>>>>>>
>>>>>>>test-integration-test-sudo-TestSudo-test-sudo-rule-restricted-to-one-hostmask
>>>>>>>
>>>>>>>test-integration-test-sudo-TestSudo-test-sudo-rule-restricted-to-running-as-single-local-user
>>>>>>>
>>>>>>>test-integration-test-sudo-TestSudo-test-sudo-rule-restricted-to-run-as-users-from-local-group
>>>>>>>
>>>>>>>test-integration-test-sudo-TestSudo-test-sudo-rule-restricted-to-running-as-single-local-group
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>=================================== FAILURES
>>>>>>>===================================
>>>>>>>______________
TestSudo.test_sudo_rule_restricted_to_one_hostmask
>>>>>>>______________
>>>>>>>
>>>>>>>self = <ipatests.test_integration.test_sudo.TestSudo object
at
>>>>>>>0x7fe0d7ace8d0>
>>>>>>>
>>>>>>> def test_sudo_rule_restricted_to_one_hostmask(self):
>>>>>>> if self.__class__.skip_hostmask_based:
>>>>>>> raise pytest.skip("Hostmask could not be
detected")
>>>>>>>
>>>>>>> result1 =
self.list_sudo_commands("testuser1")
>>>>>>>
assert "(ALL : ALL) NOPASSWD:
ALL" in result1.stdout_text
>>>>>>>E assert
'(ALL : ALL) NOPASSWD: ALL' in ''
>>>>>>>E + where '' =
<pytest_multihost.transport.SSHCommand
>>>>>>>object at 0x7fe0d7acee10>.stdout_text
>>>>>>>
>>>>>>>test_integration/test_sudo.py:295: AssertionError
>>>>>>>______
>>>>>>>TestSudo.test_sudo_rule_restricted_to_running_as_single_local_user
______
>>>>>>>
>>>>>>>
>>>>>>>self = <ipatests.test_integration.test_sudo.TestSudo object
at
>>>>>>>0x7fe0d79de990>
>>>>>>>
>>>>>>> def
>>>>>>>test_sudo_rule_restricted_to_running_as_single_local_user(self):
>>>>>>> result1 =
self.list_sudo_commands("testuser1",
>>>>>>>verbose=True)
>>>>>>>
assert "RunAsUsers:
localuser" in result1.stdout_text
>>>>>>
>>>>>>There are just too many options. I tested RunAsUsers and
RunAsUsers
>>>>>>but only with IPA users and groups. I looks like the handling for
the
>>>>>>ipasudorunasextuser and ipasudorunasextgroup attributes are
missing.
>>>>>>
>>>>>That's the purpose of automated tests :-)
>>>>>To cover all use-case.
>>>>
>>>>These use cases are omitted by design.
>>>
>>>We might still need to at least document this change. But with a
>>>recent IPA
>>>server, I see:
>>> [jhrozek@unidirect] sssd $ [(review)] ipa sudorule-mod
>>>--externaluser=jhrozek readfiles
>>> ipa: ERROR: invalid 'externaluser': this option has been
deprecated.
>>> [jhrozek@unidirect] sssd $ [(review)] ipa sudorule-mod
>>>--runasexternalgroup=jhrozek readfiles
>>> ipa: ERROR: invalid 'runasexternalgroup': this option has been
>>>deprecated.
>>> [jhrozek@unidirect] sssd $ [(review)] ipa sudorule-mod
>>>--runasexternaluser=jhrozek readfiles
>>> ipa: ERROR: invalid 'runasexternaluser': this option has been
>>>deprecated.
>>>
>>We might ask whether Dreamworks need this feature.
>>Otherwise these patches will not help them.
>
>We talked with Jakub about it and decided that we will add support of
>ipaSudoRunAsExt* attributes. So with this patch set only hostmask test
>should fail, we will not add support for hostmask unless requested.
Even if we ignore hostmask issue there is stil issue with local user/group
test-integration-test-sudo-TestSudo-test-sudo-rule-restricted-to-one-hostmask
test-integration-test-sudo-TestSudo-test-sudo-rule-restricted-to-running-as-single-local-user
test-integration-test-sudo-TestSudo-test-sudo-rule-restricted-to-run-as-users-from-local-group
Full error:
=================================== FAILURES ===================================
______________ TestSudo.test_sudo_rule_restricted_to_one_hostmask ______________
self = <ipatests.test_integration.test_sudo.TestSudo object at 0x7f9d8b399c10>
def test_sudo_rule_restricted_to_one_hostmask(self):
if self.__class__.skip_hostmask_based:
raise pytest.skip("Hostmask could not be detected")
result1 = self.list_sudo_commands("testuser1")
assert "(ALL : ALL) NOPASSWD: ALL" in
result1.stdout_text
E assert '(ALL : ALL) NOPASSWD: ALL' in
''
E + where '' = <pytest_multihost.transport.SSHCommand object at
0x7f9d8b37c910>.stdout_text
test_integration/test_sudo.py:295: AssertionError
______ TestSudo.test_sudo_rule_restricted_to_running_as_single_local_user ______
self = <ipatests.test_integration.test_sudo.TestSudo object at 0x7f9d88f0a990>
def test_sudo_rule_restricted_to_running_as_single_local_user(self):
result1 = self.list_sudo_commands("testuser1", verbose=True)
assert "RunAsUsers: localuser" in result1.stdout_text
E assert 'RunAsUsers: localuser' in 'Matching Defaults entries for
testuser1 on nec-em17:\n !visiblepw, env_reset, env_keep="COLORS DISPLAY HOSTNAME
HI... may run the following commands on nec-em17:\n RunAsUsers: root\n Options:
!authenticate\n Commands:\n\tALL\n'
E + where 'Matching Defaults entries for testuser1 on nec-em17:\n
!visiblepw, env_reset, env_keep="COLORS DISPLAY HOSTNAME HI... may run the following
commands on nec-em17:\n RunAsUsers: root\n Options: !authenticate\n
Commands:\n\tALL\n' = <pytest_multihost.transport.SSHCommand object at
0x7f9d88f0a490>.stdout_text
test_integration/test_sudo.py:418: AssertionError
_____ TestSudo.test_sudo_rule_restricted_to_run_as_users_from_local_group ______
self = <ipatests.test_integration.test_sudo.TestSudo object at 0x7f9d8b37c690>
def test_sudo_rule_restricted_to_run_as_users_from_local_group(self):
result1 = self.list_sudo_commands("testuser1", verbose=True)
assert "RunAsUsers: %localgroup" in
result1.stdout_text
E assert 'RunAsUsers: %localgroup' in
'Matching Defaults entries for testuser1 on nec-em17:\n !visiblepw, env_reset,
env_keep="COLORS DISPLAY HOSTNAME HI... may run the following commands on nec-em17:\n
RunAsUsers: root\n Options: !authenticate\n Commands:\n\tALL\n'
E + where 'Matching Defaults entries for testuser1 on nec-em17:\n
!visiblepw, env_reset, env_keep="COLORS DISPLAY HOSTNAME HI... may run the following
commands on nec-em17:\n RunAsUsers: root\n Options: !authenticate\n
Commands:\n\tALL\n' = <pytest_multihost.transport.SSHCommand object at
0x7f9d8b487910>.stdout_text
test_integration/test_sudo.py:460: AssertionError
_____ TestSudo.test_sudo_rule_restricted_to_running_as_single_local_group ______
self = <ipatests.test_integration.test_sudo.TestSudo object at 0x7f9d89109d10>
def test_sudo_rule_restricted_to_running_as_single_local_group(self):
result1 = self.list_sudo_commands("testuser1", verbose=True)
assert "RunAsUsers: root" in result1.stdout_text
assert "RunAsGroups: localgroup" in
result1.stdout_text
E assert 'RunAsGroups: localgroup' in
'Matching Defaults entries for testuser1 on nec-em17:\n !visiblepw, env_reset,
env_keep="COLORS DISPLAY HOSTNAME HI... may run the following commands on nec-em17:\n
RunAsUsers: root\n Options: !authenticate\n Commands:\n\tALL\n'
E + where 'Matching Defaults entries for testuser1 on nec-em17:\n
!visiblepw, env_reset, env_keep="COLORS DISPLAY HOSTNAME HI... may run the following
commands on nec-em17:\n RunAsUsers: root\n Options: !authenticate\n
Commands:\n\tALL\n' = <pytest_multihost.transport.SSHCommand object at
0x7f9d89109ed0>.stdout_text
test_integration/test_sudo.py:503: AssertionError
==================== 4 failed, 71 passed in 736.12 seconds =====================
LS