On (17/10/16 14:35), Fabiano FidĂȘncio wrote:
On Mon, Oct 17, 2016 at 11:46 AM, Lukas Slebodnik
<lslebodn(a)redhat.com> wrote:
> On (30/09/16 16:55), fidencio wrote:
>> URL:
https://github.com/SSSD/sssd/pull/33
>>Author: fidencio
>> Title: #33: SECRETS: Some small misc fixes + fixing #3168
>>Action: synchronized
>>
>>To pull the PR as Git branch:
>>git remote add ghsssd
https://github.com/SSSD/sssd
>>git fetch ghsssd pull/33/head:pr33
>>git checkout pr33
>
> >From 06a0a81193d6bbe3a0932c8b584433f3cc13fa51 Mon Sep 17 00:00:00 2001
>>From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio(a)redhat.com>
>>Date: Sun, 25 Sep 2016 20:49:16 +0200
>>Subject: [PATCH 1/6] CONFIG: Add secrets responder to the allowed sections
>>MIME-Version: 1.0
>>Content-Type: text/plain; charset=UTF-8
>>Content-Transfer-Encoding: 8bit
>>
>>The regular expression used is quite specific for the two cases we
>>support:
>>- [secrets]
>>- [secrets/users/$uid]
>>
>>It could be done a bit more generic, but the way it's right now it can
>>easily catch errors like: [secrets/usrs/$uid] or [secrets/].
>>
>>Related:
>>https://fedorahosted.org/sssd/ticket/3207
>>
>>Signed-off-by: Fabiano FidĂȘncio <fidencio(a)redhat.com>
>>---
>> src/config/cfg_rules.ini | 1 +
>> 1 file changed, 1 insertion(+)
>>
>>diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
>>index 01be0c6..023ceac 100644
>>--- a/src/config/cfg_rules.ini
>>+++ b/src/config/cfg_rules.ini
>>@@ -8,6 +8,7 @@ section = autofs
>> section = ssh
>> section = pac
>> section = ifp
>>+section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
>> section_re = ^domain/.*$
>
> Is it expected that section the name "secrets/users/"
> is allowed.
I don't think so.
I'll answer your questions based on the my understanding of the
conversation I had with Jakub on the #sssd channel.
Jakub, Simo, please, feel free to jump in and correct me if I'm
mistaken in any point.
>
> Which of following section should be allowed?
>
> sh# cat /etc/sssd/conf.d/10_secrets.conf
> [secrets
> description = temp
Not allowed, but [secrets] is allowed.
This one was a typo on my side.
>
> [secrets/users]
> description = temp
>
Shouldn't be allowed.
> [secrets/users/]
> description = temp
>
Shouldn't be allowed.
But it is allowed.
Following change should fix it.
I didn't tested it.
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index b6316be..0a654ff 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -8,7 +8,7 @@ section = autofs
section = ssh
section = pac
section = ifp
-section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
+section_re = ^secrets\(/users/[0-9]\+\)\?$
section_re = ^domain/.*$
[rule/allowed_sssd_options]
> [secrets/users/$uid]
> description = temp
>
Shouldn't be allowed.
yes, it's denied.
> [secrets/users/0]
> description = temp
>
Should be allowed.
OK, I was not sure about root UID.
> [secrets/users/1]
> description = temp
Should be allowed.
>
> [secrets/users/1000]
> description = temp
Should be allowed.
>
> LS
Is some of these cases breaking to you?
If yes, please, let me know and I'll provide a follow up patch fixing the issue.
I think we shoudl also split following rule
"[rule/allowed_sec_options]"
I do not think that following options are read from
sections "secrets/users/.*"
option = timeout
option = debug
option = debug_level
option = debug_timestamps
option = debug_microseconds
option = debug_to_files
option = command
option = reconnection_retries
option = fd_limit
option = client_idle_timeout
option = description
Fabiano,
Could you preapre a patch?
LS