tech@mandoc.bsd.lv
 help / color / mirror / Atom feed
From: sternenseemann <sternenseemann@systemli.org>
To: tech@mandoc.bsd.lv
Subject: [PATCH v2 makewhatis] refactor HOMEBREWDIR into READ_ALLOWED_PATH
Date: Sat, 10 Apr 2021 17:57:37 +0200	[thread overview]
Message-ID: <3d224309-ff55-6354-8996-2c0e2b38f7a6@systemli.org> (raw)
In-Reply-To: <20210331173434.GA57338@athene.usta.de>

Hi Ingo,

I went ahead and implemented your suggestion of READ_ALLOWED_PATH. It is
mostly your code with the const char * assignment fixed and a tweak to
the parser, so it doesn't allow every realpath if READ_ALLOWED_PATH has
a leading or trailing ':'.

The previous checks on basedir and HOMEBREWDIR if defined have been
refactored into a function called read_allowed which also tracks the
length of the allowed dir which allows us to at least strip the prefix
from READ_ALLOWED_PATHS in filescan (this wasn't done previously for
HOMEBREWDIR).

HOMEBREWDIR is still supported as a configuration option, but we just
append it to READ_ALLOWED_PATHS in configure. The variable expansion
shenanigans in there are not strictly necessary as the parser would also
support leading colons, but since it works in ksh as well, there seems
to be no harm in having a clean READ_ALLOWED_PATH.

To indicate deprecation, I've removed HOMEBREWDIR from
configure.local.example.

I tested the following cases:

* "/nix/store"
* ":/nix/store"
* "/nix/store:"
* "/nix/store:/usr/local/Cellar"
* "/nix/store:/usr/local/Cellar:"
* ":/nix/store:/usr/local/Cellar"
* "/nix/store:/usr/local/Cellar:/gnu/store"

with a few example paths. Also I've rebuild my system with the new patch
and it works as expected. Did some sanity checks with makewhatis -n as well.

Cheers,

Lukas

PS: Still not sure about the naming, but I don't really care.

Index: configure
===================================================================
RCS file: /cvs/mandoc/configure,v
retrieving revision 1.77
diff -r1.77 configure
464c464,469
< [ -n "${HOMEBREWDIR}" ] && echo "#define HOMEBREWDIR \"${HOMEBREWDIR}\""
---
> if [ -n "${HOMEBREWDIR}" ]; then
>   # support deprecated configuration variable HOMEBREWDIR
>   # by appending it to READ_ALLOWED_PATH
>
READ_ALLOWED_PATH="${READ_ALLOWED_PATH:+$READ_ALLOWED_PATH:}${HOMEBREWDIR}"
> fi
> [ -n "${READ_ALLOWED_PATH}" ] && echo "#define READ_ALLOWED_PATH
\"${READ_ALLOWED_PATH}\""
Index: configure.local.example
===================================================================
RCS file: /cvs/mandoc/configure.local.example,v
retrieving revision 1.39
diff -r1.39 configure.local.example
214,216c214,220
< # into the manual trees.  To allow mandoc to follow such symlinks,
< # you have to specify the physical location of the cellar as returned
< # by realpath(3), for example:
---
> # into the manual trees.  A similar situation arises on Linux
> # distribution such as NixOS and Guix where all man pages are in a
> # so-called “store” directory which are then symlinked into the man
> # basedir. To allow mandoc to follow such symlinks, you have to specify
> # the physical location of the cellar / store directory as returned by
> # realpath(3) like in the following example. You can specify multiple
> # locations by separating them with colons.
219c223
< HOMEBREWDIR="${PREFIX}/Cellar"
---
> READ_ALLOWED_PATH="/nix/store:/gnu/store:${PREFIX}/Cellar"
Index: mandocdb.c
===================================================================
RCS file: /cvs/mandoc/mandocdb.c,v
retrieving revision 1.267
diff -r1.267 mandocdb.c
167a168
> static	ssize_t	 read_allowed(char *);
614,618c615
< 			if (strncmp(buf, basedir, basedir_len) != 0
< #ifdef HOMEBREWDIR
< 			    && strncmp(buf, HOMEBREWDIR, strlen(HOMEBREWDIR))
< #endif
< 			) {
---
> 			if (read_allowed(buf) == -1) {
788a786
> 	ssize_t		 prefix_len;
824,829c822,823
< 	else if (strncmp(usefile, basedir, basedir_len) == 0)
< 		start = usefile + basedir_len;
< #ifdef HOMEBREWDIR
< 	else if (strncmp(usefile, HOMEBREWDIR, strlen(HOMEBREWDIR)) == 0)
< 		start = usefile;
< #endif
---
> 	else if ((prefix_len = read_allowed(usefile)) != -1)
> 		start = usefile + prefix_len;
1947a1942,1980
> }
>
> /*
>  * Checks if we may read from a given realpath when
>  * constructing a database. This checks if the given
>  * path is in the current set basedir or any directory
>  * in READ_ALLOWED_PATH if it is defined.
>  *
>  * Returns -1 if reading is not allowed, the length
>  * of the allowed directory part of the realpath if
>  * reading is allowed. Note that stripping a prefix of
>  * this length is only guaranteed to be a man dir if
>  * the file is in basedir.
>  */
> static ssize_t
> read_allowed(char *realpath)
> {
> 	// if we have no basedir, don't check
> 	if(basedir_len == 0 || basedir == NULL || *basedir == '\0')
> 		return basedir_len;
>
> 	if(strncmp(realpath, basedir, basedir_len) == 0)
> 		return basedir_len;
>
> #ifdef READ_ALLOWED_PATH
> 	const char *pb = READ_ALLOWED_PATH;
>
> 	while (*pb != '\0') {
> 		size_t len = strcspn(pb, ":");
>
> 		if (len > 0 && strncmp(realpath, pb, len) == 0)
> 			return len;
>
> 		pb += len;
> 		pb += strspn(pb, ":");
> 	}
> #endif
>
> 	return -1;

--
 To unsubscribe send an email to tech+unsubscribe@mandoc.bsd.lv


  parent reply	other threads:[~2021-04-10 15:57 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-30 19:49 [PATCH makewhatis] add NIXSTOREDIR configuration variable to support NixOS sternenseemann
2021-03-30 20:30 ` Ingo Schwarze
2021-03-30 21:03   ` sternenseemann
2021-03-31 17:34     ` Ingo Schwarze
2021-03-31 19:33       ` sternenseemann
2021-04-10 15:57       ` sternenseemann [this message]
2021-04-10 16:15         ` [PATCH v2 makewhatis] refactor HOMEBREWDIR into READ_ALLOWED_PATH sternenseemann
2021-08-03 16:42           ` sternenseemann
2021-08-05 12:35             ` Ingo Schwarze
2021-08-07  0:29               ` sternenseemann
2021-08-07 13:17                 ` Ingo Schwarze

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3d224309-ff55-6354-8996-2c0e2b38f7a6@systemli.org \
    --to=sternenseemann@systemli.org \
    --cc=tech@mandoc.bsd.lv \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).