Bug 49036 - Wrong permissions for /etc/univention/ssl/$FQDN
Wrong permissions for /etc/univention/ssl/$FQDN
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: SSL
UCS 4.4
Other Linux
: P5 normal (vote)
: UCS 4.4-3-errata
Assigned To: Julia Bremer
Florian Best
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2019-03-19 14:05 CET by Philipp Hahn
Modified: 2019-12-18 13:33 CET (History)
6 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 6: Setup Problem: Issue for the setup process
Who will be affected by this bug?: 2: Will only affect a few installed domains
How will those affected feel about the bug?: 2: A Pain – users won’t like this once they notice it
User Pain: 0.137
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score:
hahn: Patch_Available+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Philipp Hahn univentionstaff 2019-03-19 14:05:00 CET
After installation of UCS-4.4-0 via PXE *directory* "/etc/univention/ssl/$FQDN" has permissions 0o640 - should be 0o750.

This is caused by some script doing "ln -s /etc/univention/ssl/$FQDN /etc/univention/ssl/$HOSTNAME" where ".../$HOSTNAME" already exists.
This leads to *another* symblink being created inside ".../$FQDN/".

When next the computer account is modified (in my case "/usr/lib/univention-install/96univention-samba4.inst") the listener module "/usr/lib/univention-directory-listener/system/gencertificate.py" runs.
It uses "os.path.walk()" which does *NOT* distinguish between "regular file", "directory" and "symbolic link".
It changes all referenced files to 0o640.

1. s/ln -s/ln -snf/
2. s/os.path.walk()/os.walk()/ or +if os.path.islink(): continue+

(see <https://hutten.knut.univention.de/blog/unix-109-ln-s/>)
Comment 1 Philipp Hahn univentionstaff 2019-03-19 17:52:29 CET
UCS technical training 2019-03-21/22
Comment 2 Florian Best univentionstaff 2019-03-20 13:56:43 CET
The diagnostic check should be checked/adjusted accordingly (it seems it already checks for 0o755):
management/univention-management-console-module-diagnostic/umc/python/diagnostic/plugins/31_file_permissions.py
  145 »   »   cf_type('/etc/univention/ssl/{}.{}'.format(host, domain), '{}$'.format(host) if is_primary else 'root', 'DC Backup Hosts' if is_dc else 'root', 0o750, must_exist=True),
Comment 3 Philipp Hahn univentionstaff 2019-03-20 16:21:58 CET
(In reply to Florian Best from comment #2)
> The diagnostic check should be checked/adjusted accordingly (it seems it
> already checks for 0o755):

NO, the check is right, do not hide the underlying problems!
Comment 4 Florian Best univentionstaff 2019-03-20 16:30:28 CET
(In reply to Philipp Hahn from comment #3)
> (In reply to Florian Best from comment #2)
> > The diagnostic check should be checked/adjusted accordingly (it seems it
> > already checks for 0o755):
> 
> NO, the check is right, do not hide the underlying problems!
Yes, that's why I said that it already checks for 0o755.

So if this bug occurs it will currently already be displayed in the diagnostic module and there is a button to fix it.
Comment 5 Philipp Hahn univentionstaff 2019-05-06 13:06:02 CEST
(In reply to Florian Best from comment #4)
> (In reply to Philipp Hahn from comment #3)
> > (In reply to Florian Best from comment #2)
> > > The diagnostic check should be checked/adjusted accordingly (it seems it
> > > already checks for 0o755):
> > 
> > NO, the check is right, do not hide the underlying problems!
> Yes, that's why I said that it already checks for 0o755.
> 
> So if this bug occurs it will currently already be displayed in the
> diagnostic module and there is a button to fix it.

1. The PXE installation should not be bad by default.
2. The is NO such button to fix it.

UCS technical training 2019-05-08/09
Comment 6 Philipp Hahn univentionstaff 2019-05-06 14:32:50 CEST
Fixed in branch git:phahn/49036-ssl-listener

[phahn/49036-ssl-listener] 25aa3895ab Bug #49193 USS: Fix ssl symlink creation
 base/univention-system-setup/debian/changelog                  |  6 ++++++
 .../lib/univention-system-setup/scripts/10_basis/10hostname    |  4 ++--
 .../lib/univention-system-setup/scripts/10_basis/12domainname  |  6 +++---
 .../usr/lib/univention-system-setup/scripts/40_ssl/10ssl       |  2 +-
 .../usr/lib/univention-system-setup/scripts/setup-join.sh      |  4 ++--
 doc/errata/staging/univention-system-setup.yaml                | 10 ++++++++++
 6 files changed, 24 insertions(+), 8 deletions(-)

[phahn/49036-ssl-listener] 89cfeeb14b Bug #49193 server: Fix ssl symlink creation
 base/univention-server/debian/changelog                        | 6 ++++++
 base/univention-server/debian/univention-server-master.preinst | 2 +-
 doc/errata/staging/univention-server.yaml                      | 7 ++++---
 3 files changed, 11 insertions(+), 4 deletions(-)

[phahn/49036-ssl-listener] 17ea0a7b6f Bug #49036 ssl: Changelog and YAML
 base/univention-ssl/debian/changelog   |  6 ++++++
 doc/errata/staging/univention-ssl.yaml | 12 ++++++++++++
 2 files changed, 18 insertions(+)

[phahn/49036-ssl-listener] df7b33ab7a Bug #49036 ssl: Add mypy annotations.
 base/univention-ssl/gencertificate.py | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

[phahn/49036-ssl-listener] 04cf97d1c5 Bug #49036 ssl: Add Python DocStrings
 base/univention-ssl/gencertificate.py | 36 +++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

[phahn/49036-ssl-listener] 81cc375f6e Bug #49036 ssl: Improve error reporting
 base/univention-ssl/gencertificate.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

[phahn/49036-ssl-listener] 69df7c6a8a Bug #49036 ssl: Fix cleanup tree
 base/univention-ssl/gencertificate.py | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

[phahn/49036-ssl-listener] a6e0ed2614 Bug #49036 ssl: Use relative path for symlink
 base/univention-ssl/debian/univention-ssl.postinst | 2 +-
 base/univention-ssl/gencertificate.py              | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

[phahn/49036-ssl-listener] 43425c6125 Bug #49036 ssl: Fix broken symbolic links
 base/univention-ssl/gencertificate.py | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)


QA:
 name="bug49036"
 dom="$(ucr get domainname)"
 lb="$(ucr get ldap/base)" 
 fqdn="/etc/univention/ssl/$name.$dom"
 udm computers/memberserver create --name "$name"
 ln -snf "$fqdn" "$fqdn/"
 udm computers/memberserver modify --dn "cn=$name,$lb" --set  description="$RANDOM"
 stat -c '%a' "$fqdn" # wrong:640 -> correct:750

PS: The last two commits fix USS and server to get rid of the broken "ln -s" without "-n" and "-f". and also convert all symbolic links to relative paths (Bug #34106)
Comment 8 Ingo Steuwer univentionstaff 2019-05-14 17:49:56 CEST
Please correct me if I'm wrong: use case here is "only" if a DC Master is deployed by PXE
Comment 9 Philipp Hahn univentionstaff 2019-05-15 09:16:26 CEST
(In reply to Ingo Steuwer from comment #8)
> Please correct me if I'm wrong: use case here is "only" if a DC Master is
> deployed by PXE

At least it happens there every month I do a UCS technical training.
I don't care if there is some other way to trigger it, I want to get it fixed for at least my use-case. It is annoying enough for me to get it fixed: patch is ready, just needs QA the next BSD.
Comment 10 Arvid Requate univentionstaff 2019-05-15 12:24:25 CEST
> just needs QA the next BSD.

Yes, fine, that was the opinion in the discussion.
Comment 11 Philipp Hahn univentionstaff 2019-10-21 11:26:30 CEST
FYI: "ucs-test/00_checks/81_diagnostic_checks" regularly fails on "backup092": There are multiple code paths in UCS which create / copy / touch / modify files below /etc/univention/ssl/ and use slightly different users / groups / permissions. Also see Bug #50394
Comment 12 Julia Bremer univentionstaff 2019-12-10 18:58:00 CET
8729eb0564 Bug #49036: yaml update
9b9788cd02 Bug #49036: Merge branch 'fbest/49036-ssl-listener' into 4.4-3

Successful build
Package: univention-ssl
Version: 13.0.0-6A~4.4.0.201912101852
Branch: ucs_4.4-0
Scope: errata4.4-3
User: jbremer


Merged the branch to 4.4-3
Comment 13 Florian Best univentionstaff 2019-12-11 11:19:56 CET
REOPEN: univention-system-setup has not been built
Comment 14 Julia Bremer univentionstaff 2019-12-11 12:32:34 CET
ec6f5a4533 Bug #49036: yaml update
c2306b4df9 Bug #49036: Update changelog

Successful build
Package: univention-system-setup
Version: 12.0.2-19A~4.4.0.201912111155
Branch: ucs_4.4-0
Scope: errata4.4-3
User: jbremer

Sorry for that, 
I build univention-system-setup and updated the yaml file
Comment 15 Felix Botner univentionstaff 2019-12-12 10:32:52 CET
hmm, still fails on 
https://jenkins.knut.univention.de:8181/job/UCS-4.4/job/UCS-4.4-3/job/AutotestJoin/SambaVersion=s4,Systemrolle=backup/lastCompletedBuild/testReport/00_checks/81_diagnostic_checks/backup093/

univention-system-setup	12.0.2-19A~4.4.0.201912111155
univention-ssl	13.0.0-6A~4.4.0.201912101852
Comment 16 Florian Best univentionstaff 2019-12-12 11:01:59 CET
(In reply to Felix Botner from comment #15)
> hmm, still fails on 
> https://jenkins.knut.univention.de:8181/job/UCS-4.4/job/UCS-4.4-3/job/
> AutotestJoin/SambaVersion=s4,Systemrolle=backup/lastCompletedBuild/
> testReport/00_checks/81_diagnostic_checks/backup093/
> 
> univention-system-setup	12.0.2-19A~4.4.0.201912111155
> univention-ssl	13.0.0-6A~4.4.0.201912101852

Could the reason be, that the certificates are created with the old univention-system-setup version? Or is that package upgraded before the profile is activated?
Comment 17 Philipp Hahn univentionstaff 2019-12-12 11:05:33 CET
(In reply to Felix Botner from comment #15)
> hmm, still fails on 
> https://jenkins.knut.univention.de:8181/job/UCS-4.4/job/UCS-4.4-3/job/
> AutotestJoin/SambaVersion=s4,Systemrolle=backup/lastCompletedBuild/
> testReport/00_checks/81_diagnostic_checks/backup093/
> 
> univention-system-setup	12.0.2-19A~4.4.0.201912111155
> univention-ssl	13.0.0-6A~4.4.0.201912101852

Please read comment 11 on why it still fails:
 base/univention-ssl/gencertificate.py uses 640:root:DC Backup Hosts
 base/univention-ssl/make-certificates.sh uses 600/700:root:DC Backup Hosts

And there are cases, where the group lookup for "DC Backup Hosts" does not (yet) work and thus the files are assigned to "root" instead of "DC Backup Hosts".

And if I remember correctly there is a cron job, which "fixes" the permissions. Depending on when the jobs runs relative to the join process the check either fails or succeeds.
Comment 18 Florian Best univentionstaff 2019-12-12 14:56:27 CET
I found the case where the permissions are set to 0775:

/usr/sbin/univention-join:857
    univention-ssh-rsync "$DCPWD" -az "${DCACCOUNT}@${DCNAME}:/etc/univention/ssl/*" /etc/univention/ssl/ >>/var/log/univention/join.log 2>&1
Comment 19 Florian Best univentionstaff 2019-12-12 14:58:18 CET
(In reply to Florian Best from comment #18)
> I found the case where the permissions are set to 0775:
> 
> /usr/sbin/univention-join:857
>     univention-ssh-rsync "$DCPWD" -az
> "${DCACCOUNT}@${DCNAME}:/etc/univention/ssl/*" /etc/univention/ssl/
> >>/var/log/univention/join.log 2>&1

Urgs, typo: 750.
But that is the expected.

The case where it is set to 770 is in:
/usr/lib/univention-install/20univention-join.inst
Comment 20 Florian Best univentionstaff 2019-12-12 15:18:09 CET
chmod change by by: `chmod g+rwx "/etc/univention/ssl/$hostname"`
in:
/usr/lib/univention-install/20univention-join.inst:110

     test -d "/etc/univention/ssl/$hostname" && chgrp -R "DC Backup Hosts" "/etc/univention/ssl/$hostname" && chmod g+rwx "/etc/univention/ssl/$hostname" && find "/etc/univention/ssl/$hostname/" -type f | xargs chmod g+rw
Comment 21 Julia Bremer univentionstaff 2019-12-12 16:50:09 CET
Successful build
Package: univention-join
Version: 11.0.1-27A~4.4.0.201912121646
Branch: ucs_4.4-0
Scope: errata4.4-3


8e0106d27f Bug #49036: update yaml
94b3d9dcf9 Bug #49036: Fix wrong permissions set in 20univention-join.inst

Todo: See if the tests succeed.
Comment 22 Florian Best univentionstaff 2019-12-13 10:24:25 CET
There are several commits with the wrong bug number (Bug #49193) in the message:

univention-system-setup (12.0.2-16)
315a9de2b628 | Bug #49193 USS: Fix ssl symlink creation

univention-system-setup.yaml
315a9de2b628 | Bug #49193 USS: Fix ssl symlink creation

univention-server (14.0.0-10)
60ae87b6df24 | Bug #49193 server: Fix ssl symlink creation

univention-server.yaml
60ae87b6df24 | Bug #49193 server: Fix ssl symlink creation
Comment 23 Florian Best univentionstaff 2019-12-13 10:45:00 CET
OK: univention-server
OK: univention-system-setup
OK: univention-join
OK: univention-ssl
OK: reproduced && fixed (comment #6)
OK: jenkins test for diagnose module
OK: YAML's (adjusted in adc20bb65317efa730096d850fac882ca49ec8e7)
Comment 24 Florian Best univentionstaff 2019-12-13 10:50:39 CET
Btw: Philipp already analyzed the reason for the failing Jenkins Test in Bug #45372 comment 2