audit: Fix wrong task in comparison of session ID
Subsystems (2)
PATCH 1/1 audit: Fix wrong task in comparison of session ID
The audit_filter_rules() function in auditsc.c compared the session ID with the credentials of the current task, while it should use the credentials of the task given to audit_filter_rules() as a parameter (tsk).
GitHub issue:
https://github.com/linux-audit/audit-kernel/issues/82
--- kernel/auditsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/auditsc.c +1 -1
@@ -513,7 +513,7 @@ static int audit_filter_rules(struct task_struct *tsk, result =audit_gid_comparator (cred->fsgid, f->op, f->gid);break ;case AUDIT_SESSIONID : - sessionid =audit_get_sessionid (current); + sessionid =audit_get_sessionid (tsk); result =audit_comparator (sessionid, f->op, f->val);break ;case AUDIT_PERS :
Signature · 1 line
2.17.0
Quoted 11 lines · Ondrej Mosnacek
The
audit_filter_rules()function inauditsc.ccompared the session ID with the credentials of the current task, while it should use the credentials of the task given toaudit_filter_rules()as a parameter (tsk).GitHub issue:
https://github.com/linux-audit/audit-kernel/issues/82Fixes 8fae47705685 ("audit: add support for session ID user filter")Signed-off-by Ondrej Mosnacek <[email protected]>
Quoted 20 lines
--- kernel/auditsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)kernel/auditsc.c +1
-1@@ -513,7 +513,7 @@ static int audit_filter_rules(struct task_struct *tsk, result =audit_gid_comparator (cred->fsgid, f->op, f->gid);break ;case AUDIT_SESSIONID : - sessionid =audit_get_sessionid (current); + sessionid =audit_get_sessionid (tsk); result =audit_comparator (sessionid, f->op, f->val);break ;case AUDIT_PERS :Signature · 1 line
2.17.0
- RGB
Paul Moore
maintainerQuoted 8 lines · Ondrej Mosnacek
The
audit_filter_rules()function inauditsc.ccompared the session ID with the credentials of the current task, while it should use the credentials of the task given toaudit_filter_rules()as a parameter (tsk).GitHub issue:
https://github.com/linux-audit/audit-kernel/issues/82
Quoted 6 lines
Fixes 8fae47705685 ("audit: add support for session ID user filter")Signed-off-by Ondrej Mosnacek <[email protected]>--- kernel/auditsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Good catch. However, I'm not completely convinced this is stable material.
This bug really only affects "task" filters which I believe to be fairly limited in the wild, due to the limited number of fields that one can filter on. Every other filter, including the "exit" filter, work as expected (which is why the audit-testsuite didn't catch this bug). Further, even in the case of the task filter, the *only* time where the current session ID would be different from the tsk session ID is in the case of a login event, but even in this case the two values should be equal during the "task" filtering as you can't change the login-ID/session-ID until after you have successfully fork()'d/clone()'d the new task.
I'll hold off on merging this in case I'm missing some even more subtle point that you've found. From what I can tell this is a good fix, but not something an actual user would ever really encounter and therefore not something that warrants inclusion in stable.
Quoted 15 lines
kernel/auditsc.c +1
-2@@ -513,7 +513,7 @@ static int audit_filter_rules(struct task_struct *tsk, result =audit_gid_comparator (cred->fsgid, f->op, f->gid);break ;case AUDIT_SESSIONID : - sessionid =audit_get_sessionid (current); + sessionid =audit_get_sessionid (tsk); result =audit_comparator (sessionid, f->op, f->val);break ;case AUDIT_PERS : --2.17.0
Signature · 2 lines
paul moore www.paul-moore.com
2018-05-18 21:23 GMT+02:00 Paul Moore <[email protected]>:
Quoted 29 lines · Ondrej Mosnacek
Quoted 8 lines
The
audit_filter_rules()function inauditsc.ccompared the session ID with the credentials of the current task, while it should use the credentials of the task given toaudit_filter_rules()as a parameter (tsk).GitHub issue:
https://github.com/linux-audit/audit-kernel/issues/82Quoted 6 lines
Fixes 8fae47705685 ("audit: add support for session ID user filter")Signed-off-by Ondrej Mosnacek <[email protected]>--- kernel/auditsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Good catch. However, I'm not completely convinced this is stable material.
This bug really only affects "task" filters which I believe to be fairly limited in the wild, due to the limited number of fields that one can filter on. Every other filter, including the "exit" filter, work as expected (which is why the audit-testsuite didn't catch this bug). Further, even in the case of the task filter, the *only* time where the current session ID would be different from the tsk session ID is in the case of a login event, but even in this case the two values should be equal during the "task" filtering as you can't change the login-ID/session-ID until after you have successfully fork()'d/clone()'d the new task.
Quoted 4 lines
I'll hold off on merging this in case I'm missing some even more subtle point that you've found. From what I can tell this is a good fix, but not something an actual user would ever really encounter and therefore not something that warrants inclusion in stable.
Fair enough, I don't have any objections.
Quoted 20 lines
Quoted 15 lines
kernel/auditsc.c +1
-2@@ -513,7 +513,7 @@ static int audit_filter_rules(struct task_struct *tsk, result =audit_gid_comparator (cred->fsgid, f->op, f->gid);break ;case AUDIT_SESSIONID : - sessionid =audit_get_sessionid (current); + sessionid =audit_get_sessionid (tsk); result =audit_comparator (sessionid, f->op, f->val);break ;case AUDIT_PERS : --Quoted 1 line
2.17.0
--
paul moore
www.paul-moore.com
Signature · 3 lines
Ondrej Mosnacek <omosnace at redhat dot com> Associate Software Engineer, Security Technologies Red Hat, Inc.
Quoted 30 lines · Ondrej Mosnacek
2018-05-18 21:23 GMT+02:00 Paul Moore <[email protected]>:
Quoted 29 lines · Ondrej Mosnacek
Quoted 8 lines
The
audit_filter_rules()function inauditsc.ccompared the session ID with the credentials of the current task, while it should use the credentials of the task given toaudit_filter_rules()as a parameter (tsk).GitHub issue:
https://github.com/linux-audit/audit-kernel/issues/82Quoted 6 lines
Fixes 8fae47705685 ("audit: add support for session ID user filter")Signed-off-by Ondrej Mosnacek <[email protected]>--- kernel/auditsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Good catch. However, I'm not completely convinced this is stable material.
This bug really only affects "task" filters which I believe to be fairly limited in the wild, due to the limited number of fields that one can filter on. Every other filter, including the "exit" filter, work as expected (which is why the audit-testsuite didn't catch this bug). Further, even in the case of the task filter, the *only* time where the current session ID would be different from the tsk session ID is in the case of a login event, but even in this case the two values should be equal during the "task" filtering as you can't change the login-ID/session-ID until after you have successfully fork()'d/clone()'d the new task.
Quoted 6 lines
Quoted 4 lines
I'll hold off on merging this in case I'm missing some even more subtle point that you've found. From what I can tell this is a good fix, but not something an actual user would ever really encounter and therefore not something that warrants inclusion in stable.
Fair enough, I don't have any objections.
Merged into audit/next. In the future you don't have to worry about adding the stable tag to patches, if it warrants sending to stable I can always add it when I merge the patch.
Signature · 2 lines
paul moore www.paul-moore.com