On (18/01/16 15:25), Lukas Slebodnik wrote:
>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
>=====================
>
I would like to appologize for confusion the latest patches
works fine for local{user,group}
There is only issue with hostmask but we do not pland to add support now.
+1
tested by: LS (I didn't read patches)
LS
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
