Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SUDO: Timezone issues with sudoNotBefore and sudoNotAfter #6354

Closed
antbob opened this issue Sep 20, 2022 · 1 comment
Closed

SUDO: Timezone issues with sudoNotBefore and sudoNotAfter #6354

antbob opened this issue Sep 20, 2022 · 1 comment
Labels
Closed: Fixed Issue was closed as fixed.

Comments

@antbob
Copy link
Contributor

antbob commented Sep 20, 2022

The sssd sudo does not respect generalized time as specified in related before/after attributes. The problem with the current implementation is that it essentially treats them as local time with no regard to TZ and DST.

eg trying to execute a command past 'sudoNotAfter' date:

Time zone: America/Toronto (EDT, -0400)
NTP enabled: yes
NTP synchronized: yes
RTC in local TZ: no
DST active: yes

dn: cn=sr_test1,ou=SUDOers,dc=example,dc=com
changetype: modify
replace: sudoNotAfter
sudoNotAfter: 20220704164000Z

$ while true; do sudo lvs; date -u; sleep 30; done
...
Mon Jul 4 16:40:05 UTC 2022
...
Mon Jul 4 16:40:35 UTC 2022
...
Mon Jul 4 16:41:05 UTC 2022
...
Mon Jul 4 16:41:35 UTC 2022
...
Mon Jul 4 16:42:05 UTC 2022
...
Mon Jul 4 16:42:35 UTC 2022
...

etc (command output is skipped for brevity) but basically the command continue to execute well past the specified 'sudoNotAfter' date.

The logs show the following rule processing output:

(2022-07-04 12:41:35): [sudo] [sysdb_sudo_check_time] (0x4000): Rule [sr_test1] matches time restrictions
(2022-07-04 12:41:35): [sudo] [sudosrv_cmd_reply] (0x0400): Got 1 rules after time filter
(2022-07-04 12:41:35): [sudo] [sudosrv_build_response] (0x2000): error: [0]
(2022-07-04 12:41:35): [sudo] [sudosrv_build_response] (0x2000): rules_num: [0]
(2022-07-04 12:41:35): [sudo] [sudosrv_build_response] (0x2000): rule [1]/[1]
(2022-07-04 12:41:35): [sudo] [sudosrv_response_append_attr] (0x2000): cn:sr_test1
(2022-07-04 12:41:35): [sudo] [sudosrv_response_append_attr] (0x2000): objectClass:sudoRule
(2022-07-04 12:41:35): [sudo] [sudosrv_response_append_attr] (0x2000): sudoCommand:/usr/sbin/lvs
(2022-07-04 12:41:35): [sudo] [sudosrv_response_append_attr] (0x2000): sudoNotAfter:20220704164000Z
...

antbob added a commit to antbob/sssd that referenced this issue Sep 20, 2022
The current code does not respect generalized time as specified in related before/after attributes.
The problem with the current implementation is that it essentially treats them as local time,
with no regard to TZ and DST.

This patch is using timegm(3) instead of mktime(3) to address said timezone issues and some bare
minimum static unit tests with known verified values to make sure the API is consitent with them.

Resolves:
SSSD#6354
antbob added a commit to antbob/sssd that referenced this issue Sep 26, 2022
The current code does not respect generalized time as specified in related before/after attributes.
The problem with the current implementation is that it essentially treats them as local time,
with no regard to TZ and DST.

This patch is using timegm(3) instead of mktime(3) to address said timezone issues and some bare
minimum static unit tests with known verified values to make sure the API is consitent with them.

Resolves:
SSSD#6354
pbrezina pushed a commit that referenced this issue Sep 27, 2022
The current code does not respect generalized time as specified in related before/after attributes.
The problem with the current implementation is that it essentially treats them as local time,
with no regard to TZ and DST.

This patch is using timegm(3) instead of mktime(3) to address said timezone issues and some bare
minimum static unit tests with known verified values to make sure the API is consitent with them.

Resolves:
#6354

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Pavel Březina <pbrezina@redhat.com>
(cherry picked from commit 7767e516a0ad30224c0045d320a0265ad15d122a)
@pbrezina
Copy link
Member

Pushed PR: #6351

  • master

  • sssd-1-16

  • sssd-2-7

    • 9a8b925 - SUDO: Fix timezone issues with sudoNotBefore and sudoNotAfter

@pbrezina pbrezina added the Closed: Fixed Issue was closed as fixed. label Sep 27, 2022
pbrezina pushed a commit that referenced this issue Sep 27, 2022
The current code does not respect generalized time as specified in related before/after attributes.
The problem with the current implementation is that it essentially treats them as local time,
with no regard to TZ and DST.

This patch is using timegm(3) instead of mktime(3) to address said timezone issues and some bare
minimum static unit tests with known verified values to make sure the API is consitent with them.

Resolves:
#6354

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Pavel Březina <pbrezina@redhat.com>
pbrezina pushed a commit that referenced this issue Sep 27, 2022
The current code does not respect generalized time as specified in related before/after attributes.
The problem with the current implementation is that it essentially treats them as local time,
with no regard to TZ and DST.

This patch is using timegm(3) instead of mktime(3) to address said timezone issues and some bare
minimum static unit tests with known verified values to make sure the API is consitent with them.

Resolves:
#6354

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Pavel Březina <pbrezina@redhat.com>
(cherry picked from commit 0198f64)
etrunko pushed a commit to etrunko/sssd that referenced this issue Nov 16, 2023
The current code does not respect generalized time as specified in related before/after attributes.
The problem with the current implementation is that it essentially treats them as local time,
with no regard to TZ and DST.

This patch is using timegm(3) instead of mktime(3) to address said timezone issues and some bare
minimum static unit tests with known verified values to make sure the API is consitent with them.

Resolves:
SSSD#6354

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Pavel Březina <pbrezina@redhat.com>
(cherry picked from commit 0198f64)
etrunko pushed a commit to etrunko/sssd that referenced this issue Nov 16, 2023
The current code does not respect generalized time as specified in related before/after attributes.
The problem with the current implementation is that it essentially treats them as local time,
with no regard to TZ and DST.

This patch is using timegm(3) instead of mktime(3) to address said timezone issues and some bare
minimum static unit tests with known verified values to make sure the API is consitent with them.

Resolves:
SSSD#6354

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Pavel Březina <pbrezina@redhat.com>
(cherry picked from commit 0198f64)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed: Fixed Issue was closed as fixed.
Projects
None yet
Development

No branches or pull requests

2 participants