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. |