[Bug 753027] New: Review Request: pam-pgsql - PAM module to authenticate using a PostgreSQL database
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: pam-pgsql - PAM module to authenticate using a PostgreSQL database
https://bugzilla.redhat.com/show_bug.cgi?id=753027
Summary: Review Request: pam-pgsql - PAM module to authenticate using a PostgreSQL database Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: bochecha@fedoraproject.org QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: ---
Spec URL: http://bochecha.fedorapeople.org/packages/pam-pgsql.spec SRPM URL: http://bochecha.fedorapeople.org/packages/pam-pgsql-0.7.3.1-1.fc16.src.rpm Description: PAM module using a PostgreSQL database to authenticate users against a table. It also supports: checking account info (pam_acct_expired, new_authtok_reqd), updating auth token.
$ rpmlint pam-pgsql* pam-pgsql.src: W: spelling-error %description -l en_US authtok -> author, authentic, autochthon pam-pgsql.src: W: spelling-error %description -l en_US reqd -> req, red, reed pam-pgsql.x86_64: W: spelling-error %description -l en_US authtok -> author, authentic, autochthon pam-pgsql.x86_64: W: spelling-error %description -l en_US reqd -> req, red, reed 3 packages and 1 specfiles checked; 0 errors, 4 warnings.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=753027
Amir Hedayaty hedayaty@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |hedayaty@gmail.com
--- Comment #1 from Amir Hedayaty hedayaty@gmail.com 2011-11-19 05:55:21 EST --- Thanks for the nice package, I also a big user of vim, I really enjoyed this It does not look so nice at first glance, the fact that you can assign, compile/run to tabs is pretty handy!
The following build-requirements are a little bit hard to work-around BuildRequires: pkgconfig(gtk+-2.0) BuildRequires: pkgconfig(vte) BuildRequires: pkgconfig(gee-1.0) BuildRequires: pkgconfig(gio-2.0)
Is it better to use gtk2-devel, cte-devel. libgee-devel, glib2-devel
Seems you are trying to open an unrelated pixmap not available of my fedora Failed to open file '/usr/share/pixmaps/gnome-term.png': No such file or directory
Another minor bug is the ability to handle duplicates, for example if 2 instances are running, quit one of them (maybe the older one). I had a few crash experiences and when trying to invoke it again, there was an error "Could not aquire DBUS name", after killing the old one it become OK.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=753027
--- Comment #2 from Amir Hedayaty hedayaty@gmail.com 2011-11-19 05:57:59 EST --- sorry, wrong comment! this was meant for another report!
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=753027
Amir Hedayaty hedayaty@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-review+
--- Comment #3 from Amir Hedayaty hedayaty@gmail.com 2011-11-19 08:40:51 EST --- To my knowledge this package is quite suitable for Fedora my suggestions are:
remove "%defattr(-,root,root,-)" it is not needed any more
also if you add some selinux workaround it would be nice, by default login is now allowed to connect to postgresql unless selinux is on permissive mode (which is similar to being disabled)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=753027
--- Comment #4 from Mathieu Bridon bochecha@fedoraproject.org 2011-11-19 09:07:05 EST --- (In reply to comment #3)
To my knowledge this package is quite suitable for Fedora my suggestions are:
remove "%defattr(-,root,root,-)" it is not needed any more
Right, I keep forgetting about this one.
also if you add some selinux workaround it would be nice, by default login is now allowed to connect to postgresql unless selinux is on permissive mode (which is similar to being disabled)
Oh, good catch!
The system I tested this package on has SELinux disabled, so I didn't see this issue. I guess I still have some work to do then. :)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=753027
Parag pnemade@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |pnemade@redhat.com Flag|fedora-review+ |
--- Comment #5 from Parag pnemade@redhat.com 2011-11-19 09:36:11 EST --- Amir, Please don't change any flags. you are not yet sponsored by me. Once you get sponsorship you can start official reviews and accept packages in Fedora.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=753027
Robert Scheck redhat@linuxnetz.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |redhat-bugzilla@linuxnetz.d | |e AssignedTo|nobody@fedoraproject.org |redhat-bugzilla@linuxnetz.d | |e Flag| |fedora-review?
--- Comment #6 from Robert Scheck redhat@linuxnetz.de 2011-11-20 21:17:27 EST --- Mathieu, your %post/%postun doesn't make sense for PAM at all, it's meant for libraries only.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=753027
--- Comment #7 from Mathieu Bridon bochecha@fedoraproject.org 2012-01-30 23:21:39 EST --- Sorry it took me so long to reply, I've been completely swamped lately (but things are improving fortunately :)
Here is an updated package, with Amir's and Robert's feedback addressed.
Spec URL: http://bochecha.fedorapeople.org/packages/pam-pgsql.spec SRPM URL: http://bochecha.fedorapeople.org/packages/pam-pgsql-0.7.3.1-2.fc16.src.rpm
$ rpmlint pam-pgsql* pam-pgsql.src: W: spelling-error %description -l en_US authtok -> author pam-pgsql.src: W: spelling-error %description -l en_US reqd -> red, reed, read pam-pgsql.src: W: spelling-error %description -l en_US auth -> auto, Ruth, author pam-pgsql.x86_64: W: spelling-error %description -l en_US authtok -> author pam-pgsql.x86_64: W: spelling-error %description -l en_US reqd -> red, reed, read pam-pgsql.x86_64: W: spelling-error %description -l en_US auth -> auto, Ruth, author 3 packages and 1 specfiles checked; 0 errors, 6 warnings.
Thanks for the feedback. I'll try my best to react in a more timely fashion for the rest of the review.
https://bugzilla.redhat.com/show_bug.cgi?id=753027
--- Comment #8 from Florian Weimer fweimer@redhat.com --- I don't think libpq (the PostgreSQL client library) is designed to be used in a SUID process, so this PAM module is likely not entirely safe to use as the system default.
https://bugzilla.redhat.com/show_bug.cgi?id=753027
--- Comment #9 from Mathieu Bridon bochecha@fedoraproject.org --- (In reply to Florian Weimer from comment #8)
I don't think libpq (the PostgreSQL client library) is designed to be used in a SUID process, so this PAM module is likely not entirely safe to use as the system default.
Do you mean you wouldn't recommend to introduce this package in Fedora at all?
Or do you mean that if this package is introduced into Fedora, then it should not be used on a default Fedora install?
I certainly don't intend to do the latter, and if you have strong concerns over the security of this package, maybe even the former is not a great idea? :-/
(I still didn't have time to resolve the selinux issues mentioned in comment 3, which might be an additional concern)
https://bugzilla.redhat.com/show_bug.cgi?id=753027
James Hogarth james.hogarth@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |bochecha@daitauha.fr, | |james.hogarth@gmail.com Assignee|redhat-bugzilla@linuxnetz.d |nobody@fedoraproject.org |e | Flags|fedora-review? |needinfo?(bochecha@daitauha | |.fr)
--- Comment #10 from James Hogarth james.hogarth@gmail.com --- This package appears to be stalled in review.
As per process I'm resetting the assignee and flags.
https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
Mathieu do you have intentions to complete the review?
https://bugzilla.redhat.com/show_bug.cgi?id=753027
--- Comment #11 from Florian Weimer fweimer@redhat.com --- (In reply to Mathieu Bridon from comment #9)
(In reply to Florian Weimer from comment #8)
I don't think libpq (the PostgreSQL client library) is designed to be used in a SUID process, so this PAM module is likely not entirely safe to use as the system default.
Do you mean you wouldn't recommend to introduce this package in Fedora at all?
Yes, the implementation needs to be split into a in-process stub and a separate daemon, like nss-pam-ldapd.
(I still didn't have time to resolve the selinux issues mentioned in comment 3, which might be an additional concern)
That would also help to address the SELinux issue.
https://bugzilla.redhat.com/show_bug.cgi?id=753027
Mathieu Bridon bochecha@daitauha.fr changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution|--- |WONTFIX Flags|needinfo?(bochecha@daitauha | |.fr) | Last Closed| |2015-12-04 02:11:33
--- Comment #12 from Mathieu Bridon bochecha@daitauha.fr --- (In reply to James Hogarth from comment #10)
Mathieu do you have intentions to complete the review?
Not any more no. (I had submitted this review while at a previous job where we needed it, but leaving that job left me without a need for this package)
Sorry I kept this review request open for so long instead of just closing it, I had just completely forgotten about it.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla