Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 221957
Summary: | Review Request: ksshaskpass - A KDE version of ssh-askpass with KWallet support | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Aurelien Bompard <gauret> |
Component: | Package Review | Assignee: | Rex Dieter <rdieter> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | chitlesh |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-01-14 21:10:21 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Aurelien Bompard
2007-01-09 09:36:30 UTC
Looks interesting, I can review. Offhand, you don't need: # work around an improper ${kdelibsuff} export QTLIB=${QTDIR}/lib QTINC=${QTDIR}/include anymore. qt was fixed wrt a long time ago. (: I'm leary of this as well: cat > $RPM_BUILD_ROOT%{_sysconfdir}/profile.d/ksshaskpass.sh << EOF SSH_ASKPASS=%{_bindir}/ksshaskpass export SSH_ASKPASS (Besides, ksshaskpass-README.Fedora mentions also running ssh-add, so why is that not also present here?) I was going to say, perhaps /etc/kde/env would be a better place for this, but then it would be strictly limited to kde users... hey, some gnome user may be crazy enough to want to try this, right? (: Otherwise, package is pretty simple (and easy to review). (: * SOURCE matches upstream: md5sum 3d0addbecfcb31aedb59f36f7a34b349 ksshaskpass-0.3.tar.gz The more I think about it (comment #3), the more I think /etc/kde/env should be used for the startup script. We definitely don't want SSH_ASKPASS defined for console/text sessions, so * MUSTFIX: put ksshaskpass.sh in /etc/kde/env * MUSTFIX: add ssh-add to ksshaskpass.sh per README (unless you had some good reason to omit that) fixup those items, and I think we have a winner. > Offhand, you don't need: > # work around an improper ${kdelibsuff} > export QTLIB=${QTDIR}/lib QTINC=${QTDIR}/include > anymore. qt was fixed wrt a long time ago. (: Great ! That was abusive cut'n'paste... About the environement script, I just took the example from the one provided by the openssh-askpass package. I agree about moving it to /etc/kde/env (I didn't know of this dir, interesting :) ), but I don't think adding an ssh-add call to it is a good idea, for the following reasons : - scripts in env should not start programs, only define environment vars. Startup applications should be in ~/.kde/Autostart or /usr/share/autostart. - some people dislike ssh-agent, because it knows about their password. By calling ssh-add in the script, we would force it on every user on the system. that's why I added a README.Fedora with usage instructions. > We definitely don't want SSH_ASKPASS defined for console/text sessions Actually, if you call ssh-add from an interactive terminal, it will not use the SSH_ASKPASS application, it will just prompt for the password. That's why the openssh-askpass package defines it in /etc/profile.d Upstream provides an sha1sum on this page : http://hanz.nl/p/program , you can check it too if you want. * Tue Jan 09 2007 Aurelien Bompard <abompard> 0.3-2 - remove useless workaround - put the environment script in /etc/kde/env http://gauret.free.fr/fichiers/rpms/fedora/ksshaskpass.spec http://gauret.free.fr/fichiers/rpms/fedora/ksshaskpass-0.3-2.src.rpm Excellent, thanks for the explanations and update. APPROVED. Now we need the author's also cool-looking ksshagent (: Imported, branched, tagged and published. Thanks. |