Provide secure_getenv on Linux where possible

[benesch]

In order to source ~/.editrc, libedit looks up the value of the HOME environment variable using secure_getenv. Previously, because HAVE_SECURE_GETENV was not defined when compiling libedit on Linux, libedit would be unable to lookup HOME and give up on sourcing ~/.editrc.

Defining HAVE_SECURE_GETENV fixes the problem, but only with glibc. Musl does not provide a secure_getenv function. Work around that by providing a shim implementation of secure_getenv for musl that uses the issetugid function instead.

Compilation against a non-glibc library that does not provide issetugid will fail. I don't think it's possible to fix this (i.e., compiling but refusing to lookup HOME at runtime) while continuing to compile libedit with cgo, though.

Fix #3.

[maddyblue]

What have we done to ourselves? I just want to delete a word.

[knz]

@benesch can you check the CI failures?

I wonder if it would be possible to use a syscall that retrieves info from the user db directly? I recall there was something like that on linux.

[benesch]

CI failures are fixed! Even though glibc 2.17+ includes the __secure_getenv symbol, it's versioned and unavailable for linking fresh programs. So just needed to condition our rewriting of secure_getenv on the glibc version.

[benesch]

@mjibson but just look at the 15k lines of line-editing code we didn't have to write!

[~/go/src/github.com/knz/go-libedit/unix/src/c-libedit]
benesch@bonobo$ cloc .
      55 text files.
      55 unique files.                              
       0 files ignored.

github.com/AlDanial/cloc v 1.76  T=1.04 s (53.1 files/s, 21669.4 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
C                               29           2362           3301          13611
C/C++ Header                    26            357           1008           1816
-------------------------------------------------------------------------------
SUM:                            55           2719           4309          15427
-------------------------------------------------------------------------------

[knz]

Not to mention the many more thousands that go in termcap/terminfo interfaces. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[maddyblue]

I meant "we" there as "computer people". I'm happy that we (cockroach) can finally get a good line editor. But sometimes it feels like we (computer people) have outsmarted ourselves.

[knz]

@benesch can you check how far you can get with getpwnam() across platforms instead of a Linux specific solution.

http://man7.org/linux/man-pages/man3/getpwnam.3.html

https://www.freebsd.org/cgi/man.cgi?query=getpwnam&sektion=3&manpath=freebsd-release-ports -- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[benesch]

@knz I'm a bit confused. We only compile libedit from source on Linux, so ISTM we only need a solution that works on Linux. Regardless if we do start compiling for other platforms the shim version of secure_getenv in this patch is relatively portable, as issetugid seems to exist on all the BSDs I've looked at.

[knz]

1) There are calls to getenv() in several places for TERM including the common c_editline.c. Don't these also need to use secure_getenv?

2) here's a trick on how to provide secure_getenv only if the c library does not expose it:

• create a separate library archive libsg.a containing just secure_getenv
• give the link flags -lc -lsg -lc. If secure_getenv is already in libc, the libsg.a will be ignored by the linker. If it is not, the libsg will be picked up (and you need a last -lc to providegetenv that libsg depends on)

Would that work?

[benesch]

Oh now that’s an interesting idea! Let me try that. I attempted to do the same using weak symbols but weak symbols only work with static linking.

Is looking up TERM security-sensitive? HOME is because it determines a file that gets read, but I doesn’t think that TERM does.

On Sun, Apr 15, 2018 at 12:26 PM kena notifications@github.com wrote:

1.

There are calls to getenv() in several places for TERM including the common c_editline.c. Don't these also need to use secure_getenv? 2.

here's a trick on how to provide secure_getenv only if the c library does not expose it:

• create a separate library archive libsg.a containing just secure_getenv
• give the link flags -lc -lsg -lc. If secure_getenv is already in libc, the libsg.a will be ignored by the linker. If it is not, the libsg will be picked up (and you need a last -lc to providegetenv that libsg depends on)

Would that work?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/knz/go-libedit/pull/9#issuecomment-381419083, or mute the thread https://github.com/notifications/unsubscribe-auth/AA15IK0GSWr7AVReSCMgguKFp6WkG0_Nks5to3S0gaJpZM4TUqoc .

[knz]

$TERM determines the termcap/terminfo entry that needs to be read. Does that count?

[benesch]

I don't know!

[knz]

After some thinking I think it's OK to skip secure_getenv for $TERM. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[knz]

Ok this PR is not needed, the badness is really elsewhere. Will fix.

[knz]

Superseded by #11.