Bug 753027 - Review Request: pam-pgsql - PAM module to authenticate using a PostgreSQL database
Summary: Review Request: pam-pgsql - PAM module to authenticate using a PostgreSQL dat...
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-11-11 04:31 UTC by Mathieu Bridon
Modified: 2015-12-04 07:11 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-12-04 07:11:33 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Mathieu Bridon 2011-11-11 04:31:34 UTC
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.

Comment 1 Amir Hedayaty 2011-11-19 10:55:21 UTC
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.

Comment 2 Amir Hedayaty 2011-11-19 10:57:59 UTC
sorry, wrong comment! this was meant for another report!

Comment 3 Amir Hedayaty 2011-11-19 13:40:51 UTC
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)

Comment 4 Mathieu Bridon 2011-11-19 14:07:05 UTC
(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. :)

Comment 5 Parag Nemade 2011-11-19 14:36:11 UTC
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.

Comment 6 Robert Scheck 2011-11-21 02:17:27 UTC
Mathieu, your %post/%postun doesn't make sense for PAM at all, it's meant for 
libraries only.

Comment 7 Mathieu Bridon 2012-01-31 04:21:39 UTC
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.

Comment 8 Florian Weimer 2013-11-21 15:19:46 UTC
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.

Comment 9 Mathieu Bridon 2013-11-22 03:28:25 UTC
(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)

Comment 10 James Hogarth 2015-12-04 03:15:48 UTC
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?

Comment 11 Florian Weimer 2015-12-04 07:06:30 UTC
(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.

Comment 12 Mathieu Bridon 2015-12-04 07:11:33 UTC
(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.


Note You need to log in before you can comment on or make changes to this bug.