Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Would you consider putting config in /etc? #74

Open
jafcobend opened this issue Sep 12, 2022 · 11 comments
Open

Would you consider putting config in /etc? #74

jafcobend opened this issue Sep 12, 2022 · 11 comments
Assignees

Comments

@jafcobend
Copy link

For my use I'm patching TWIN to place the system config files in /etc/twin as opposed to /usr/lib/twin. This seems more *NIX-like to me. I also moved the personal config files to ~/.config/twin. I suppose that's just personal taste. I'd like my home folder to be less messy. :-D I have tried to make this patch work with your auto* stuff. But since I don't use these tools, and rather dislike them, I have likely done it wrong.

Here's my patch:

diff --git a/Makefile.in b/Makefile.in
index f9f7ca4..82cc764 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -411,7 +411,7 @@ sbindir = @sbindir@
 sharedstatedir = @sharedstatedir@
 srcdir = @srcdir@
 sys_symbol_underscore = @sys_symbol_underscore@
-sysconfdir = @sysconfdir@
+sysconfdir = @sysconfdir@/twin
 target_alias = @target_alias@
 top_build_prefix = @top_build_prefix@
 top_builddir = @top_builddir@
@@ -458,10 +458,10 @@ $(ACLOCAL_M4): @MAINTAINER_MODE_TRUE@ $(am__aclocal_m4_deps)
 $(am__aclocal_m4_deps):
 install-pkglibexecSCRIPTS: $(pkglibexec_SCRIPTS)
 	@$(NORMAL_INSTALL)
-	@list='$(pkglibexec_SCRIPTS)'; test -n "$(pkglibexecdir)" || list=; \
+	@list='$(pkglibexec_SCRIPTS)'; test -n "$(sysconfdir)" || list=; \
 	if test -n "$$list"; then \
-	  echo " $(MKDIR_P) '$(DESTDIR)$(pkglibexecdir)'"; \
-	  $(MKDIR_P) "$(DESTDIR)$(pkglibexecdir)" || exit 1; \
+	  echo " $(MKDIR_P) '$(DESTDIR)$(sysconfdir)'"; \
+	  $(MKDIR_P) "$(DESTDIR)$(sysconfdir)" || exit 1; \
 	fi; \
 	for p in $$list; do \
 	  if test -f "$$p"; then d=; else d="$(srcdir)/"; fi; \
@@ -480,17 +480,17 @@ install-pkglibexecSCRIPTS: $(pkglibexec_SCRIPTS)
 	while read type dir files; do \
 	     if test "$$dir" = .; then dir=; else dir=/$$dir; fi; \
 	     test -z "$$files" || { \
-	       echo " $(INSTALL_SCRIPT) $$files '$(DESTDIR)$(pkglibexecdir)$$dir'"; \
-	       $(INSTALL_SCRIPT) $$files "$(DESTDIR)$(pkglibexecdir)$$dir" || exit $$?; \
+	       echo " $(INSTALL_SCRIPT) $$files '$(DESTDIR)$(sysconfdir)$$dir'"; \
+	       $(INSTALL_SCRIPT) $$files "$(DESTDIR)$(sysconfdir)$$dir" || exit $$?; \
 	     } \
 	; done
 
 uninstall-pkglibexecSCRIPTS:
 	@$(NORMAL_UNINSTALL)
-	@list='$(pkglibexec_SCRIPTS)'; test -n "$(pkglibexecdir)" || exit 0; \
+	@list='$(pkglibexec_SCRIPTS)'; test -n "$(sysconfdir)" || exit 0; \
 	files=`for p in $$list; do echo "$$p"; done | \
 	       sed -e 's,.*/,,;$(transform)'`; \
-	dir='$(DESTDIR)$(pkglibexecdir)'; $(am__uninstall_files_from_dir)
+	dir='$(DESTDIR)$(sysconfdir)'; $(am__uninstall_files_from_dir)
 
 mostlyclean-libtool:
 	-rm -f *.lo
diff --git a/server/Makefile.am b/server/Makefile.am
index 479bbc6..7a90cfc 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -14,7 +14,7 @@ if LIBTERM_la
   pkglib_LTLIBRARIES += libterm.la
 endif
 
-AM_CPPFLAGS           = -I$(top_srcdir)/include $(LTDLINCL) -DPKG_LIBDIR="\"$(pkglibdir)\""
+AM_CPPFLAGS           = -I$(top_srcdir)/include $(LTDLINCL) -DPKG_LIBDIR="\"$(pkglibdir)\"" -DPKG_CONFDIR="\"$(sysconfdir)\""
 twin_CPPFLAGS         = -I$(top_srcdir)/include $(LTDLINCL) -DBINDIR="\"$(bindir)\""
 
 twdisplay_SOURCES     = alloc.cpp display.cpp dl_helper.cpp missing.cpp hw.cpp
diff --git a/server/Makefile.in b/server/Makefile.in
index 32e5cb9..c21511d 100644
--- a/server/Makefile.in
+++ b/server/Makefile.in
@@ -526,7 +526,7 @@ top_srcdir = @top_srcdir@
 SUBDIRS = hw
 bin_SCRIPTS = twstart
 pkglib_LTLIBRARIES = $(am__append_1) $(am__append_2) $(am__append_3)
-AM_CPPFLAGS = -I$(top_srcdir)/include $(LTDLINCL) -DPKG_LIBDIR="\"$(pkglibdir)\""
+AM_CPPFLAGS = -I$(top_srcdir)/include $(LTDLINCL) -DPKG_LIBDIR="\"$(pkglibdir)\"" -DPKG_CONFDIR="\"$(sysconfdir)\""
 twin_CPPFLAGS = -I$(top_srcdir)/include $(LTDLINCL) -DBINDIR="\"$(bindir)\""
 twdisplay_SOURCES = alloc.cpp display.cpp dl_helper.cpp missing.cpp hw.cpp
 twin_SOURCES = wrapper.c
diff --git a/server/data.cpp b/server/data.cpp
index 920a73c..0bfa920 100644
--- a/server/data.cpp
+++ b/server/data.cpp
@@ -29,6 +29,14 @@
 
 const char *pkg_libdir = PKG_LIBDIR;
 
+/* TODO: replace with autoconf var */
+#ifndef PKG_CONFDIR
+#warning PKG_CONFDIR is not #defined, assuming "/usr/local/etc"
+#define PKG_CONFDIR "/usr/local/etc"
+#endif
+
+const char *pkg_confdir = PKG_CONFDIR "/twin";
+
 /* First, some structures */
 
 #define L 0x55
diff --git a/server/data.h b/server/data.h
index 0cda065..a092452 100644
--- a/server/data.h
+++ b/server/data.h
@@ -17,6 +17,7 @@ typedef struct s_rgb {
 } rgb;
 
 extern const char *pkg_libdir;
+extern const char *pkg_confdir;
 
 extern rgb Palette[tmaxcol + 1], defaultPalette[tmaxcol + 1];
 
diff --git a/server/rcparse.h b/server/rcparse.h
index 2bdd991..5ef7656 100644
--- a/server/rcparse.h
+++ b/server/rcparse.h
@@ -1421,7 +1421,7 @@ static byte rcload(void) {
 #endif
   byte c = tfalse;
 
-  if (!(path = FindFile(".twinrc", &len)))
+  if (!(path = FindFile("twinrc", &len)))
     return c;
 
   /*
diff --git a/server/util.cpp b/server/util.cpp
index 15ed6e3..015900b 100644
--- a/server/util.cpp
+++ b/server/util.cpp
@@ -1104,10 +1104,10 @@ byte SetServerUid(uldat uid, byte privileges) {
 }
 
 /*
- * search for a file relative to HOME, to PKG_LIBDIR or as path
+ * search for a file relative to HOME, to PKG_CONFDIR or as path
  *
  * this for example will search "foo"
- * as "${HOME}/foo", "${PKG_LIBDIR}/system.foo" or plain "foo"
+ * as "${HOME}/.foo", "${PKG_CONFDIR}/foo" or plain "foo"
  */
 char *FindFile(const char *name, uldat *fsize) {
   const char *prefix[3], *infix[3];
@@ -1117,14 +1117,14 @@ char *FindFile(const char *name, uldat *fsize) {
   struct stat buf;
 
   prefix[0] = HOME;
-  infix[0] = (HOME && *HOME) ? "/" : "";
-  prefix[1] = pkg_libdir;
-  infix[1] = "/system";
+  infix[0] = (HOME && *HOME) ? "/.config/twin/" : ".";
+  prefix[1] = pkg_confdir;
+  infix[1] = "/system.";
   prefix[2] = "";
   infix[2] = "";
 
   if (flag_secure)
-    min_i = max_i = 1; /* only pkg_libdir */
+    min_i = max_i = 1; /* only pkg_confdir */
   else
     min_i = 0, max_i = 2;
 
@@ -1199,7 +1199,7 @@ void RunTwEnvRC(void) {
   if (flag_secure == 0) {
     flag_envrc = 0;
 
-    if ((path = FindFile(".twenvrc.sh", &len))) {
+    if ((path = FindFile("twenvrc.sh", &len))) {
       if ((pipe(fds) >= 0)) {
         switch (fork()) {
         case -1: /* error */

With this patch you could place the config files in the "lib" directory with a configure command like:

./configure --prefix=/usr --sysconfdir=/usr/lib

But people I share this with always go, "Huh?!?!", when I tell them where to find the system-wide config files. And I'm fairly certain that Debian packaging politics (policy) demands they be placed in /etc. I'm sure other distros are just as opinionated.

I can't say I've tested the uninstall since *NO* program gets installed on my system without being placed in a .deb first. So if I uninstall it I let dpkg do the clean up. Hence, I never run make uninstall.

@cosmos72
Copy link
Owner

cosmos72 commented Sep 12, 2022

Yes, placing system-wide configuration files in /usr/local/etc or /etc makes sense.

Placing per-user configuration files in ~/.config/twin make sense too.

I will have a look at your patch, and check whether it's complete or needs some adjustments

@jafcobend
Copy link
Author

@cosmos72, since were on the subject of config files: is there really any point of not starting the socket service by default? I'm in and out of servers all day starting TWIN sessions on a regular basis. It really is a nuisance to have to remember to turn on the socket server every time.

I know there are likely to be very specific security instances where you might want to limit one's self to only the built in twterm to prevent someone else from pounding your TWIN socket. But it would seem in the majority of situations you want the socket server started so you can have ready access to whichever TWIN apps you have available. I'm usually launching many TWIN commands, which at the moment are mostly limited to twsysmon and twterm -e with specific commands I want in other windows. Like multiple editors, or man, or less, or tail, or watch, or ....

More than one app / window... is after all, what TWIN is all about. ;-)

@cosmos72
Copy link
Owner

cosmos72 commented Sep 15, 2022

As you correctly point out, not starting the socket service by default is a security feature.

I agree that starting it manually every time is annoying and time consuming:
you can change the default, and start socket service automatically,
by copying the file system.twinrc to ~/.twinrc and adding the line

Module "socket" On

Anyway, I am not inclined to change the socket service behaviour from "off by default" to "on by default":
the default behavior is the secure one, and can be changed easily if one wants.

@jafcobend
Copy link
Author

Here's the thing: When was the last time you started X without a socket? You can't, and for good reason. Its useless without some way for clients to talk to it. TWIN is in a little better shape because twterm is built in. So you can kind of use it without the socket. The thing is that there is never a time I leave it off. Especially if you want an app eco system the socket server needs to be started to use them.

I won't belabor the point since I can simply change the default in my build and even if I had to hack the source I would enable the socket on by default. I think socket security deserves some rethinking. I'll have to delve through the code to see what the whole situation is. You do have to have the token to be able to do anything with it.

Anyhow I have a bundle of ideas if you want to hash it out in another issue or some other form of communication.

@cosmos72
Copy link
Owner

cosmos72 commented Sep 15, 2022

Hi @jafcobend,
do you mean you prefer modifying the sources and enable the socket service by default,
rather than creating your own ~/.twinrc and modify a single line to achieve the same effect?
Then I must admit I'm really surprised. Are you willing to explain why your preference?

About the security implications of enabling the socket service (with any mechanism: modifying the sources, from ~/.twinrc, manually from menu):
the code parsing incoming TCP requests has not been audited for vulnerabilites, so there may be bugs and unexpected corner cases - actually none of twin sources have been audited.
That's why I wouldn't expose it on the internet (which may happen if it's enabled by default): I'd never risk exposing a server that (no matter if the probability is low) is not audited for security thus may contain bugs, possibly even remote execution bugs.

This is only partially mitigated by the fact that the initial surface attack area is quite small: an attacker would first need to successfully guess the content of user's ~/.TwinAuth (or somehow bypass such check - again possible only if there are bugs).

@jafcobend
Copy link
Author

... I wouldn't expose it on the internet...

I agree whole heartedly! I don't care if TWIN were audited. There is no good reason to expose it to the internet! If someone has one... then they need to rethink things. :-D Ok... maybe a server that does nothing but hosts demo TWIN sessions for people to play with and has no data of value with plenty of warning "use at your own risk!" But that seems pretty far fetched. ;-)

For context to answer your questions, I currently have three basic use cases that I'm using TWIN in:

  1. As an Uber replacement of screen and tmux on all the web servers I manage. I love the freedom it gives me. It almost fulfills my dream for a remote-management environment.
  2. Personal computing devices where I don't want to hassle with Xorg or give up the storage space and I don't have any particular need for graphics. I'm quite happy with text buttons. I don't need 3D rendered graphics. Although the whole GUI world seems to be going backward and creating widgets with such a complete lack of style there is no visual benefit to them... other than rounded corners. In this case TWIN answers my needs by allowing me to have multiple console/TWIN apps running visually side-by-side or in any other arrangement I prefer. I don't have to keep flipping VTs and wishing I could use that wasted space on each.
  3. As a daemon management environment of sorts on the various PIs I use (Raspberry, Orange, Nano, ClockWork, ...). Many of these are headless. I SSH in to them like in 1 (above). My various scripts/programs and status tools then get run their entire lives in twterms, which then becomes my volatile log storage. :-) Many benefits to that. But I also have simple visual displays that are produced by their operation that are well suited to terminals.

In all of those situations, never is TWIN exposed to the internet. As it should always be, there are firewalls that prevent the accidental exposure of services. And I, personally, can never see a time I'd directly expose TWIN to the iNet. So, I'm not concerned about that angle.

What I will do is simply modify my package so system.twinrc defaults the socket server on. So when I do the next round of updates across ALL of the machines I manage it will be done and I won't have to keep making copies and editing personal configs. Frankly I'll likely be the only one using TWIN on all these machines, even though I may be running as any number of different users.

But I understand the concern of an unknowing user getting TWIN, being attached unprotected to the iNet (that was painful just to write), and starting TWIN with the socket server enabled. But then I would say that in that scenario that if the user doesn't give up on TWIN, not seeing the potential, but instead they explore it, they WILL start the socket server unprotected!

My suggestion in that case would be to bind the listening socket to lo (127.0.0.1 / ::1). No exposure to the iNet and yet still functional. I keep hoping I'll have a bunch of TWIN apps one of these days, all needing sockets. :-D

Frankly my preference would be to not to start an IP (4/6) socket at all. But to use a UNIX domain socket in the user's home directory. I HATE things putting personal (user specific) temp files in /tmp! Then have an IP socket as an option. I don't see ever using that feature... But I do allow X to talk across my private net from time to time. So you never know. :-D

@cosmos72
Copy link
Owner

cosmos72 commented Sep 16, 2022

Then I hope you'll agree that making it easy to accidentally expose twin socket service to the Internet is bad too.

The socket service actually creates (and listens on) two sockets:

  • an IPv4 TCP socket on port 7754 + N,
  • a unix domain socket at /tmp/.Twin:N

The latter has rwx permissions only for the user that created it, but permissions on sockets are not checked by all operating systems: Linux respects them, while IIRC the various *BSD ignore them.
Your proposal to put a unix domain socket in user's home directory (for example in ~/.config/twin/) may help in this regard.

Clients already use the unix domain socket by default, and resort to TCP socket only if $TWDISPLAY is set to HOSTNAME_OR_IPADDRESS:N - this behavior was copied verbatim from $DISPLAY.
So I don't see much benefit in listening on 127.0.0.1 by default: in such case you can just use the unix domain socket instead.

Maybe a reasonable compromise is to enable the socket service by default, but only listen on the unix domain socket in user's home directory?
The TCP socket may then be enabled manually, or by editing ~/.twinrc (or as you proposed, something like ~/.config/twin/twinrc)

@dimitrik-fr
Copy link

To be honest, I'm seeing enabling socket access via config file is the only right way and agree with all security points ! -- e.g. enabling socket should be an "understood" step by a final user, and not something enabled by default of what user will even not be aware..

Personally I'm always using socket module as all my servers and VM are running in the well protected and isolated LAB env. -- so I can always deploy my "custom" TWIN config files on all systems. But when I just started to use TWIN there were many things yet to discover, and having good secure defaults from the start was a big advantage.

just my 2c

Rgds, -Dimitri

@jafcobend
Copy link
Author

@cosmos72 & @dimitrik-fr

I'm going to add my nickel's (2c+2c+1c ;-) ) worth here and answer both of you:

Maybe a reasonable compromise is to enable the socket service by default, but only listen on the unix domain socket in user's home directory?
The TCP socket may then be enabled manually, or by editing ~/.twinrc (or as you proposed, something like ~/.config/twin/twinrc)

I agree leaving the TCP service off is the most secure. @cosmos72 I think that idea is a winner! I did not know (and wondered) what those /tmp sockets were getting used for. So its good to know how that works.

The thing I love about UNIX sockets is that, at least in Linux, there are native permissions involved:

  1. You can't access them outside the machine.
  2. You can restrict who has access to them via FS permissions.

I routinely use them when I want to keep a communication channel secure on a multi-user box. Anybody on the box can access an IP socket. But not the UNIX socket.

It seems like maybe the thing to do with the socket is to put it in a protected directory. You could use something like ~/.twin or ~/.config/twin but I'd prefer to go one more level down to something like ~/.config/twin/run. You could even make it .run. If you check and set permissions on that folder you can make sure it remains 0700. If a program can't get to the socket it can't use it. So only that user would be able to use their UNIX socket(s).

I would put the sockets in their own folder under the user's TWIN config folder so that things remain tidy. Hiding it makes it even less likely to be a bother or get noticed. Since your include facility also looks in the TWIN config folder for the included files this leaves the main TWIN directory free for whatever TWINisms the user wants to setup. And you won't accidentally include a socket!

I suppose the only real issue with personal temp folders, like stashing sockets in the home folder, is that at boot they aren't cleared. Well... at least not since Mandrake. So in a crash/reboot scenario the old sockets will remain.

As for TCP service maybe the only way to turn it on should be by specifying the address or interface to bind to in the twinrc. Someone could always set it to 0.0.0.0 to make it universally accessible. Or if for some reason they only want to share their TWIN session with others on the same host then 127.0.0.1 | ::1 | lo.

Making this a config file edit addresses @dimitrik-fr's scenario. A user has to know something about what they are doing before they expose themselves to the iNet. Yet we still have a fully functional TWIN environment before then. This is good for new users, since they won't do something like attempt to start twsysmon or twkalc see an error or nothing happening and give up on the project because its broke or not useful.

Frankly I will most likely never enable TCP on my TWIN sessions. I seldom use that ability of X. But then my time in TWIN-land is still relatively new. TWIN over the wire is more functional than X (less bandwidth). So I may change my mind later. But I think something needs to change with the tokens before then.... For now its access through SSH and --hw=tty.

@cosmos72
Copy link
Owner

cosmos72 commented Oct 2, 2022

I ended up writing a conceptually analogous but more extensive patch - see commit a90a276:

Move configuration files to XDG-compliant paths:

    1. Load configuration file twinrc from XDG-compliant path
    ~/.config/twin/twinrc instead of old path ~/.twinrc.

    2. If still present, the file ~/.twinrc is automatically moved
    to the new location ~/.config/twin/twinrc when twin_server starts.

    3. Rename system.twinrc -> twinrc and system.twenvrc.sh -> twenvrc.sh
    and install them in $(sysconfdir)/twin instead of $(libdir)/twin
    i.e. usually in /usr/local/etc/twin (depends on sysconfdir set by ./configure)
    instead of /usr/local/lib/twin (depends on libdir set by ./configure)

If that's what @jafcobend intended, I'll close this issue and move the discussion about sockets to a separate issue.

@jafcobend
Copy link
Author

@cosmos72 yes, that's brilliant. I was lazy and didn't fight the names and since I don't have any TWINs installed with the confs in the libdir I didn't need to worry about upgrades. :-) But bonus points to you for doing so!

One more thought on the conf file location: How about allowing it to be specified from the CLI? That's pretty typical and I'll be wanting to have specific TWIN sessions where specific things get loaded each time the session starts. But I'll still want to be able to start an empty session (default config).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants