man.cgi(8): add unique HTML titles
Here's a proposal to add unique HTML titles to man-pages served using
man.cgi. The name of the man-page is used as a title prefix (inspired by
NetBSD's adoption of mandoc).
There might be a more elegant way to produce the title given the
filename.
Index: cgi.c
===
RCS file: /cvs/src/usr.bin/mandoc/cgi.c,v
retrieving revision 1.85
diff -u -p -r1.85 cgi.c
--- cgi.c 25 Jan 2017 03:19:56 - 1.85
+++ cgi.c 5 Feb 2017 09:20:03 -
@@ -68,6 +68,7 @@ staticint http_decode(char *);
static void parse_manpath_conf(struct req *);
static void parse_path_info(struct req *req, const char *path);
static void parse_query_string(struct req *, const char *);
+static char*path_to_title(const char *);
static void pg_error_badrequest(const char *);
static void pg_error_internal(void);
static void pg_index(const struct req *);
@@ -76,7 +77,7 @@ staticvoid pg_search(const struct req
static void pg_searchres(const struct req *,
struct manpage *, size_t);
static void pg_show(struct req *, const char *);
-static void resp_begin_html(int, const char *);
+static void resp_begin_html(int, const char *, const char *);
static void resp_begin_http(int, const char *);
static void resp_catman(const struct req *, const char *);
static void resp_copy(const char *);
@@ -127,6 +128,36 @@ static const char *const arch_names[] =
static const int arch_MAX = sizeof(arch_names) / sizeof(char *);
/*
+ * Returns path transformed to a representation suitable for use as a HTML
+ * title.
+ * Example: "man1/man.1" -> "man(1)"
+ */
+static char *
+path_to_title(const char *path)
+{
+ char*e, *s, *title;
+ ssize_t len;
+
+ if ((s = strrchr(path, '/')) == NULL)
+ return NULL;
+ s++;
+
+ if ((e = strrchr(s, '.')) == NULL)
+ return NULL;
+ e++;
+
+ len = strlen(s) + 2;/* right parens and nul */
+ title = mandoc_malloc(len);
+ if (snprintf(title, len, "%.*s(%s)", (int)(e - s - 1), s, e) >= len) {
+ warnx("title buffer too small");
+ free(title);
+ return NULL;
+ }
+
+ return title;
+}
+
+/*
* Print a character, escaping HTML along the way.
* This will pass non-ASCII straight to output: be warned!
*/
@@ -341,8 +372,21 @@ resp_copy(const char *filename)
}
static void
-resp_begin_html(int code, const char *msg)
+resp_begin_html(int code, const char *msg, const char *title)
{
+ const char *delim = " - ";
+ char*prefix = NULL;
+ ssize_t len;
+
+ if (title != NULL) {
+ len = strlen(title) + strlen(delim) + 1;
+ prefix = mandoc_malloc(len);
+ if (snprintf(prefix, len, "%s%s", title, delim) >= len) {
+ warnx("prefix buffer too small");
+ free(prefix);
+ prefix = NULL;
+ }
+ }
resp_begin_http(code, msg);
@@ -352,12 +396,15 @@ resp_begin_html(int code, const char *ms
" \n"
" \n"
- " %s\n"
+ " %s%s\n"
"\n"
"\n",
- CSS_DIR, CUSTOMIZE_TITLE);
+ CSS_DIR, prefix != NULL ? prefix : "", CUSTOMIZE_TITLE);
resp_copy(MAN_DIR "/header.html");
+
+ if (prefix != NULL)
+ free(prefix);
}
static void
@@ -488,7 +535,7 @@ static void
pg_index(const struct req *req)
{
- resp_begin_html(200, NULL);
+ resp_begin_html(200, NULL, NULL);
resp_searchform(req, FOCUS_QUERY);
printf("\n"
"This web interface is documented in the\n"
@@ -505,7 +552,7 @@ pg_index(const struct req *req)
static void
pg_noresult(const struct req *req, const char *msg)
{
- resp_begin_html(200, NULL);
+ resp_begin_html(200, NULL, NULL);
resp_searchform(req, FOCUS_QUERY);
puts("");
puts(msg);
@@ -517,7 +564,7 @@ static void
pg_error_badrequest(const char *msg)
{
- resp_begin_html(400, "Bad Request");
+ resp_begin_html(400, "Bad Request", NULL);
puts("Bad Request\n"
"\n");
puts(msg);
@@ -530,7 +577,7 @@ pg_error_badrequest(const char *msg)
static void
pg_error_internal(void)
{
- resp_begin_html(500, "Internal Server Error");
+ resp_begin_html(500, "Internal Server Error", NULL);
puts("Internal Server Error");
resp_end_html();
}
@@ -569,7 +616,7 @@ pg_searchres(const struct req *req, stru
return;
}
- resp_begin_html(200, NULL);
+ resp_begin_html(200, NULL, NULL);
resp_searchform(req,
add empty /root/.ssh/authorized_keys to mtree/sets ?
Hi,
when installing 'throwaway' VMs (manually, not always using autoinstall for
$REASONS) i've often found myself having to do right after the install:
install -d -m 700 /root/.ssh
install -m 600 /dev/null /root/.ssh/authorized_keys
(or touch /root/.ssh/authorized_keys && chmod 600
/root/.ssh/authorized_keys, ymmv)
those are present in /etc/skel for "real" users, so why not creating
them for the root account ? install.sub also creates /mnt/root/.ssh when
using autoinstall and giving an ssh pubkey, so that'll be one less step
to do there.
We advise ppl to set prohibit-password for PermitRootLogin, so why not make it
easier to use it ? This ways, the correct modes are set.. i often fat-fingered
this, to see sshd complaining (rightly!) about bad modes on
.ssh/authorized_keys.
Conceptual (untested) diff below for discussion, i'll build a release with it
depending on the feedback/opinions..
Landry
Index: Makefile
===
RCS file: /cvs/src/etc/Makefile,v
retrieving revision 1.449
diff -u -r1.449 Makefile
--- Makefile2 Feb 2017 21:35:05 - 1.449
+++ Makefile5 Feb 2017 09:34:58 -
@@ -110,6 +110,8 @@
${DESTDIR}/root/.Xdefaults; \
${INSTALL} -c -o root -g wheel -m 644 dot.cvsrc \
${DESTDIR}/root/.cvsrc; \
+ ${INSTALL} -c -o root -g wheel -m 600 /dev/null \
+ ${DESTDIR}/root/.ssh/authorized_keys
rm -f ${DESTDIR}/.cshrc ${DESTDIR}/.profile; \
${INSTALL} -c -o root -g wheel -m 644 dot.cshrc \
${DESTDIR}/.cshrc; \
Index: mtree/4.4BSD.dist
===
RCS file: /cvs/src/etc/mtree/4.4BSD.dist,v
retrieving revision 1.293
diff -u -r1.293 4.4BSD.dist
--- mtree/4.4BSD.dist 27 Dec 2016 09:17:52 - 1.293
+++ mtree/4.4BSD.dist 5 Feb 2017 09:34:58 -
@@ -118,6 +118,8 @@
mnt
..
root mode=0700
+.ssh uname=root mode=0700
+..
..
sbin
..
Re: add empty /root/.ssh/authorized_keys to mtree/sets ?
On Sun, Feb 05, 2017 at 10:46:41AM +0100, Landry Breuil wrote:
> Hi,
>
> when installing 'throwaway' VMs (manually, not always using autoinstall for
> $REASONS) i've often found myself having to do right after the install:
> install -d -m 700 /root/.ssh
> install -m 600 /dev/null /root/.ssh/authorized_keys
> (or touch /root/.ssh/authorized_keys && chmod 600
> /root/.ssh/authorized_keys, ymmv)
>
> those are present in /etc/skel for "real" users, so why not creating
> them for the root account ? install.sub also creates /mnt/root/.ssh when
> using autoinstall and giving an ssh pubkey, so that'll be one less step
> to do there.
>
> We advise ppl to set prohibit-password for PermitRootLogin, so why not make it
> easier to use it ? This ways, the correct modes are set.. i often fat-fingered
> this, to see sshd complaining (rightly!) about bad modes on
> .ssh/authorized_keys.
Conceptually I'd like this going in.
> Conceptual (untested) diff below for discussion, i'll build a release with it
> depending on the feedback/opinions..
>
> Landry
>
> Index: Makefile
> ===
> RCS file: /cvs/src/etc/Makefile,v
> retrieving revision 1.449
> diff -u -r1.449 Makefile
> --- Makefile 2 Feb 2017 21:35:05 - 1.449
> +++ Makefile 5 Feb 2017 09:34:58 -
> @@ -110,6 +110,8 @@
> ${DESTDIR}/root/.Xdefaults; \
> ${INSTALL} -c -o root -g wheel -m 644 dot.cvsrc \
> ${DESTDIR}/root/.cvsrc; \
> + ${INSTALL} -c -o root -g wheel -m 600 /dev/null \
> + ${DESTDIR}/root/.ssh/authorized_keys
> rm -f ${DESTDIR}/.cshrc ${DESTDIR}/.profile; \
> ${INSTALL} -c -o root -g wheel -m 644 dot.cshrc \
> ${DESTDIR}/.cshrc; \
> Index: mtree/4.4BSD.dist
> ===
> RCS file: /cvs/src/etc/mtree/4.4BSD.dist,v
> retrieving revision 1.293
> diff -u -r1.293 4.4BSD.dist
> --- mtree/4.4BSD.dist 27 Dec 2016 09:17:52 - 1.293
> +++ mtree/4.4BSD.dist 5 Feb 2017 09:34:58 -
> @@ -118,6 +118,8 @@
> mnt
> ..
> root mode=0700
> +.ssh uname=root mode=0700
> +..
> ..
> sbin
> ..
>
Re: specify curves via ecdhe statement in httpd.conf
On 02/05/17 07:41, Joel Sing wrote:
On Saturday 04 February 2017 15:51:02 Andreas Bartelt wrote:
On 02/04/17 05:26, Joel Sing wrote:
On Wednesday 01 February 2017 15:41:29 Andreas Bartelt wrote:
Hello,
after reading the LibreSSL accouncement from today, I assumed that
specifying ecdhe "auto" in /etc/httpd.conf would enable X25519, P-256
and P-384 on current.
This is correct.
I've noticed that "auto" enables only curves x25519 and P-256 (which is
what I'd want to use - but somehow unexpected with regard to the
announcement).
Why do you believe this is the case?
Tested with a build of today's current:
- httpd started with ecdhe "auto" in /etc/httpd.conf
- then trying to connect via openssl s_client with -groups P-384 option
doesn't negotiate a cipher suite.
However, specifying -groups P-256 works. I don't know how to specify
x25519 with OpenBSD's openssl s_client (it's not yet listed in openssl
ecparam -list_curves output) but SSL Labs successfully negotiates via
x25519 and P-256 (but not P-384). P-384 doesn't seem to be enabled with
"auto".
You can just specify X25519 as a group - it will not appear in `openssl
ecparam -list_curves' since it is not a standard EC curve.
thanks - I didn't notice that capitalization is important here. Maybe
x25519 and ecdh_x25519 [IANA] should also be accepted as valid names
[http://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml].
The definition of the curve itself is provided in RFC 7748 - RFCs for
some other listed curves (e.g., brainpool) are also only tagged as
Informational. What is missing with regard to X25519?
Another confusing test result:
- httpd started with ecdhe "secp384r1" (P-384)
- then trying to connect via openssl s_client with -groups P-384 option
also doesn't negotiate a cipher suite!
However, SSL Labs successfully connects to httpd and confirms support
for secp384r1.
Can you reproduce this?
No, it works correctly for me (OpenBSD -current, amd64).
With "tls ecdhe auto":
$ for group in X25519 P-256 P-384; do openssl s_client -connect localhost:443
-groups $group &1 | grep 'Server Temp Key:'; done
Server Temp Key: ECDH, X25519, 253 bits
Server Temp Key: ECDH, P-256, 256 bits
Server Temp Key: ECDH, P-384, 384 bits
With "tls ecdhe secp384r1":
$ for group in X25519 P-256 P-384; do openssl s_client -connect localhost:443
-groups $group &1 | grep 'Server Temp Key:'; done
Server Temp Key: ECDH, P-384, 384 bits
This is really weird. Although my test results for X25519 were similarly
confusing -- for simplicity, I'll provide some more results which were
restricted to explicitly testing secp384r1.
TLS config of httpd has been stripped down for simplicity:
tls {
key "/etc/ssl/private/server.key"
certificate "/etc/ssl/server.crt"
ecdhe "secp384r1"
}
Server side:
one the same amd64 -current box with the same certs:
- httpd with ecdhe "secp384r1"
- nginx with ssl_ecdh_curve secp384r1;
external site:
- www.openbsd.org
Client side (on amd64 -current):
- openssl s_client -connect :443 -servername
-groups secp384r1
- eopenssl s_client -connect :443 -servername
-curves secp384r1
- nc -vvv -c 443
The following combinations were tested:
server httpd with ecdhe "secp384r1" & server nginx with ssl_ecdh_curve
secp384r1; (identical results)
connect via openssl [secp384r1]: fails
connect via eopenssl [secp384r1]: fails
connect via nc: succeeds
connect via SSL Labs: succeeds
connect via firefox: succeeds
LibreSSL 2.4.2 based client from OpenBSD current ~06/2016 on some older
test laptop:
connect via openssl [-groups was not yet available]: succeeds
then installed OpenSSL 1.0.2h from ports:
connect via eopenssl [without -curves option]: succeeds!
connect via eopenssl [with -curves secp384r1]: fails!
connect to www.openbsd.org:443
connect via openssl [secp384r1]: succeeds
connect via eopenssl [secp384r1]: succeeds
connect via nc: succeeds
I can't make any sense out of this.
Error messages when failing against httpd:
> openssl s_client -connect :443 -servername
-groups secp384r1
CONNECTED(0003)
1225438578:error:14FFF410:SSL routines:SSL_internal:sslv3 alert
handshake failure:/usr/src/lib/libssl/ssl_pkt.c:1205:SSL alert number 40
1225438578:error:14FFF0E5:SSL routines:SSL_internal:ssl handshake
failure:/usr/src/lib/libssl/ssl_pkt.c:585:
---
no peer certificate available
---
No client certificate CA names sent
---
SSL handshake has read 7 bytes and written 0 bytes
---
New, (NONE), Cipher is (NONE)
Secure Renegotiation IS NOT supported
Compression: NONE
Expansion: NONE
No ALPN negotiated
SSL-Session:
Protocol : TLSv1.2
Cipher:
Session-ID:
Session-ID-ctx:
Master-Key:
Start Time: 1486287427
Timeout : 7200 (sec)
Verify return code: 0 (ok)
---
> eopenssl s_client -connect :443 -servername
-curves secp384r1:
Re: specify curves via ecdhe statement in httpd.conf
On 02/05/17 11:13, Andreas Bartelt wrote: ... The following combinations were tested: server httpd with ecdhe "secp384r1" & server nginx with ssl_ecdh_curve secp384r1; (identical results) connect via openssl [secp384r1]: fails connect via eopenssl [secp384r1]: fails replying to myself here... this is interesting: in case -groups or -curves secp384r1 is omitted on the client side, it also succeeds against httpd and nginx on current [with the server side enforcing secp384r1].
stop building arm64 kernels with -mcpu=cortex-a57
Ask for a generic armv8-a encoding rather than one based on and tuned
for cortex-a57.
Index: Makefile.arm64
===
RCS file: /cvs/src/sys/arch/arm64/conf/Makefile.arm64,v
retrieving revision 1.4
diff -u -p -r1.4 Makefile.arm64
--- Makefile.arm64 25 Jan 2017 11:15:07 - 1.4
+++ Makefile.arm64 5 Feb 2017 10:10:31 -
@@ -28,7 +28,7 @@ CWARNFLAGS= -Werror -Wall -Wimplicit-fun
-Wno-constant-conversion -Wno-address-of-packed-member \
-Wframe-larger-than=2047
-CMACHFLAGS=-mcpu=cortex-a57+nofp+nosimd \
+CMACHFLAGS=-march=armv8-a+nofp+nosimd \
-fno-omit-frame-pointer -mno-omit-leaf-frame-pointer \
-ffixed-x18
CMACHFLAGS+= -ffreestanding ${NOPIE_FLAGS}
Re: stop building arm64 kernels with -mcpu=cortex-a57
On Sun, Feb 05, 2017 at 09:57:13PM +1100, Jonathan Gray wrote:
> Ask for a generic armv8-a encoding rather than one based on and tuned
> for cortex-a57.
If that works for you, sure, ok patrick@.
>
> Index: Makefile.arm64
> ===
> RCS file: /cvs/src/sys/arch/arm64/conf/Makefile.arm64,v
> retrieving revision 1.4
> diff -u -p -r1.4 Makefile.arm64
> --- Makefile.arm6425 Jan 2017 11:15:07 - 1.4
> +++ Makefile.arm645 Feb 2017 10:10:31 -
> @@ -28,7 +28,7 @@ CWARNFLAGS= -Werror -Wall -Wimplicit-fun
> -Wno-constant-conversion -Wno-address-of-packed-member \
> -Wframe-larger-than=2047
>
> -CMACHFLAGS= -mcpu=cortex-a57+nofp+nosimd \
> +CMACHFLAGS= -march=armv8-a+nofp+nosimd \
> -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer \
> -ffixed-x18
> CMACHFLAGS+= -ffreestanding ${NOPIE_FLAGS}
>
Re: specify curves via ecdhe statement in httpd.conf
On Sunday 05 February 2017 11:13:16 Andreas Bartelt wrote: > On 02/05/17 07:41, Joel Sing wrote: > > You can just specify X25519 as a group - it will not appear in `openssl > > ecparam -list_curves' since it is not a standard EC curve. > > thanks - I didn't notice that capitalization is important here. Maybe > x25519 and ecdh_x25519 [IANA] should also be accepted as valid names > [http://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml]. I'll consider this, however all of the RFCs refer to it as X25519 (e.g. RFC7748 and draft-ietf-curdle-pkix-03). This is also no different to the fact that you cannot use 'Prime256v1' or 'p-384'. > The definition of the curve itself is provided in RFC 7748 - RFCs for > some other listed curves (e.g., brainpool) are also only tagged as > Informational. What is missing with regard to X25519? There is nothing missing as such - X25519 is a function that performs scalar multiplication on a curve known as curve25519. The X25519 function is in turn used to perform Diffie-Hellman key exchange. Neither X25519 nor curve25519 fit the OpenSSL Elliptic Curve API (largely due to design), hence it does not make sense for it to appear in 'openssl ecparam -list_curves', which would require it to be treated and manipulated as if it was a standard EC curve. > This is really weird. Although my test results for X25519 were similarly > confusing -- for simplicity, I'll provide some more results which were > restricted to explicitly testing secp384r1. [snip] > Error messages when failing against httpd: > > openssl s_client -connect :443 -servername > > -groups secp384r1 > CONNECTED(0003) > 1225438578:error:14FFF410:SSL routines:SSL_internal:sslv3 alert > handshake failure:/usr/src/lib/libssl/ssl_pkt.c:1205:SSL alert number 40 > 1225438578:error:14FFF0E5:SSL routines:SSL_internal:ssl handshake > failure:/usr/src/lib/libssl/ssl_pkt.c:585: This is the server-side responding with a fatal SSL handshake failure alert - there are only a few cases where this will happen, the most likely of which is when there is no matching cipher suite. - Is there any other configuration that would limit the cipher suites in use? - What cipher suite is used if you connect without specifying -groups? - What type of public certificate are you using (RSA or ECDSA)? - If you're still unable to get to the bottom of it, try running 'openssl s_client' with -debug and provide the output.
Re: specify curves via ecdhe statement in httpd.conf
On 02/05/17 15:41, Joel Sing wrote:
On Sunday 05 February 2017 11:13:16 Andreas Bartelt wrote:
On 02/05/17 07:41, Joel Sing wrote:
You can just specify X25519 as a group - it will not appear in `openssl
ecparam -list_curves' since it is not a standard EC curve.
thanks - I didn't notice that capitalization is important here. Maybe
x25519 and ecdh_x25519 [IANA] should also be accepted as valid names
[http://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml].
I'll consider this, however all of the RFCs refer to it as X25519 (e.g.
RFC7748 and draft-ietf-curdle-pkix-03). This is also no different to the fact
that you cannot use 'Prime256v1' or 'p-384'.
The definition of the curve itself is provided in RFC 7748 - RFCs for
some other listed curves (e.g., brainpool) are also only tagged as
Informational. What is missing with regard to X25519?
There is nothing missing as such - X25519 is a function that performs scalar
multiplication on a curve known as curve25519. The X25519 function is in turn
used to perform Diffie-Hellman key exchange. Neither X25519 nor curve25519 fit
the OpenSSL Elliptic Curve API (largely due to design), hence it does not make
sense for it to appear in 'openssl ecparam -list_curves', which would require
it to be treated and manipulated as if it was a standard EC curve.
thanks for the explanation.
This is really weird. Although my test results for X25519 were similarly
confusing -- for simplicity, I'll provide some more results which were
restricted to explicitly testing secp384r1.
[snip]
Error messages when failing against httpd:
> openssl s_client -connect :443 -servername
-groups secp384r1
CONNECTED(0003)
1225438578:error:14FFF410:SSL routines:SSL_internal:sslv3 alert
handshake failure:/usr/src/lib/libssl/ssl_pkt.c:1205:SSL alert number 40
1225438578:error:14FFF0E5:SSL routines:SSL_internal:ssl handshake
failure:/usr/src/lib/libssl/ssl_pkt.c:585:
This is the server-side responding with a fatal SSL handshake failure alert -
there are only a few cases where this will happen, the most likely of which is
when there is no matching cipher suite.
- Is there any other configuration that would limit the cipher suites in use?
I've tested the following tls configurations with httpd:
config 1)
tls {
key "/etc/ssl/private/server.key"
certificate "/etc/ssl/server.crt"
ecdhe "secp384r1"
ocsp "/etc/ssl/server_ocsp.der" # [manually fetched a
fresh one via ocspcheck]
}
config 2)
tls {
key "/etc/ssl/private/server.key"
certificate "/etc/ssl/server.crt"
ecdhe "secp384r1"
ciphers
"ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-ECDSA-AES256-SHA384"
ocsp "/etc/ssl/server_ocsp.der"
}
A fresh /etc/ssl/server_ocsp.der was manually fetched with ocspcheck.
- What cipher suite is used if you connect without specifying -groups?
config 1: ECDHE-ECDSA-AES256-GCM-SHA384
config 2: ECDHE-ECDSA-CHACHA20-POLY1305
- What type of public certificate are you using (RSA or ECDSA)?
ECDSA with P-256. Certificate signed by letsencrypt (via RSA).
Must-staple is enabled - that's why I'm also using the ocsp line for
testing.
I could verify that OCSP stapling is not the problem here since I've
also tested without the ocsp line in httpd.conf [and then using another
certificate without the encoded "must staple" extension]. Both results
were identical.
- If you're still unable to get to the bottom of it, try running 'openssl
s_client' with -debug and provide the output.
> openssl s_client -connect www.bartelt.name:443 -servername
www.bartelt.name -debug -groups secp384r1
CONNECTED(0003)
write to 0x59765414700 [0x59775e64003] (261 bytes => 261 (0x105))
- 16 03 01 01 00 01 00 00-fc 03 03 79 f3 5f 4c fe ...y._L.
0010 - 4f b8 30 11 07 81 ba cc-a4 1e 1b 21 da 3f da 0c O.0!.?..
0020 - 69 b8 f0 12 b9 33 83 75-ac 35 1d 00 00 7e c0 30 i3.u.5...~.0
0030 - c0 2c c0 28 c0 24 c0 14-c0 0a 00 a3 00 9f 00 6b .,.(.$.k
0040 - 00 6a 00 39 00 38 cc a9-cc a8 cc aa cc 14 cc 13 .j.9.8..
0050 - cc 15 ff 85 00 c4 00 c3-00 88 00 87 00 81 00 9d
0060 - 00 3d 00 35 00 c0 00 84-c0 2f c0 2b c0 27 c0 23 .=.5./.+.'.#
0070 - c0 13 c0 09 00 a2 00 9e-00 67 00 40 00 33 00 32 .g.@.3.2
0080 - 00 be 00 bd 00 45 00 44-00 9c 00 3c 00 2f 00 ba .E.D...<./..
0090 - 00 41 c0 11 c0 07 00 05-00 04 c0 12 c0 08 00 16 .A..
00a0 - 00 13 00 0a 00 15 00 12-00 09 00 ff 01 00 00 55 ...U
00b0 - 00 00 00 15 00 13 00 00-10 77 77 77 2e 62 61 72 .www.bar
00c0 - 74 65 6c 74 2e 6e 61 6d-65 00 0b 00 02 01 00 00 telt.name...
00d0 - 0a 00 04 00 02 00 18 00-23 00 00 00 0d 00 26 00 #.&.
00e0 - 24 06 01 06 02 06 03 ef-ef 05 01 05 02 05 03 04
Implement fork1_to_pid(). It's fork1(), but with pid as argument
init(8) is wanted to have process ID 1. It's also the only process
which is assigned non-random PID (well, there's also swapper as PID
0).
This patch renames fork1() to fork1_to_pid() and introduces new
argument "pid" which can be used to select PID for new process. When
pid is 0, random PID is assigned. fork1() is then wrapper to
fork1_to_pid() with pid argument being 0. No functional change in
fork1().
Only caller (outside fork1()) for fork1_to_pid() should be in kernel
main() to start the init(8) process.
With above changes it's possible to remove global variable "randompid"
which only purpose was to assign PID 1 to init(8). It's also possible
to remove static variable "lastpid" from allocpid().
Tested in amd64 by building the base and kernel, and run some of the
regress test set.
diff --git a/sys/kern/init_main.c b/sys/kern/init_main.c
index e21a8306854..20f92752921 100644
--- a/sys/kern/init_main.c
+++ b/sys/kern/init_main.c
@@ -437,14 +437,12 @@ main(void *framep)
{
struct proc *initproc;
- if (fork1(p, FORK_FORK, NULL, 0, start_init, NULL, NULL,
- &initproc))
+ if (fork1_to_pid(p, 1, FORK_FORK, NULL, 0, start_init, NULL,
+ NULL, &initproc))
panic("fork init");
initprocess = initproc->p_p;
}
- randompid = 1;
-
/*
* Create any kernel threads whose creation was deferred because
* initprocess had not yet been created.
diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index 3ff2085f732..b1d1d249ca7 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -71,7 +71,6 @@
intnprocesses = 1; /* process 0 */
intnthreads = 1; /* proc 0 */
-intrandompid; /* when set to 1, pid's go random */
struct forkstat forkstat;
void fork_return(void *);
@@ -80,7 +79,7 @@ pid_t alloctid(void);
pid_t allocpid(void);
int ispidtaken(pid_t);
-void process_new(struct proc *, struct process *, int);
+void process_new(struct proc *, struct process *, pid_t, int);
void
fork_return(void *arg)
@@ -176,7 +175,7 @@ process_initialize(struct process *pr, struct proc *p)
* Allocate and initialize a new process.
*/
void
-process_new(struct proc *p, struct process *parent, int flags)
+process_new(struct proc *p, struct process *parent, pid_t pid, int flags)
{
struct process *pr;
@@ -193,7 +192,7 @@ process_new(struct proc *p, struct process *parent, int
flags)
(caddr_t)&pr->ps_endcopy - (caddr_t)&pr->ps_startcopy);
process_initialize(pr, p);
- pr->ps_pid = allocpid();
+ pr->ps_pid = pid ? pid : allocpid();
/* post-copy fixups */
pr->ps_pptr = parent;
@@ -256,6 +255,15 @@ fork1(struct proc *curp, int flags, void *stack, pid_t
*tidptr,
void (*func)(void *), void *arg, register_t *retval,
struct proc **rnewprocp)
{
+ return (fork1_to_pid(curp, 0, flags, stack, tidptr, func, arg, retval,
+ rnewprocp));
+}
+
+int
+fork1_to_pid(struct proc *curp, pid_t pid, int flags, void *stack,
+pid_t *tidptr, void (*func)(void *), void *arg, register_t *retval,
+struct proc **rnewprocp)
+{
struct process *curpr = curp->p_p;
struct process *pr;
struct proc *p;
@@ -270,7 +278,8 @@ fork1(struct proc *curp, int flags, void *stack, pid_t
*tidptr,
if (flags & FORK_THREAD) {
if ((flags & FORK_SHAREFILES) == 0 ||
(flags & FORK_SIGHAND) == 0 ||
- (flags & FORK_SYSTEM) != 0)
+ (flags & FORK_SYSTEM) != 0 ||
+ pid != 0)
return (EINVAL);
}
if (flags & FORK_SIGHAND && (flags & FORK_SHAREVM) == 0)
@@ -362,7 +371,7 @@ fork1(struct proc *curp, int flags, void *stack, pid_t
*tidptr,
p->p_p = pr = curpr;
pr->ps_refcnt++;
} else {
- process_new(p, curpr, flags);
+ process_new(p, curpr, pid, flags);
pr = p->p_p;
}
p->p_fd = pr->ps_fd;
@@ -590,19 +599,12 @@ ispidtaken(pid_t pid)
pid_t
allocpid(void)
{
- static pid_t lastpid;
pid_t pid;
- if (!randompid) {
- /* only used early on for system processes */
- pid = ++lastpid;
- } else {
- /* Find an unused pid satisfying lastpid < pid <= PID_MAX */
- do {
- pid = arc4random_uniform(PID_MAX - lastpid) + 1 +
- lastpid;
- } while (ispidtaken(pid));
- }
+ /* Find an unused pid satisfying 1 < pid <= PID_MAX */
+ do {
+ pid = 2 + arc4random_uniform(PID_MAX - 1);
+ } while (ispidtaken(pid));
return pid;
}
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index 1a1f0966518..c2163e1ac27 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@
Re: add empty /root/.ssh/authorized_keys to mtree/sets ?
On 2017/02/05 09:53, Robert Peichaer wrote:
> On Sun, Feb 05, 2017 at 10:46:41AM +0100, Landry Breuil wrote:
> > Hi,
> >
> > when installing 'throwaway' VMs (manually, not always using autoinstall for
> > $REASONS) i've often found myself having to do right after the install:
> > install -d -m 700 /root/.ssh
> > install -m 600 /dev/null /root/.ssh/authorized_keys
> > (or touch /root/.ssh/authorized_keys && chmod 600
> > /root/.ssh/authorized_keys, ymmv)
> >
> > those are present in /etc/skel for "real" users, so why not creating
> > them for the root account ? install.sub also creates /mnt/root/.ssh when
> > using autoinstall and giving an ssh pubkey, so that'll be one less step
> > to do there.
> >
> > We advise ppl to set prohibit-password for PermitRootLogin, so why not make
> > it
> > easier to use it ? This ways, the correct modes are set.. i often
> > fat-fingered
> > this, to see sshd complaining (rightly!) about bad modes on
> > .ssh/authorized_keys.
>
> Conceptually I'd like this going in.
+1. (On "managed" systems I use root-owned authorized_keys in a system
directory,
but this doesn't get in the way, and it makes things easier on ad-hoc installed
systems).
> > Conceptual (untested) diff below for discussion, i'll build a release with
> > it
> > depending on the feedback/opinions..
> >
> > Landry
> >
> > Index: Makefile
> > ===
> > RCS file: /cvs/src/etc/Makefile,v
> > retrieving revision 1.449
> > diff -u -r1.449 Makefile
> > --- Makefile2 Feb 2017 21:35:05 - 1.449
> > +++ Makefile5 Feb 2017 09:34:58 -
> > @@ -110,6 +110,8 @@
> > ${DESTDIR}/root/.Xdefaults; \
> > ${INSTALL} -c -o root -g wheel -m 644 dot.cvsrc \
> > ${DESTDIR}/root/.cvsrc; \
> > + ${INSTALL} -c -o root -g wheel -m 600 /dev/null \
> > + ${DESTDIR}/root/.ssh/authorized_keys
> > rm -f ${DESTDIR}/.cshrc ${DESTDIR}/.profile; \
> > ${INSTALL} -c -o root -g wheel -m 644 dot.cshrc \
> > ${DESTDIR}/.cshrc; \
> > Index: mtree/4.4BSD.dist
> > ===
> > RCS file: /cvs/src/etc/mtree/4.4BSD.dist,v
> > retrieving revision 1.293
> > diff -u -r1.293 4.4BSD.dist
> > --- mtree/4.4BSD.dist 27 Dec 2016 09:17:52 - 1.293
> > +++ mtree/4.4BSD.dist 5 Feb 2017 09:34:58 -
> > @@ -118,6 +118,8 @@
> > mnt
> > ..
> > root mode=0700
> > +.ssh uname=root mode=0700
> > +..
> > ..
> > sbin
> > ..
> >
>
Re: Implement fork1_to_pid(). It's fork1(), but with pid as argument
On Sun, 5 Feb 2017, Ossi Herrala wrote: > init(8) is wanted to have process ID 1. It's also the only process which > is assigned non-random PID (well, there's also swapper as PID 0). > > This patch renames fork1() to fork1_to_pid() and introduces new argument > "pid" which can be used to select PID for new process. When pid is 0, > random PID is assigned. fork1() is then wrapper to fork1_to_pid() with > pid argument being 0. No functional change in fork1(). I wouldn't have a problem with this diff...except it adds a *ninth* argument to a function. fork1() is already really bad with eight arguments: we need to break up its uses and refactor it to be less confusing and overloaded and not add Yet Another Argument With Magic Value. Philip Guenther
Password corruption in adduser
Hi all, I've noticed something strange in adduser -- when attempting to add a user completely though command line argument it seems to corrupt the entry in /etc/master.passwd. Example: $ echo "HorseBatteryStaple" | encrypt $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 # adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh \ -message no -batch some.user "" "Some User" \ $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 Added user ``some.user'' # vipw ... some.user:b/bin/ksh9/9uoOrbTRaf//3ZprAb9k.hOpfe9vYVqjf1a:5000:5000:: \ 0:0:Some User:/home/some.user:/bin/ksh ... As you can see the password entry gets corrupted with a 'b/bin/ksh...' This behavior does not occur with -unencrypted. Behavior *is* present when hash is wrapped in " Take care, John
Re: Password corruption in adduser
On Sun, 5 Feb 2017, John McGuigan wrote: > I've noticed something strange in adduser -- when attempting to add a > user completely though command line argument it seems to corrupt the > entry in /etc/master.passwd. > > Example: > > $ echo "HorseBatteryStaple" | encrypt > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 > > # adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh \ > -message no -batch some.user "" "Some User" \ > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 > Added user ``some.user'' ... > some.user:b/bin/ksh9/9uoOrbTRaf//3ZprAb9k.hOpfe9vYVqjf1a:5000:5000:: \ > 0:0:Some User:/home/some.user:/bin/ksh > > As you can see the password entry gets corrupted with a 'b/bin/ksh...' Let's see what the adduser command is seeing by passing that all to 'echo' instead: # echo \ > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh \ > -message no -batch some.user "" "Some User" \ > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh -message no -batch some.user Some User b/bin/ksh9/FGXw.9oNjr3BLTS7DJp5n4M2 # Ah, so the expansion is happening *outside* of adduser...in the shell. Yes, the shell does variable expansion even if the dollar-sign is in the middle of a word, so it's expanding the variables $2 --> "" $0 --> "/bin/ksh" $ssZSLC6laHsTS7O2FwJ4Mufw6mSS --> "" > Behavior *is* present when hash is wrapped in " Sure, because double-quotes only stop file-globbing and field splitting and not variable expansion. You need single quotes for that: # echo \ > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh \ > -message no -batch some.user "" "Some User" \ > '$2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2' adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh -message no -batch some.user Some User $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 # Philip Guenther
Re: Password corruption in adduser
On Sun, Feb 05, 2017 at 09:47:35PM -0800, Philip Guenther wrote: > On Sun, 5 Feb 2017, John McGuigan wrote: > > I've noticed something strange in adduser -- when attempting to add a > > user completely though command line argument it seems to corrupt the > > entry in /etc/master.passwd. > > > > Example: > > > > $ echo "HorseBatteryStaple" | encrypt > > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 > > > > # adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh \ > > -message no -batch some.user "" "Some User" \ > > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 > > Added user ``some.user'' > ... > > some.user:b/bin/ksh9/9uoOrbTRaf//3ZprAb9k.hOpfe9vYVqjf1a:5000:5000:: \ > > 0:0:Some User:/home/some.user:/bin/ksh > > > > As you can see the password entry gets corrupted with a 'b/bin/ksh...' > > Let's see what the adduser command is seeing by passing that all to 'echo' > instead: > > # echo \ > > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh \ > > -message no -batch some.user "" "Some User" \ > > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh -message no > -batch some.user Some User b/bin/ksh9/FGXw.9oNjr3BLTS7DJp5n4M2 > # > > Ah, so the expansion is happening *outside* of adduser...in the shell. > Yes, the shell does variable expansion even if the dollar-sign is in the > middle of a word, so it's expanding the variables > $2 --> "" > $0 --> "/bin/ksh" > $ssZSLC6laHsTS7O2FwJ4Mufw6mSS --> "" > > > > Behavior *is* present when hash is wrapped in " > > Sure, because double-quotes only stop file-globbing and field splitting > and not variable expansion. You need single quotes for that: > > # echo \ > > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh \ > > -message no -batch some.user "" "Some User" \ > > '$2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2' > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh -message no > -batch some.user Some User > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 > # The adduser.8 manual page has an example with no quotes in it, so we should fix that. Also, let's use a new hash using $2b$ instead of $2a$. Index: adduser.8 === RCS file: /var/cvs/src/usr.sbin/adduser/adduser.8,v retrieving revision 1.44 diff -u -p -r1.44 adduser.8 --- adduser.8 24 Dec 2015 16:54:37 - 1.44 +++ adduser.8 6 Feb 2017 05:49:00 - @@ -373,7 +373,7 @@ The password has been created using .Xr encrypt 1 : .Bd -literal -offset indent # adduser -batch falken guest,staff,beer 'Prof. Falken' \e -$2a$06$1Sdjxjoxg4cNmT6zAxriGOLgdLXQ3HdJ2dKBbzEk68jSrO1EtLJ3C +'$2b$10$aOadQNznQ1YJFnqNaRRneOvYvZAEO7atYiTND3EsLf6afHT5t1UIK' .Ed .Pp Create user
Re: Password corruption in adduser
ok beck@ On Sun, Feb 5, 2017 at 22:53 Theo Buehler wrote: > On Sun, Feb 05, 2017 at 09:47:35PM -0800, Philip Guenther wrote: > > On Sun, 5 Feb 2017, John McGuigan wrote: > > > I've noticed something strange in adduser -- when attempting to add a > > > user completely though command line argument it seems to corrupt the > > > entry in /etc/master.passwd. > > > > > > Example: > > > > > > $ echo "HorseBatteryStaple" | encrypt > > > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 > > > > > > # adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh \ > > > -message no -batch some.user "" "Some User" \ > > > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 > > > Added user ``some.user'' > > ... > > > some.user:b/bin/ksh9/9uoOrbTRaf//3ZprAb9k.hOpfe9vYVqjf1a:5000:5000:: \ > > > 0:0:Some User:/home/some.user:/bin/ksh > > > > > > As you can see the password entry gets corrupted with a 'b/bin/ksh...' > > > > Let's see what the adduser command is seeing by passing that all to > 'echo' > > instead: > > > > # echo \ > > > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh \ > > > -message no -batch some.user "" "Some User" \ > > > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 > > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh > -message no -batch some.user Some User b/bin/ksh9/FGXw.9oNjr3BLTS7DJp5n4M2 > > # > > > > Ah, so the expansion is happening *outside* of adduser...in the shell. > > Yes, the shell does variable expansion even if the dollar-sign is in the > > middle of a word, so it's expanding the variables > > $2 --> "" > > $0 --> "/bin/ksh" > > $ssZSLC6laHsTS7O2FwJ4Mufw6mSS --> "" > > > > > > > Behavior *is* present when hash is wrapped in " > > > > Sure, because double-quotes only stop file-globbing and field splitting > > and not variable expansion. You need single quotes for that: > > > > # echo \ > > > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh \ > > > -message no -batch some.user "" "Some User" \ > > > '$2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2' > > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh > -message no -batch some.user Some User > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 > > # > > The adduser.8 manual page has an example with no quotes in it, so we > should fix that. Also, let's use a new hash using $2b$ instead of $2a$. > > Index: adduser.8 > === > RCS file: /var/cvs/src/usr.sbin/adduser/adduser.8,v > retrieving revision 1.44 > diff -u -p -r1.44 adduser.8 > --- adduser.8 24 Dec 2015 16:54:37 - 1.44 > +++ adduser.8 6 Feb 2017 05:49:00 - > @@ -373,7 +373,7 @@ The password has been created using > .Xr encrypt 1 : > .Bd -literal -offset indent > # adduser -batch falken guest,staff,beer 'Prof. Falken' \e > -$2a$06$1Sdjxjoxg4cNmT6zAxriGOLgdLXQ3HdJ2dKBbzEk68jSrO1EtLJ3C > +'$2b$10$aOadQNznQ1YJFnqNaRRneOvYvZAEO7atYiTND3EsLf6afHT5t1UIK' > .Ed > .Pp > Create user > >
Re: mount(8): some code shuffling to avoid a pledge problem
ping
On Sat, Jan 28, 2017 at 03:25:53PM +0100, Theo Buehler wrote:
> The problem:
>
> $ mount /dev/tty /tmp
> Abort trap (core dumped)
>
> The relevant kdump snippet:
>
> 45441 mountCALL open(0x7f7eb580,0x1)
> 45441 mountNAMI "/dev/tty"
> 45441 mountRET open 3
> 45441 mountCALL ioctl(3,DIOCGDINFO,0x7f7eb350)
> 45441 mountPLDG ioctl, "tty", errno 1 Operation not permitted
> 45441 mountPSIG SIGABRT SIG_DFL
>
> and the backtrace:
>
> #0 0x1103967a72aa in ioctl () at :2
> #1 0x110364ca97b9 in readlabelfs (device=0x7f7d0ada "/dev/tty",
> verbose=0) at /usr/src/lib/libutil/readlabel.c:128
> #2 0x1100d4201210 in main () from /usr/obj/sbin/mount/mount
>
> As usual, the fix consists of hoisting the problematic bit above the
> pledge call. This has the side effect that the string bashing functions
> hasopt(), catopt() and maketypelist() are now called without pledge from
> the getopt switch, but I don't think that's a problem. It would be easy
> to postpone the call to maketypelist() until after pledge, if anyone is
> worried about it.
>
> Index: mount.c
> ===
> RCS file: /var/cvs/src/sbin/mount/mount.c,v
> retrieving revision 1.70
> diff -u -p -r1.70 mount.c
> --- mount.c 25 Jan 2017 02:33:25 - 1.70
> +++ mount.c 28 Jan 2017 14:11:28 -
> @@ -109,9 +109,6 @@ main(int argc, char * const argv[])
> int all, ch, forceall, i, mntsize, rval, new;
> char *options, mntpath[PATH_MAX];
>
> - if (pledge("stdio rpath disklabel proc exec", NULL) == -1)
> - err(1, "pledge");
> -
> all = forceall = 0;
> options = NULL;
> vfstype = "ffs";
> @@ -168,6 +165,25 @@ main(int argc, char * const argv[])
> argc -= optind;
> argv += optind;
>
> + if (typelist == NULL && argc == 2) {
> + /*
> + * If -t flag has not been specified, and spec contains either
> + * a ':' or a '@' then assume that an NFS filesystem is being
> + * specified ala Sun. If not, check the disklabel for a
> + * known filesystem type.
> + */
> + if (strpbrk(argv[0], ":@") != NULL)
> + vfstype = "nfs";
> + else {
> + char *labelfs = readlabelfs(argv[0], 0);
> + if (labelfs != NULL)
> + vfstype = labelfs;
> + }
> + }
> +
> + if (pledge("stdio rpath disklabel proc exec", NULL) == -1)
> + err(1, "pledge");
> +
> #define BADTYPE(type)
> \
> (strcmp(type, FSTAB_RO) && \
> strcmp(type, FSTAB_RW) && strcmp(type, FSTAB_RQ))
> @@ -261,23 +277,7 @@ main(int argc, char * const argv[])
> mntonname, options, fs->fs_mntops, skip);
> break;
> case 2:
> - /*
> - * If -t flag has not been specified, and spec contains either
> - * a ':' or a '@' then assume that an NFS filesystem is being
> - * specified ala Sun. If not, check the disklabel for a
> - * known filesystem type.
> - */
> - if (typelist == NULL) {
> - if (strpbrk(argv[0], ":@") != NULL)
> - vfstype = "nfs";
> - else {
> - char *labelfs = readlabelfs(argv[0], 0);
> - if (labelfs != NULL)
> - vfstype = labelfs;
> - }
> - }
> - rval = mountfs(vfstype,
> - argv[0], argv[1], options, NULL, 0);
> + rval = mountfs(vfstype, argv[0], argv[1], options, NULL, 0);
> break;
> default:
> usage();
>
ldpad(8): fix deletion of individual attribute values
TL;DR: OpenBSD's ldapd(8) has issues when deleting individual attribute
values. Patch below.
ZHANG Huangbin reported a misbehaviour in ldapd(8)'s MOD_DELETE
operation when connecting to ldapd(8) with the python-ldap library.
In ldapd(8) The MOD_DELETE operation always deletes all values of an
attribute and not only those specified in the request. (Mails from
Zhang Huangbin to bugs@ on May 18, 2016 and December 30, 2016).
I reproduced this issue connecting to ldapd(8) with the openLDAP client
tools (instead of the pyton-ldap library).
To illustrate the issue, lets take this LDAP entry (take note of the
"memberUID" attribute and its values):
dn: cn=detectives,ou=Group,dc=example,dc=org
objectClass: posixGroup
cn: detectives
gidNumber: 1012
memberUID: dasinger
memberUID: wergard
memberUID: gems
memberUID: amberdon
description: Detectives of the Kyth Interstaller Detective Agency
To delete the memberUID value of "amberdon" from this entry you submit
the following LDIF to the ldapd server:
dn: cn=detectives,ou=group,dc=example,dc=org
changeType: modify
delete: memberUid
memberUid: amberdon
I'm using the openLDAP command line tool "ldapmodify" for this. The
LDIF above is the contents of a file "del_amberdon.ldif":
ldapmodify -x -h $HOST -p 389 -D $BINDDN -w $PASSWD del_amberdon.ldif
The expected result would be a "detectives" group of:
dn: cn=detectives,ou=Group,dc=example,dc=org
objectClass: posixGroup
cn: detectives
gidNumber: 1012
memberUID: dasinger
memberUID: wergard
memberUID: gems
description: Detectives of the Kyth Interstaller Detective Agency
However, ldapd(8) now has removed all values for the "memberUID"
attribute (in LDAP parlance "the entire attribute is removed") and you
get the following entry::
dn: cn=detectives,ou=Group,dc=example,dc=org
objectClass: posixGroup
cn: detectives
gidNumber: 1012
description: Detectives of the Kyth Interstaller Detective Agency
Looking at the source, I found these issues (suggested fixes in
parentheses, tentative patch attached):
- in modify.c:ldap_modify(), lines 298 ff., in case LDAP_MOD_DELETE
there was a check for BER_TYPE_SET, however
1. AttributeValues are always in a set, even if it is empty
(PartialAttribute, see RFC4511, Section 4.1.7), so that check
couldn't have worked, even if the right variable had been checked
--- see next point.
2. The `vals' variable has a value of SET, however the variable
checked, `vals->be_sup' is already an element of the set, that is,
either it has a type of EOC (when there are no attribute values),
or it has a type of OCTETSTRING and contains the first attribute
value. (Look for a type of BER_TYPE_OCTETSTRING instead).
- in attributes.c:ldap_del_values(), lines 222 ff.
1. the elements inspected (variables `vk' and `xk') are not those
containing the attribute values; the attribute values are in `v'
and `x', `xk' and `vk' are (probably) uninitialized. (Use `v' and
`x' instead.)
2. When freeing the element found, current `v' is freed, and
`v->be_next' has no meaning anymore. (Use `next' variable to save
the pointer.)
3. Setting `prev' to `v' is wrong when an element has been
removed. (Set a flag if element is removed and re-set `prev' only
if the flag isn't set.)
- in ber.c:ber_free_elements() the current and all following elements
are freed. (Add ber_free_element() which frees only the current
element and use this function in attributes.c:ldap_del_values().)
Index: attributes.c
===
RCS file: /cvs/src/usr.sbin/ldapd/attributes.c,v
retrieving revision 1.4
diff -u -p -r1.4 attributes.c
--- attributes.c20 Jan 2017 11:55:08 - 1.4
+++ attributes.c1 Feb 2017 14:34:42 -
@@ -207,9 +207,9 @@ int
ldap_del_values(struct ber_element *elm, struct ber_element *vals)
{
char*attr;
- struct ber_element *old_vals, *v, *x, *vk, *xk, *prev;
+ struct ber_element *old_vals, *v, *x, *prev, *next;
struct ber_element *removed;
-
+ int removed_p;
assert(elm);
assert(vals);
assert(vals->be_sub);
@@ -220,19 +220,25 @@ ldap_del_values(struct ber_element *elm,
}
prev = old_vals;
- for (v = old_vals->be_sub; v; v = v->be_next) {
- vk = v->be_sub;
+ removed_p = 0;
+ for (v = old_vals->be_sub; v; v = next) {
+next = v->be_next;
+
for (x = vals->be_sub; x; x = x->be_next) {
- xk = x->be_sub;
- if (xk && vk->be_len == xk->be_len &&
- memcmp(vk->be_val, xk->be_val, xk->be_len) == 0) {
+ if (x && v->be_len == x->be_len &&
+ memcmp(v->be_val, x->be_val, x->be_len) == 0) {
removed = ber_unlink_elements(prev);
