Mailing List Archive

I've posted the patch as a PR up at
https://github.com/openssh/openssh-portable/pull/346

Is there a better way to get this moving forward?

Pat

On Wed, 2022-09-21 at 12:00 -0500, Pat Riehecky wrote:
> The patch attached to
> https://bugzilla.mindrot.org/show_bug.cgi?id=2950
> applies to the current portable OpenSSH source.
>
> How can I help get the patch in that bug applied?
>
> I'm starting to hear rumblings about storing authentication tokens on
> persistent filesystems (like /tmp, when it isn't tmpfs).  The patch
> itself is fairly tiny, but would pay off for ensuring any forwarded
> credentials are destroyed on reboot and not waiting on disk for
> someone
> with rescue media to borrow.
>
> Pat

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

On Tue, 7 Feb 2023, Patrick Riehecky wrote:

>I've posted the patch as a PR up at
>https://github.com/openssh/openssh-portable/pull/346

+ char ccname[PATH_MAX] = {0};

This is a double ouch. This allocates at least 1 KiB, often
multiple times that, on the stack (big no-go) and even breaks
entirely in some circumstances (the filesystem’s is longer,
or the OS doesn’t have a fix PATH_MAX, in which case this is
a compile-time error). You’ll want pathconf(2) or, in this
case, probably asprintf(3) instead, though that’s not portable
so you’ll probably need to run it twice and allocate a buffer
in between (although you can get by with a small on-stack one
to avoid the allocation in the common small case).

Unsure if it’s safe to have create_private_runtime_directory
share one, persistent at that, directory between multiple ssh
sessions forever. Why don’t you always mkdtemp?

Should session_get_runtime_directory hardcode /tmp? Yes, the
code it replaces does that, but here’s a chance to honour
$TMPDIR, unless there’s good reasons not to.

If all callers of the new functions (session, sshpam…) to get
the runtime directory always use it in an asprintf-like or
strdup-like manner, then it might make sense to keep the value
around after first run instead of freeing it? But then, it’s
not used often enough so that’s worth that?

XDG_RUNTIME_DIR must also be validated to be an absolute path
and not used if it’s not, says the spec.

Just random thoughts from looking at this, nothing official,
//mirabilos
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev