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
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
