Review Request 129526: RFE: kwallet-pam should use XDG_RUNTIME_DIR instead of /tmp for the socketPath

[David Faure]

> On Jan. 8, 2017, 4:09 p.m., David Faure wrote:
> > pam_kwallet.c, line 422
> > <https://git.reviewboard.kde.org/r/129526/diff/1/?file=486385#file486385line422>
> >
> >     trailing spaces
> 
> Damjan Georgievski wrote:
>     > according to http://standards.freedesktop.org/basedir-spec/latest/, one is supposed to check permissions
>     
>     I don't see it in the specs, and it says: „The directory MUST be owned by the user, and he MUST be the only one having read and write access to it. Its Unix access mode MUST be 0700.“ - but it might be a sensible thing to check (although there are race conditions in checking and only trying to use it later).
>     
>     > trailing spaces
>     
>     ughh, what do I do now, "Update diff"?
> 
> David Faure wrote:
>     Yes, these "MUST" are exactly what I meant the code is supposed to check before using XDG_RUNTIME_DIR :)
>     
>     I'm confused by your reply, you say "I don't see it in the spec" and then you quote exactly what I am referring to.
>     
>     There is no race condition in checking for "I own it and it's 0700" before using, because this can only change if root intervenes, another user cannot do anything about a dir that he doesn't own and that is 0700. And if root is compromised, all is lost anyway ;)
> 
> Damjan Georgievski wrote:
>     > I'm confused by your reply, you say "I don't see it in the spec" and then you quote exactly what I am referring to.
>     
>     huh, I might be mistaken, but the way I read it, the creator of $XDG_RUNTIME_DIR MUST do those things, otherwise it shouldn't set the environment variable.

I see. It's indeed unclear in the spec. I guess it was clearer in the initial discussion on the ML...

Every one else checks though ;)
https://git.enlightenment.org/core/enlightenment.git/commit/?id=2520c73d04049f1c84c67cc5e3e55c10d2078025
https://github.com/whitequark/rust-xdg/pull/10/files
+ what I did in QStandardPaths.

Anyway, compared to /tmp I guess a world-writable socket dir isn't worse, I'm not going to veto it since the spec is unclear, I'll let someone else approve this though. Meanwhile I'll ask for clarification on the xdg list ;)


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129526/#review101873
-----------------------------------------------------------


On Jan. 8, 2017, 4:59 p.m., Damjan Georgievski wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129526/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2017, 4:59 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 365722
>     https://bugs.kde.org/show_bug.cgi?id=365722
> 
> 
> Repository: kwallet-pam
> 
> 
> Description
> -------
> 
> Most recent Linux distributions setup a per-user XDG_RUNTIME_DIR as a tmpfs, which is also tied to their session lifecycle. Typically this is in /run/user/1000/.
> 
> My suggestion is to use $XDG_RUNTIME_DIR/kwallet5.socket if XDG_RUNTIME_DIR exists, or fallback to /tmp/kwallet5_${username}.socket if it doesn't.
> 
> Reproducible: Always
> 
> 
> Diffs
> -----
> 
>   pam_kwallet.c 809ab9a 
> 
> Diff: https://git.reviewboard.kde.org/r/129526/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Damjan Georgievski
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170108/da04e858/attachment.html>