Spec URL: https://decathorpe.fedorapeople.org/packages/gsignond.spec SRPM URL: https://decathorpe.fedorapeople.org/packages/gsignond-1.0.6-1.fc26.src.rpm Description: The GSignOn daemon is a D-Bus service which performs user authentication on behalf of its clients. There are currently authentication plugins for OAuth 1.0 and 2.0, SASL, Digest-MD5, and plain username/password combination. Fedora Account System Username: decathorpe koji scratch build for rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=19008549 This package is one of the dependencies for pantheon-mail, pantheon-online-accounts / switchboard-plug-onlineaccounts - which are among the last missing pieces for a complete Pantheon desktop experience on fedora.
Taking this review.
Going through fedora-review and analyzing the spec, I've noticed several issues: * fedora-review complains about gsignond being setuid. Is there a reason for this? If so, why? If anything, I don't see a good reason for it at all. If there *is* a good reason for it, it needs to be documented... * Your default-config subpackage doesn't make a ton of sense. First, why does it require the libs, when the libs don't care? It would probably make sense for the default-config subpackage to provide "%{name}-config" and have gsignond require it. * Your doc subpackage shouldn't require the libs, since it's development documentation. If you want it to require anything, it should require the devel subpackage, as it is developer documentation. * At the very least, the doc subpackage should be noarch, but as the config file doesn't appear to have any arch specific information, the default-config subpackage can also be noarch. * Some dependencies seem to have failed to be detected. Of note, it looked like it was looking for CHECK and LIBECRYPTFS, and those failed to be found. At least with those two, it looks like you don't have the appropriate pkgconfig() BuildRequires (pkgconfig(check) and pkgconfig(libecryptfs)). It was also unable to locate /usr/include/trousers/tss.h (provided by trousers-devel). * Since you're generating the autofoo, you need automake, autoconf, and autoconf-archive as BuildRequires. I'm not sure how the autogen.sh is working at the moment (maybe something else is pulling it in when it shouldn't), but in this case, it's better to be explicit about it.
1) It seems even elementaryOS ships gsignond without the setuid bit. I'll include a patch to not set the setuid bit. 2) I've changed it around so the main package depends on gsignond-config, which the -default-config package provides (and other packages can also provide in the future). I've also removed the dependency on the -libs subpackage (I think that was a copy-paste-error). I've also marked the -default-config subpackage as noarch. 3+4) I've removed the dependency on the -libs subpackage and marked the -docs package as noarch as well. 5) I've added pkgconfig(check), pkgconfig(libecryptfs) and trousers-devel as BuildRequires. The only problem is that - now that more plugins are built - some of them FTBFS (I've reported this issue upstream at [1]). I could also just disable the offending plugin, since it seems to be related to Intel's IoT OS (Ostro). 6) automake and autoconf are explicit dependencies of libtool - so adding them explicitly as BRs is not necessary. [1]: https://gitlab.com/accounts-sso/gsignond/issues/9
> I could also just disable the offending plugin, since it seems to be related to Intel's IoT OS (Ostro). Since you've reported the issue upstream and it doesn't appear to be useful, disabling the broken plugin should be fine.
I think I have incorporated fixes for all the issues you pointed out. Also, I have disabled the Ostro OS and Tizen extensions. Links are the same. New scratch build, with noarch -doc and -defautl-config subpackages: https://koji.fedoraproject.org/koji/taskinfo?taskID=19040060
Issues: >[!]: Package must own all directories that it creates. > Note: Directories without known owners: /usr/share/gir-1.0, /usr/share > /gtk-doc, /usr/share/dbus-1, /usr/share/vala/vapi, > /usr/share/dbus-1/services, /usr/share/gtk-doc/html/gsignond, > /usr/share/gtk-doc/html, /usr/lib64/girepository-1.0, > /usr/share/dbus-1/interfaces, /usr/share/vala Some of the ownership issues can be ignored (like /usr/share/vala, /usr/share/vala/vapi, /usr/share/gtk-doc, /usr/share/gtk-doc/html, /usr/lib64/girepository-1.0). However, gsignond needs a runtime requires for dbus%{?_isa}, as it is necessary for its functionality to be useful. The doc subpackage file entry needs the asterisk removed, because it's preventing RPM from considering that it should own /usr/share/gtk-doc/html/gsignond too. >Rpmlint: > gsignond.x86_64: E: missing-call-to-setgroups-before-setuid /usr/bin/gsignond Apparently, gsignond was a setuid binary on purpose. In the code, it uses seteuid() in daemon/main.c and common/gsignond-storage-manager.c. However, it seems to gracefully fail when it does that. That said, apparently it's not doing setegid() before using seteuid() in common/gsignond-storage-manager.c (or in the ostro/tizen code, but I don't care much about them). From what I can tell, it's setuid so that it can drop privileges as a daemon and set storage directories to be individually owned by specific users. In Fedora, we prefer if this can function with file capabilities, as they are more granular and when used well, can limit the damage caused by vulnerabilities to privileged applications. Please file a bug upstream to see if this can be appropriately resolved. Action items: * Fix the directory ownership issues * File a bug upstream about the rpmlint error and to request gsignond to work with file caps instead. - https://fedoraproject.org/wiki/Features/RemoveSETUID - https://www.mankier.com/7/capabilities
- added: Requires: dbus%{?isa} - removed asterisk from doc directory - filed bug about setuid / file caps upstream at [1] [1]: https://gitlab.com/accounts-sso/gsignond/issues/10
Aside from the already known issues, fedora-review is happy with it. PACKAGE APPROVED.
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/gsignond
I've reported a bug about the setgroups / setuid usage upstream at: https://gitlab.com/accounts-sso/gsignond/issues/11
gsignond-1.0.6-1.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-4cfec99e64
gsignond-1.0.6-1.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-85a6084eee
gsignond-1.0.6-1.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-85a6084eee
gsignond-1.0.6-2.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-edda6d990d
gsignond-1.0.6-1.fc26 has been pushed to the Fedora 26 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-4cfec99e64
gsignond-1.0.6-2.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-edda6d990d
gsignond-1.0.6-2.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.