[PATCH] More secure SYSRQ for xtables-addons

[PATCH] More secure SYSRQ for xtables-addons

[John Haxby]

Hello All,

This is a patch to the SYSRQ xtables-addon that is, I believe, secure 
enough to use in moderately untrustworthy environments.  I'm relatively 
new to posting patches so please forgive me if I've messed this up.

Rationale:

I want to be able to use SYSRQ to reboot, crash or partially diagnose 
machines that become unresponsive for one reason or another.   These 
machines, typically, are blades or rack mounted machines that do not 
have a PS/2 connection for a keyboard and the old method of wheeling 
round a "crash trolley" that has a monitor and a keyboard on it no 
longer works:  USB keyboards rarely, if ever, work because by the time 
the machine is responding only to a ping, udev is incapable of setting 
up a new keyboard.

This patch extends the xt_SYSRQ module to avoid both disclosing the 
sysrq password and preventing replay.  This is done by changing the 
request packet from the simple "<key><password>" to a slightly more 
complex "<key>,<seqno>,<salt>,<hash>".   The hash is the sha1 checksum 
of "<key>,<seqno>,<salt>,<password>".   A request can be constructed in 
a small shell script, for example:

    key="s"
    password="cookie"
    seqno=$(date +%s)
    salt="$(dd bs=12 count=1 if=/dev/urandom 2>/dev/null | openssl enc 
-base64)"
    req="$key,$seqno,$salt"
    req="$req,$(echo -n "$req,$password" | sha1sum | cut -c 1-40)"

As before this can be sent to the victim machine using socat or netcat.

Verification of the hash in xt_SYSRQ follows much the same process.   
The sequence number, seqno, is initialised to the current time (in 
seconds) when the xt_SYSRQ module is loaded and is updated each time a 
valid request is received.   A request with a sequence number less than 
the current sequence number or a wrong hash is silently ignored.   
(Using the time for the sequence number assumes (requires) that time 
doesn't go backwards on a reboot and that the requester and victim have 
reasonably synchronized clocks.)

The random salt is there to prevent pre-computed dictionary attacks 
difficult: dictionary attacks are still feasible if you capture a packet 
because the hash is computed quickly -- taking perhaps several 
milliseconds to compute a more complex hash in xt_SYSRQ when the machine 
is unresponsive is probably not the best thing you could do.  However, 
cracking, say, a random 32 character password would take some time and 
is probably beyond what the people in the target untrustworthy 
environment are prepared to do or have the resources for.   It almost 
goes without saying that no two victim machines should use the same 
password.

Ideally the iptables rules for SYSRQ would include a minimum time 
between requests to avoid the worst effects of sysrq-request bombing, 
although I haven't mentioned that in the updated man page (mostly 
because I'm not sure how to do it).

Finally, the module allocates all the resources it need at module 
initialisation time on the assumption that if things are going badly 
resource allocation is going to be troublesome.

jch


diff -up xtables-addons-1.6/extensions/libxt_SYSRQ.man.sysrq xtables-addons-1.6/extensions/libxt_SYSRQ.man
--- xtables-addons-1.6/extensions/libxt_SYSRQ.man.sysrq	2008-11-18 17:16:34.000000000 +0000
+++ xtables-addons-1.6/extensions/libxt_SYSRQ.man	2008-11-27 11:02:12.000000000 +0000
@@ -5,13 +5,15 @@ stuck as a result -- if still possible, 
 processes are stuck, interrupts are likely to be still processed, and as such,
 sysrq can be triggered through incoming network packets.
 .PP
-This xt_SYSRQ implementation does not use any encryption, so you should change
-the SYSRQ password after use unless you have made sure it was transmitted
-securely and no one sniffed the network, e.g. by use of an IPsec tunnel whose
-endpoint is at the machine where you want to trigger the sysrq. Also, you
-should limit as to who can issue commands using \fB-s\fP and/or \fB-m mac\fP,
-and also that the destination is correct using \fB-d\fP (to protect against
-potential broadcast packets), noting that it is still short of MAC/IP spoofing:
+The xt_SYSRQ implementation uses a salted SHA1 hash and a sequence number to
+prevent network sniffers from either guessing the password or replaying earlier
+requests.  The initial sequence number comes from the time of day so you will
+have a small window of vulnerability should time go backwards at a reboot.
+However, the file /sys/module/xt_SYSREQ/seqno can be used to both query and
+update the current sequence number.  Also, you should limit as to who can issue
+commands using \fB-s\fP and/or \fB-m mac\fP, and also that the destination is
+correct using \fB-d\fP (to protect against potential broadcast packets), noting
+that it is still short of MAC/IP spoofing:
 .IP
 -A INPUT -s 10.10.25.1 -m mac --mac-source aa:bb:cc:dd:ee:ff -d 10.10.25.7
 -p udp --dport 9 -j SYSRQ
@@ -24,10 +26,9 @@ This extension does not take any options
 required.
 .PP
 The SYSRQ password can be changed through
-/sys/module/xt_SYSRQ/parameters/password; note you need to use `echo -n` to
-not add a newline to the password, i.e.
+/sys/module/xt_SYSRQ/parameters/password, for example:
 .IP
-echo -n "password" >/sys/.../password
+echo "password" >/sys/module/xt_SYSRQ/parameters/password
 .PP
 Alternatively, the password may be specified at modprobe time, but this is
 insecure as people can possible see it through ps(1). You can use an option
@@ -36,12 +37,36 @@ by root.
 .IP
 options xt_SYSRQ password=cookies
 .PP
-To trigger SYSRQ from a remote host, just use netcat or socat, specifying the
-action (only one) as first character, followed by the password:
-.IP
-echo -n "scookies" | socat stdin udp-sendto:10.10.25.7:9
-.IP
-echo -n "scookies" | netcat -u 10.10.25.7 9
-.PP
-See the Linux docs for possible sysrq keys. Important ones are:
-re(b)oot, power(o)ff, (s)ync filesystems, (u)mount and remount readonly.
+The xt_SYSRQ module is normally silent unless a successful request is received,
+but the \fIdebug\fP module parameter can be used to find exactly why a
+seemingly correct request is not being processed.
+.PP
+To trigger SYSRQ from a remote host, just use netcat or socat:
+.RS
+.nf
+
+sysrq_key="s"  # the SysRq key
+password="password"
+seqno="$(date +%s)"
+salt="$(dd bs=12 count=1 if=/dev/urandom 2>/dev/null |
+    openssl enc -base64)"
+req="$sysrq_key,$seqno,$salt"
+req="$req,$(echo -n "$req,$password" | sha1sum | cut -c1-40)"
+
+echo "$req" | socat stdin udp-sendto:10.10.25.7:9
+# or
+echo "$req" | netcat -uw1 10.10.25.7 9
+
+.fi
+.RE
+.PP
+See the Linux docs for possible sysrq keys. Important ones are: re(b)oot,
+power(o)ff, (s)ync filesystems, (u)mount and remount readonly.  More than one
+sysrq key can be used at once, but bear in mind that, for example, a sync may
+not complete before a subsequent reboot or poweroff.
+.PP
+The hashing scheme should be enough to prevent mis-use of SYSRQ in many
+environments, but it is not perfect: take reasonable precautions to protect
+your machines.  Most importantly ensure that each machine has a different
+password: there is scant protection for a SYSRQ packet being applied to a
+machine that happens to have the same password.
diff -up xtables-addons-1.6/extensions/xt_SYSRQ.c.sysrq xtables-addons-1.6/extensions/xt_SYSRQ.c
--- xtables-addons-1.6/extensions/xt_SYSRQ.c.sysrq	2008-11-18 17:16:34.000000000 +0000
+++ xtables-addons-1.6/extensions/xt_SYSRQ.c	2008-11-27 11:15:34.000000000 +0000
@@ -3,7 +3,6 @@
  *	Copyright © Jan Engelhardt <jengelh [at] medozas de>, 2008
  *
  *	Based upon the ipt_SYSRQ idea by Marek Zalem <marek [at] terminus sk>
- *	xt_SYSRQ does not use hashing or timestamps.
  *
  *	This program is free software; you can redistribute it and/or
  *	modify it under the terms of the GNU General Public License
@@ -19,43 +18,137 @@
 #include <linux/netfilter_ipv4/ip_tables.h>
 #include <linux/netfilter_ipv6/ip6_tables.h>
 #include <linux/netfilter/x_tables.h>
+#include <linux/crypto.h>
+#include <linux/scatterlist.h>
 #include <net/ip.h>
 #include "compat_xtables.h"
 
 static bool sysrq_once;
 static char sysrq_password[64];
+static long seqno;
+static int debug;
 module_param_string(password, sysrq_password, sizeof(sysrq_password),
-	S_IRUSR | S_IWUSR);
+		S_IRUSR | S_IWUSR);
+module_param(seqno, long, S_IRUSR | S_IWUSR);
+module_param(debug, int, S_IRUSR | S_IWUSR);
 MODULE_PARM_DESC(password, "password for remote sysrq");
+MODULE_PARM_DESC(seqno, "sequence number for remote sysrq");
+MODULE_PARM_DESC(debug, "debugging: 0=off, 1=on");
 
+
+static struct crypto_hash *tfm;
+static int digestsize;
+static unsigned char *digest_password;
+static unsigned char *sha1_digest;
+static char *sha1_hexdigest;
+
+/*
+ * The data is of the form "<requests>,<seqno>,<salt>,<hash>" where
+ * <requests> is a series of sysrq requests; <seqno> is a sequence number
+ * which must be greater than the last sequence number; <salt> is some
+ * random bytes; and <hash> is the sha1 hash of everything up to and
+ * including the preceding "/" together with the password.
+ *
+ * For example
+ *
+ *   salt=$RANDOM
+ *   req="s,$(date +%s),$salt"
+ *   echo "$req,$(echo -n $req,secret | sha1sum | cut -c1-40)"
+ *
+ * You probably want a better salt and password than that though :-)
+ */
 static unsigned int sysrq_tg(const void *pdata, uint16_t len)
 {
 	const char *data = pdata;
-	char c;
+	int i, n;
+	struct scatterlist sg[2];
+	struct hash_desc desc;
+	int ret;
+	long new_seqno = 0;
 
 	if (*sysrq_password == '\0') {
 		if (!sysrq_once)
-			printk(KERN_INFO KBUILD_MODNAME "No password set\n");
+			printk(KERN_INFO KBUILD_MODNAME ": No password set\n");
 		sysrq_once = true;
 		return NF_DROP;
 	}
-
 	if (len == 0)
 		return NF_DROP;
 
-	c = *data;
-	if (strncmp(&data[1], sysrq_password, len - 1) != 0) {
-		printk(KERN_INFO KBUILD_MODNAME "Failed attempt - "
-		       "password mismatch\n");
+	for (i = 0; sysrq_password[i]; i++)
+		if (sysrq_password[i] == '\n')
+			break;
+	sysrq_password[i] = '\0';
+
+	i = 0;
+	for (n = 0; n < len - 1; n++) {
+		if (i == 1 && '0' <= data[n] && data[n] <= '9')
+			new_seqno = 10L * new_seqno + data[n] - '0';
+		if (data[n] == ',' && ++i == 3)
+			break;
+	}
+	n++;
+	if (i != 3) {
+		if (debug)
+			printk(KERN_WARNING KBUILD_MODNAME
+				": badly formatted request\n");
+		return NF_DROP;
+	}
+	if (seqno >= new_seqno) {
+		if (debug)
+			printk(KERN_WARNING KBUILD_MODNAME
+				": old sequence number ignored\n");
+		return NF_DROP;
+	}
+
+	desc.tfm = tfm;
+	desc.flags = 0;
+	ret = crypto_hash_init(&desc);
+	if (ret)
+		goto hash_fail;
+	sg_init_table(sg, 2);
+	sg_set_buf(&sg[0], data, n);
+	strcpy(digest_password, sysrq_password);
+	i = strlen(digest_password);
+	sg_set_buf(&sg[1], digest_password, i);
+	ret = crypto_hash_digest(&desc, sg, n + i, sha1_digest);
+	if (ret)
+		goto hash_fail;
+
+	for (i = 0; i < digestsize; i++) {
+		sha1_hexdigest[2 * i] =
+			"0123456789abcdef"[(sha1_digest[i] >> 4) & 0xf];
+		sha1_hexdigest[2 * i + 1] =
+			"0123456789abcdef"[sha1_digest[i] & 0xf];
+	}
+	sha1_hexdigest[2 * digestsize] = '\0';
+	if (len - n < digestsize) {
+		if (debug)
+			printk(KERN_INFO KBUILD_MODNAME ": Short digest,"
+				" expected %s\n", sha1_hexdigest);
+		return NF_DROP;
+	}
+	if (strncmp(data + n, sha1_hexdigest, digestsize) != 0) {
+		if (debug)
+			printk(KERN_INFO KBUILD_MODNAME ": Bad digest,"
+				" expected %s\n", sha1_hexdigest);
 		return NF_DROP;
 	}
 
+	/* Now we trust the requester */
+	seqno = new_seqno;
+	for (i = 0; i < len && data[i] != ','; i++) {
+		printk(KERN_INFO KBUILD_MODNAME ": SysRq %c\n", data[i]);
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 19)
-	handle_sysrq(c, NULL);
+		handle_sysrq(data[i], NULL);
 #else
-	handle_sysrq(c, NULL, NULL);
+		handle_sysrq(data[i], NULL, NULL);
 #endif
+	}
 	return NF_ACCEPT;
+hash_fail:
+	printk(KERN_WARNING KBUILD_MODNAME ": digest failure\n");
+	return NF_DROP;
 }
 
 static unsigned int
@@ -73,9 +166,11 @@ sysrq_tg4(struct sk_buff **pskb, const s
 	udph = (void *)iph + ip_hdrlen(skb);
 	len  = ntohs(udph->len) - sizeof(struct udphdr);
 
-	printk(KERN_INFO KBUILD_MODNAME ": " NIPQUAD_FMT ":%u -> :%u len=%u\n",
-	       NIPQUAD(iph->saddr), htons(udph->source), htons(udph->dest),
-	       len);
+	if (debug)
+		printk(KERN_INFO KBUILD_MODNAME
+			": " NIPQUAD_FMT ":%u -> :%u len=%u\n",
+			NIPQUAD(iph->saddr), htons(udph->source),
+			htons(udph->dest), len);
 	return sysrq_tg((void *)udph + sizeof(struct udphdr), len);
 }
 
@@ -94,9 +189,11 @@ sysrq_tg6(struct sk_buff **pskb, const s
 	udph = udp_hdr(skb);
 	len  = ntohs(udph->len) - sizeof(struct udphdr);
 
-	printk(KERN_INFO KBUILD_MODNAME ": " NIP6_FMT ":%hu -> :%hu len=%u\n",
-	       NIP6(iph->saddr), ntohs(udph->source),
-	       ntohs(udph->dest), len);
+	if (debug)
+		printk(KERN_INFO KBUILD_MODNAME
+			": " NIP6_FMT ":%hu -> :%hu len=%u\n",
+			NIP6(iph->saddr), ntohs(udph->source),
+			ntohs(udph->dest), len);
 	return sysrq_tg(udph + sizeof(struct udphdr), len);
 }
 
@@ -146,11 +243,54 @@ static struct xt_target sysrq_tg_reg[] _
 
 static int __init sysrq_tg_init(void)
 {
+	struct timeval now;
+	tfm = crypto_alloc_hash("sha1", 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(tfm)) {
+		printk(KERN_WARNING KBUILD_MODNAME
+			": Error: Could not find or load sha1 hash\n");
+		tfm = NULL;
+		goto fail;
+	}
+	digestsize = crypto_hash_digestsize(tfm);
+	sha1_digest = kmalloc(digestsize, GFP_KERNEL);
+	if (!sha1_digest) {
+		printk(KERN_WARNING KBUILD_MODNAME
+			": Cannot allocate sha1_digest\n");
+		goto fail;
+	}
+	sha1_hexdigest = kmalloc(2 * digestsize + 1, GFP_KERNEL);
+	if (!sha1_hexdigest) {
+		printk(KERN_WARNING KBUILD_MODNAME
+			": Cannot allocate sha1_hexdigest\n");
+		goto fail;
+	}
+	digest_password = kmalloc(sizeof(sysrq_password), GFP_KERNEL);
+	if (!digest_password) {
+		printk(KERN_WARNING KBUILD_MODNAME
+			": Cannot allocate password digest space\n");
+		goto fail;
+	}
+	do_gettimeofday(&now);
+	seqno = now.tv_sec;
 	return xt_register_targets(sysrq_tg_reg, ARRAY_SIZE(sysrq_tg_reg));
+fail:
+	if (tfm)
+		crypto_free_hash(tfm);
+	if (sha1_digest)
+		kfree(sha1_digest);
+	if (sha1_hexdigest)
+		kfree(sha1_hexdigest);
+	if (digest_password)
+		kfree(digest_password);
+	return -EINVAL;
 }
 
 static void __exit sysrq_tg_exit(void)
 {
+	crypto_free_hash(tfm);
+	kfree(sha1_digest);
+	kfree(sha1_hexdigest);
+	kfree(digest_password);
 	return xt_unregister_targets(sysrq_tg_reg, ARRAY_SIZE(sysrq_tg_reg));
 }
 


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] More secure SYSRQ for xtables-addons

[Jan Engelhardt]

On Thursday 2008-11-27 13:28, John Haxby wrote:

> Hello All,
>
> This is a patch to the SYSRQ xtables-addon that is, I believe, secure enough to
> use in moderately untrustworthy environments.

Finally - I wondered when someone will augment it with some crypto
magic :p

What I did not like with the original ipt_SYSRQ is that it used its
own copy of the SHA algorithm - while being well aware that
ipt_SYSRQ's root, if I remember correctly, dates to the time where
in-kernel crypto did not exist yet.

> Ideally the iptables rules for SYSRQ would include a minimum time
> between requests to avoid the worst effects of sysrq-request
> bombing, although I haven't mentioned that in the updated man page
> (mostly because I'm not sure how to do it).

-p udp --dport discard -m limit --limit 5/minute -j SYSRQ ...

> .IP
> -echo -n "password" >/sys/.../password
> +echo "password" >/sys/module/xt_SYSRQ/parameters/password
> .PP

I think we should not rely on \n being there. Checking for \0
should also catch the echo -n (or open from a perl/c program) case:

> +	for (i = 0; sysrq_password[i]; i++)
> +		if (sysrq_password[i] == '\n')
		|| sysrq_password[i] == '\0'

(assuming that echoing into string module parameters at least adds a \0)

> +	sg_init_table(sg, 2);
> +	sg_set_buf(&sg[0], data, n);
> +	strcpy(digest_password, sysrq_password);
> +	i = strlen(digest_password);
> +	sg_set_buf(&sg[1], digest_password, i);

Could we directly use sysrq_password instead of copying it
to digest_password first?

> +	for (i = 0; i < len && data[i] != ','; i++) {
> +		printk(KERN_INFO KBUILD_MODNAME ": SysRq %c\n", data[i]);
> #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 19)
> -	handle_sysrq(c, NULL);
> +		handle_sysrq(data[i], NULL);
> #else
> -	handle_sysrq(c, NULL, NULL);
> +		handle_sysrq(data[i], NULL, NULL);
> #endif
> +	}

I see you handle multiple sysrq requests in one packet. This should be 
reflected in the manual/comments (e.g. <keys> instead of <key>).

Care to fix it up? This looks good :)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] More secure SYSRQ for xtables-addons

[John Haxby]

Jan Engelhardt wrote:
> On Thursday 2008-11-27 13:28, John Haxby wrote:
>
>   
>> Hello All,
>>
>> This is a patch to the SYSRQ xtables-addon that is, I believe, secure enough to
>> use in moderately untrustworthy environments.
>>     
>
> Finally - I wondered when someone will augment it with some crypto
> magic :p
>
> What I did not like with the original ipt_SYSRQ is that it used its
> own copy of the SHA algorithm - while being well aware that
> ipt_SYSRQ's root, if I remember correctly, dates to the time where
> in-kernel crypto did not exist yet.
>
>   
I didn't like that much either: the reason given was that it didn't want 
to rely on kernel resources but that's quite a weak reason for including 
its own sha-1.  On the other hand, for kernels where there isn't an 
sha-1 available that does make sense.
>> Ideally the iptables rules for SYSRQ would include a minimum time
>> between requests to avoid the worst effects of sysrq-request
>> bombing, although I haven't mentioned that in the updated man page
>> (mostly because I'm not sure how to do it).
>>     
>
> -p udp --dport discard -m limit --limit 5/minute -j SYSRQ ...
>
>   
Thanks, I'll work that into the man page.
>> .IP
>> -echo -n "password" >/sys/.../password
>> +echo "password" >/sys/module/xt_SYSRQ/parameters/password
>> .PP
>>     
>
> I think we should not rely on \n being there. Checking for \0
> should also catch the echo -n (or open from a perl/c program) case:
>
>   
>> +	for (i = 0; sysrq_password[i]; i++)
>> +		if (sysrq_password[i] == '\n')
>>     
> 		|| sysrq_password[i] == '\0'
>
> (assuming that echoing into string module parameters at least adds a \0)
>
>   
I think it does this anyway doesn't it?   The "sysrq_password[i]" loop 
test stops at the '\0' and the "if (sysrq_password[i] == '\n') break" 
breaks out early if there's a '\n' in the string.  The next assignment 
either overwrites the trailing '\0' with another one or null-terminates 
the string at the first LF.
>> +	sg_init_table(sg, 2);
>> +	sg_set_buf(&sg[0], data, n);
>> +	strcpy(digest_password, sysrq_password);
>> +	i = strlen(digest_password);
>> +	sg_set_buf(&sg[1], digest_password, i);
>>     
>
> Could we directly use sysrq_password instead of copying it
> to digest_password first?
>
>   
No :-)   Eventually I discovered the reason my code wasn't working boils 
down to the definition of sg_set_buf:

    sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf))

which doesn't work for sysrq_password.   I don't know why I'll double check.
>> +	for (i = 0; i < len && data[i] != ','; i++) {
>> +		printk(KERN_INFO KBUILD_MODNAME ": SysRq %c\n", data[i]);
>> #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 19)
>> -	handle_sysrq(c, NULL);
>> +		handle_sysrq(data[i], NULL);
>> #else
>> -	handle_sysrq(c, NULL, NULL);
>> +		handle_sysrq(data[i], NULL, NULL);
>> #endif
>> +	}
>>     
>
> I see you handle multiple sysrq requests in one packet. This should be 
> reflected in the manual/comments (e.g. <keys> instead of <key>).
>
>   
Yes.

> Care to fix it up? This looks good :)
>   

Yes.

Thanks again.

jch

Re: [PATCH] More secure SYSRQ for xtables-addons

[Jan Engelhardt]

On Monday 2008-12-01 23:02, John Haxby wrote:
>>   
>> > +	for (i = 0; sysrq_password[i]; i++)
>> > +		if (sysrq_password[i] == '\n')
>> >     
>> +|| sysrq_password[i] == '\0'
>>
>> (assuming that echoing into string module parameters at least adds a \0)
>>   
> I think it does this anyway doesn't it?  The "sysrq_password[i]" loop test
> stops at the '\0'

Oh drats, I did not see that. That's why I prefer explicit zero tests
everywhere (except bools, because they are bools, and not
integers/pointers), e.g.

	for (i = 0; sysrq_password[i] != '\0'; ++i)

Also, \n could be simply tested for by adding it to the for condition:

	for (i = 0; sysrq_password[i] != '\0' &&
	     sysrq_password[i] != '\n'; ++i)
		/* loop */;

Turns out doing so saves 7 bytes on i586 ;-)

sg_set_page not usable for .bss?

[Jan Engelhardt]

On Monday 2008-12-01 23:02, John Haxby wrote:
>>>+	sg_init_table(sg, 2);
>>>+	sg_set_buf(&sg[0], data, n);
>>>+	strcpy(digest_password, sysrq_password);
>>>+	i = strlen(digest_password);
>>>+	sg_set_buf(&sg[1], digest_password, i);
>>
>> Could we directly use sysrq_password instead of copying it to
>> digest_password first?
>
> No :-)   Eventually I discovered the reason my code wasn't working
> boils down to the definition of sg_set_buf:
>
>   sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf))
>
> which doesn't work for sysrq_password.   I don't know why I'll
> double check.

Well, sysrq_password is in the .bss section, where as digest_password
is on the heap due to being kmalloc'ed. Maybe that makes a difference?
Someone more versed with the virtual memory layer might know.

>+static char sysrq_password[64];
>[...]
>+	digest_password = kmalloc(sizeof(sysrq_password), GFP_KERNEL);

Re: sg_set_page not usable for .bss?

[David Miller]

From: Jan Engelhardt <jengelh@medozas.de>
Date: Mon, 1 Dec 2008 23:40:18 +0100 (CET)

> 
> On Monday 2008-12-01 23:02, John Haxby wrote:
> >>>+	sg_init_table(sg, 2);
> >>>+	sg_set_buf(&sg[0], data, n);
> >>>+	strcpy(digest_password, sysrq_password);
> >>>+	i = strlen(digest_password);
> >>>+	sg_set_buf(&sg[1], digest_password, i);
> >>
> >> Could we directly use sysrq_password instead of copying it to
> >> digest_password first?
> >
> > No :-)   Eventually I discovered the reason my code wasn't working
> > boils down to the definition of sg_set_buf:
> >
> >   sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf))
> >
> > which doesn't work for sysrq_password.   I don't know why I'll
> > double check.
> 
> Well, sysrq_password is in the .bss section, where as digest_password
> is on the heap due to being kmalloc'ed. Maybe that makes a difference?
> Someone more versed with the virtual memory layer might know.

You can't use these interfaces on kernel image addresses.

Re: sg_set_page not usable for .bss?

[Jan Engelhardt]

On Tuesday 2008-12-02 01:10, David Miller wrote:
>> On Monday 2008-12-01 23:02, John Haxby wrote:
>> >>>+	sg_init_table(sg, 2);
>> >>>+	sg_set_buf(&sg[0], data, n);
>> >>>+	strcpy(digest_password, sysrq_password);
>> >>>+	i = strlen(digest_password);
>> >>>+	sg_set_buf(&sg[1], digest_password, i);
>> >>
>> >> Could we directly use sysrq_password instead of copying it to
>> >> digest_password first?
>> >
>> > No :-)   Eventually I discovered the reason my code wasn't working
>> > boils down to the definition of sg_set_buf:
>> >
>> >   sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf))
>> >
>> > which doesn't work for sysrq_password.   I don't know why I'll
>> > double check.
>> 
>> Well, sysrq_password is in the .bss section, where as digest_password
>> is on the heap due to being kmalloc'ed. Maybe that makes a difference?
>> Someone more versed with the virtual memory layer might know.
>
>You can't use these interfaces on kernel image addresses.
>
Great :-)  So what is the best way to use the SHA1 crypto algo
with in-kernel addresses?


Jan

Re: sg_set_page not usable for .bss?

[David Miller]

From: Jan Engelhardt <jengelh@medozas.de>
Date: Tue, 2 Dec 2008 01:13:34 +0100 (CET)

> 
> On Tuesday 2008-12-02 01:10, David Miller wrote:
> >> On Monday 2008-12-01 23:02, John Haxby wrote:
> >> >>>+	sg_init_table(sg, 2);
> >> >>>+	sg_set_buf(&sg[0], data, n);
> >> >>>+	strcpy(digest_password, sysrq_password);
> >> >>>+	i = strlen(digest_password);
> >> >>>+	sg_set_buf(&sg[1], digest_password, i);
> >> >>
> >> >> Could we directly use sysrq_password instead of copying it to
> >> >> digest_password first?
> >> >
> >> > No :-)   Eventually I discovered the reason my code wasn't working
> >> > boils down to the definition of sg_set_buf:
> >> >
> >> >   sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf))
> >> >
> >> > which doesn't work for sysrq_password.   I don't know why I'll
> >> > double check.
> >> 
> >> Well, sysrq_password is in the .bss section, where as digest_password
> >> is on the heap due to being kmalloc'ed. Maybe that makes a difference?
> >> Someone more versed with the virtual memory layer might know.
> >
> >You can't use these interfaces on kernel image addresses.
> >
> Great :-)  So what is the best way to use the SHA1 crypto algo
> with in-kernel addresses?

kmalloc and copy it there, or something like that, you just
can't use in-kernel addresses, ever.

Re: [PATCH] More secure SYSRQ for xtables-addons

[Patrick McHardy]

John Haxby wrote:
> Rationale:
> 
> I want to be able to use SYSRQ to reboot, crash or partially diagnose 
> machines that become unresponsive for one reason or another.   These 
> machines, typically, are blades or rack mounted machines that do not 
> have a PS/2 connection for a keyboard and the old method of wheeling 
> round a "crash trolley" that has a monitor and a keyboard on it no 
> longer works:  USB keyboards rarely, if ever, work because by the time 
> the machine is responding only to a ping, udev is incapable of setting 
> up a new keyboard.g/majordomo-info.html

This module is starting to look kind of useful. Maybe its time for
a resubmission for review and possibly merging once these patches
are included.

If we were to merge it, it would also be good to get some feedback
from the crypto guys about whether the chosen authentication scheme
meets its claims.

Re: sg_set_page not usable for .bss?

[Jan Engelhardt]

On Tuesday 2008-12-02 01:14, David Miller wrote:
>> >> 
>> >> Well, sysrq_password is in the .bss section, where as digest_password
>> >> is on the heap due to being kmalloc'ed. Maybe that makes a difference?
>> >> Someone more versed with the virtual memory layer might know.
>> >
>> >You can't use these interfaces on kernel image addresses.
>> >
>> Great :-)  So what is the best way to use the SHA1 crypto algo
>> with in-kernel addresses?
>
>kmalloc and copy it there, or something like that, you just
>can't use in-kernel addresses, ever.
>
Yes, kmalloc is already used. But then, what sort of address
does kmalloc return, if not an address within kernelspace?
(usually >=0xc0000000 on standard i386)

Re: [PATCH] More secure SYSRQ for xtables-addons

[Jan Engelhardt]

On Tuesday 2008-12-02 02:39, Patrick McHardy wrote:
>> 
>> I want to be able to use SYSRQ to reboot, crash or partially diagnose
>> machines that become unresponsive for one reason or another.   These
>> machines, typically, are blades or rack mounted machines that do not have a
>> PS/2 connection for a keyboard and the old method of wheeling round a "crash
>> trolley" that has a monitor and a keyboard on it no longer works:  USB
>> keyboards rarely, if ever, work because by the time the machine is responding
>> only to a ping, udev is incapable of setting up a new
>> keyboard.g/majordomo-info.html
>
> This module is starting to look kind of useful. Maybe its time for
> a resubmission for review and possibly merging once these patches
> are included.
>
> If we were to merge it, it would also be good to get some feedback
> from the crypto guys about whether the chosen authentication scheme
> meets its claims.
>
It looks similar to RFC 2617 HA1 generation. Should be ok.
In paranoia mode, the administrator can always change the
password after using it (effectively making it some sort of OTP).

On the other hand, xt_SYSRQ could, theoretically, also do a full
three-way authentication instead of the SPA it currently does. But I
think that is a bit of overkill.

Re: sg_set_page not usable for .bss?

[David Miller]

From: Jan Engelhardt <jengelh@medozas.de>
Date: Tue, 2 Dec 2008 02:41:02 +0100 (CET)

> 
> On Tuesday 2008-12-02 01:14, David Miller wrote:
> >> >> 
> >> >> Well, sysrq_password is in the .bss section, where as digest_password
> >> >> is on the heap due to being kmalloc'ed. Maybe that makes a difference?
> >> >> Someone more versed with the virtual memory layer might know.
> >> >
> >> >You can't use these interfaces on kernel image addresses.
> >> >
> >> Great :-)  So what is the best way to use the SHA1 crypto algo
> >> with in-kernel addresses?
> >
> >kmalloc and copy it there, or something like that, you just
> >can't use in-kernel addresses, ever.
> >
> Yes, kmalloc is already used. But then, what sort of address
> does kmalloc return, if not an address within kernelspace?
> (usually >=0xc0000000 on standard i386)

I said "kernel image" addresses are a problem, not "kernel space."

And by "kernel image" I mean addresses within the confines defined
by the sections of the vmlinux binary.

Re: [PATCH] More secure SYSRQ for xtables-addons

[John Haxby]

Patrick McHardy wrote:
> John Haxby wrote:
>> Rationale:
>>
>> I want to be able to use SYSRQ to reboot, crash or partially diagnose 
>> machines that become unresponsive for one reason or another.   These 
>> machines, typically, are blades or rack mounted machines that do not 
>> have a PS/2 connection for a keyboard and the old method of wheeling 
>> round a "crash trolley" that has a monitor and a keyboard on it no 
>> longer works:  USB keyboards rarely, if ever, work because by the 
>> time the machine is responding only to a ping, udev is incapable of 
>> setting up a new keyboard.g/majordomo-info.html
>
> This module is starting to look kind of useful. Maybe its time for
> a resubmission for review and possibly merging once these patches
> are included.
>
That's nice to hear!

> If we were to merge it, it would also be good to get some feedback
> from the crypto guys about whether the chosen authentication scheme
> meets its claims.

I agree.   I've talked this through with some people, but it needs some 
proper thought.    The weaknesses that I've identified are these:

 * The password can be recovered with an off-line dictionary attack.  
This is mitigated by using a good salt: in my example in the man page I 
use a 96 bit salt (dd bs=12 count=1 if=/dev/urandom) which makes a 
pre-computed dictionary attack difficult without large resources.  
However, a normal exhaustive search using the common password cracking 
techniques will yield a poorly chosen password fairly quickly.

 * The sha-1 hash is thought to be weak under some circumstances which 
makes its use for new cryptographic applications inappropriate.   The 
weaknesses, however, seem to be that SHA-1 is not good for digital 
signatures, but it would seem good enough for this purpose.  On the 
other hand, making the hash algorithm a module parameter so that it can 
be swapped out should SHA-1 prove unsuitable is straightforward (and I 
should probably do that).

 * If two machines have the same password then the mechanism is subject 
to a simple replay attack.   An attacker simply needs to capture the 
packet and send it to each of the possible target machines and see which 
ones crash :-)

 * A replay attack is possible on a single machine if the clock goes 
backwards (for example, on a reboot if the hardware clock is not UTC and 
the system time is not correctly set on boot).

 * xt_SYSRQ spamming could cause a DoS attack: simply spewing an endless 
stream of requests could tie up enough CPU resources to cause trouble.


All these attacks can be mitigated by good practices: using good, random 
passwords; changing the password(s) after an usage episode; changing the 
password(s) frequently anyway and so on.   Of course, stopping access to 
port 9 (or whatever) at a boundary and limiting the frequency of 
xt_SYSRQ requests almost goes without saying.

jch

[PATCH] More secure SYSRQ for xtables-addons

[John Haxby]

Hello All,

This is a patch to the SYSRQ xtables-addon that is, I believe, secure 
enough to use in moderately untrustworthy environments.

This is an updated version of the patch to address comments previously 
received.   The main change, prompted by Patrick McHardy's question, is 
to allow the hash algorithm to be changed at module load time.  Other 
than that I've clarified the '\n' password termination and updated the 
man page to include using -m limit, the configurable hash algorithm and 
made it a little that you can send multiple request keys.

Rationale:

I want to be able to use SYSRQ to reboot, crash or partially diagnose 
machines that become unresponsive for one reason or another.   These 
machines, typically, are blades or rack mounted machines that do not 
have a PS/2 connection for a keyboard and the old method of wheeling 
round a "crash trolley" that has a monitor and a keyboard on it no 
longer works:  USB keyboards rarely, if ever, work because by the time 
the machine is responding only to a ping, udev is incapable of setting 
up a new keyboard.

This patch extends the xt_SYSRQ module to avoid both disclosing the 
sysrq password and preventing replay.  This is done by changing the 
request packet from the simple "<key><password>" to a slightly more 
complex "<key>,<seqno>,<salt>,<hash>".   The hash is the sha1 checksum 
of "<key>,<seqno>,<salt>,<password>".   A request can be constructed in 
a small shell script, for example:

   keys="s" # keys processed in order
   password="cookie"
   seqno=$(date +%s)
   salt="$(dd bs=12 count=1 if=/dev/urandom 2>/dev/null | openssl enc 
-base64)"
   req="$keys,$seqno,$salt"
   req="$req,$(echo -n "$req,$password" | sha1sum | cut -c 1-40)"

As before this can be sent to the victim machine using socat or netcat.

Verification of the hash in xt_SYSRQ follows much the same process.   
The sequence number, seqno, is initialised to the current time (in 
seconds) when the xt_SYSRQ module is loaded and is updated each time a 
valid request is received.   A request with a sequence number less than 
the current sequence number or a wrong hash is silently ignored.   
(Using the time for the sequence number assumes (requires) that time 
doesn't go backwards on a reboot and that the requester and victim have 
reasonably synchronized clocks.)

The random salt is there to prevent pre-computed dictionary attacks 
difficult: dictionary attacks are still feasible if you capture a packet 
because the hash is computed quickly -- taking perhaps several 
milliseconds to compute a more complex hash in xt_SYSRQ when the machine 
is unresponsive is probably not the best thing you could do.  However, 
cracking, say, a random 32 character password would take some time and 
is probably beyond what the people in the target untrustworthy 
environment are prepared to do or have the resources for.   It almost 
goes without saying that no two victim machines should use the same 
password.

Finally, the module allocates all the resources it need at module 
initialisation time on the assumption that if things are going badly 
resource allocation is going to be troublesome.

jch

diff -up xtables-addons-1.6/extensions/libxt_SYSRQ.man.sysrq xtables-addons-1.6/extensions/libxt_SYSRQ.man
--- xtables-addons-1.6/extensions/libxt_SYSRQ.man.sysrq	2008-11-18 17:16:34.000000000 +0000
+++ xtables-addons-1.6/extensions/libxt_SYSRQ.man	2008-12-02 17:15:34.000000000 +0000
@@ -5,13 +5,15 @@ stuck as a result -- if still possible, 
 processes are stuck, interrupts are likely to be still processed, and as such,
 sysrq can be triggered through incoming network packets.
 .PP
-This xt_SYSRQ implementation does not use any encryption, so you should change
-the SYSRQ password after use unless you have made sure it was transmitted
-securely and no one sniffed the network, e.g. by use of an IPsec tunnel whose
-endpoint is at the machine where you want to trigger the sysrq. Also, you
-should limit as to who can issue commands using \fB-s\fP and/or \fB-m mac\fP,
-and also that the destination is correct using \fB-d\fP (to protect against
-potential broadcast packets), noting that it is still short of MAC/IP spoofing:
+The xt_SYSRQ implementation uses a salted hash and a sequence number to prevent
+network sniffers from either guessing the password or replaying earlier
+requests.  The initial sequence number comes from the time of day so you will
+have a small window of vulnerability should time go backwards at a reboot.
+However, the file /sys/module/xt_SYSREQ/seqno can be used to both query and
+update the current sequence number.  Also, you should limit as to who can issue
+commands using \fB-s\fP and/or \fB-m mac\fP, and also that the destination is
+correct using \fB-d\fP (to protect against potential broadcast packets), noting
+that it is still short of MAC/IP spoofing:
 .IP
 -A INPUT -s 10.10.25.1 -m mac --mac-source aa:bb:cc:dd:ee:ff -d 10.10.25.7
 -p udp --dport 9 -j SYSRQ
@@ -20,14 +22,19 @@ potential broadcast packets), noting tha
 ipsec --proto esp --tunnel-src 10.10.25.1 --tunnel-dst 10.10.25.7
 -p udp --dport 9 -j SYSRQ
 .PP
+You should also limit the rate at which connections can be received to limit
+the CPU time taken by illegal requests, for example:
+.IP
+-A INPUT 0s 10.10.25.1 -m mac --mac-source aa:bb:cc:dd:ee:ff -d 10.10.25.7
+-p udp --dport 9 -m limit --limit 5/minute -j SYSRQ
+.PP
 This extension does not take any options. The \fB-p udp\fP options are
 required.
 .PP
 The SYSRQ password can be changed through
-/sys/module/xt_SYSRQ/parameters/password; note you need to use `echo -n` to
-not add a newline to the password, i.e.
+/sys/module/xt_SYSRQ/parameters/password, for example:
 .IP
-echo -n "password" >/sys/.../password
+echo "password" >/sys/module/xt_SYSRQ/parameters/password
 .PP
 Alternatively, the password may be specified at modprobe time, but this is
 insecure as people can possible see it through ps(1). You can use an option
@@ -36,12 +43,41 @@ by root.
 .IP
 options xt_SYSRQ password=cookies
 .PP
-To trigger SYSRQ from a remote host, just use netcat or socat, specifying the
-action (only one) as first character, followed by the password:
-.IP
-echo -n "scookies" | socat stdin udp-sendto:10.10.25.7:9
+The hash algorithm can also be specified as a module option, for example, to
+use SHA-256 instead of the default SHA-1:
 .IP
-echo -n "scookies" | netcat -u 10.10.25.7 9
+options xt_SYSRQ hash=sha256
 .PP
-See the Linux docs for possible sysrq keys. Important ones are:
-re(b)oot, power(o)ff, (s)ync filesystems, (u)mount and remount readonly.
+The xt_SYSRQ module is normally silent unless a successful request is received,
+but the \fIdebug\fP module parameter can be used to find exactly why a
+seemingly correct request is not being processed.
+.PP
+To trigger SYSRQ from a remote host, just use netcat or socat:
+.RS
+.nf
+
+sysrq_key="s"  # the SysRq key(s)
+password="password"
+seqno="$(date +%s)"
+salt="$(dd bs=12 count=1 if=/dev/urandom 2>/dev/null |
+    openssl enc -base64)"
+req="$sysrq_key,$seqno,$salt"
+req="$req,$(echo -n "$req,$password" | sha1sum | cut -c1-40)"
+
+echo "$req" | socat stdin udp-sendto:10.10.25.7:9
+# or
+echo "$req" | netcat -uw1 10.10.25.7 9
+
+.fi
+.RE
+.PP
+See the Linux docs for possible sysrq keys. Important ones are: re(b)oot,
+power(o)ff, (s)ync filesystems, (u)mount and remount readonly.  More than one
+sysrq key can be used at once, but bear in mind that, for example, a sync may
+not complete before a subsequent reboot or poweroff.
+.PP
+The hashing scheme should be enough to prevent mis-use of SYSRQ in many
+environments, but it is not perfect: take reasonable precautions to protect
+your machines.  Most importantly ensure that each machine has a different
+password; there is scant protection for a SYSRQ packet being applied to a
+machine that happens to have the same password.
diff -up xtables-addons-1.6/extensions/xt_SYSRQ.c.sysrq xtables-addons-1.6/extensions/xt_SYSRQ.c
--- xtables-addons-1.6/extensions/xt_SYSRQ.c.sysrq	2008-11-18 17:16:34.000000000 +0000
+++ xtables-addons-1.6/extensions/xt_SYSRQ.c	2008-12-02 16:38:17.000000000 +0000
@@ -3,7 +3,6 @@
  *	Copyright © Jan Engelhardt <jengelh [at] medozas de>, 2008
  *
  *	Based upon the ipt_SYSRQ idea by Marek Zalem <marek [at] terminus sk>
- *	xt_SYSRQ does not use hashing or timestamps.
  *
  *	This program is free software; you can redistribute it and/or
  *	modify it under the terms of the GNU General Public License
@@ -19,43 +18,141 @@
 #include <linux/netfilter_ipv4/ip_tables.h>
 #include <linux/netfilter_ipv6/ip6_tables.h>
 #include <linux/netfilter/x_tables.h>
+#include <linux/crypto.h>
+#include <linux/scatterlist.h>
 #include <net/ip.h>
 #include "compat_xtables.h"
 
 static bool sysrq_once;
 static char sysrq_password[64];
+static char sysrq_hash[16] = "sha1";
+static long seqno;
+static int debug;
 module_param_string(password, sysrq_password, sizeof(sysrq_password),
-	S_IRUSR | S_IWUSR);
-MODULE_PARM_DESC(password, "password for remote sysrq");
-
+		S_IRUSR | S_IWUSR);
+module_param_string(hash, sysrq_hash, sizeof(sysrq_hash),
+		S_IRUSR);
+module_param(seqno, long, S_IRUSR | S_IWUSR);
+module_param(debug, int, S_IRUSR | S_IWUSR);
+MODULE_PARM_DESC(password, " password for remote sysrq");
+MODULE_PARM_DESC(hash, " hash algorithm, default sha1");
+MODULE_PARM_DESC(seqno, " sequence number for remote sysrq");
+MODULE_PARM_DESC(debug, " debugging: 0=off, 1=on");
+
+
+static struct crypto_hash *tfm;
+static int digestsize;
+static unsigned char *digest_password;
+static unsigned char *digest;
+static char *hexdigest;
+
+/*
+ * The data is of the form "<requests>,<seqno>,<salt>,<hash>" where <requests>
+ * is a series of sysrq requests; <seqno> is a sequence number that must be
+ * greater than the last sequence number; <salt> is some random bytes; and
+ * <hash> is the hash of everything up to and including the preceding ","
+ * together with the password.
+ *
+ * For example
+ *
+ *   salt=$RANDOM
+ *   req="s,$(date +%s),$salt"
+ *   echo "$req,$(echo -n $req,secret | sha1sum | cut -c1-40)"
+ *
+ * You will want a better salt and password than that though :-)
+ */
 static unsigned int sysrq_tg(const void *pdata, uint16_t len)
 {
 	const char *data = pdata;
-	char c;
+	int i, n;
+	struct scatterlist sg[2];
+	struct hash_desc desc;
+	int ret;
+	long new_seqno = 0;
 
 	if (*sysrq_password == '\0') {
 		if (!sysrq_once)
-			printk(KERN_INFO KBUILD_MODNAME "No password set\n");
+			printk(KERN_INFO KBUILD_MODNAME ": No password set\n");
 		sysrq_once = true;
 		return NF_DROP;
 	}
-
 	if (len == 0)
 		return NF_DROP;
 
-	c = *data;
-	if (strncmp(&data[1], sysrq_password, len - 1) != 0) {
-		printk(KERN_INFO KBUILD_MODNAME "Failed attempt - "
-		       "password mismatch\n");
+	for (i = 0; sysrq_password[i] != '\0' &&
+		     sysrq_password[i] != '\n'; i++)
+		/* loop */;
+	sysrq_password[i] = '\0';
+
+	i = 0;
+	for (n = 0; n < len - 1; n++) {
+		if (i == 1 && '0' <= data[n] && data[n] <= '9')
+			new_seqno = 10L * new_seqno + data[n] - '0';
+		if (data[n] == ',' && ++i == 3)
+			break;
+	}
+	n++;
+	if (i != 3) {
+		if (debug)
+			printk(KERN_WARNING KBUILD_MODNAME
+				": badly formatted request\n");
+		return NF_DROP;
+	}
+	if (seqno >= new_seqno) {
+		if (debug)
+			printk(KERN_WARNING KBUILD_MODNAME
+				": old sequence number ignored\n");
+		return NF_DROP;
+	}
+
+	desc.tfm = tfm;
+	desc.flags = 0;
+	ret = crypto_hash_init(&desc);
+	if (ret)
+		goto hash_fail;
+	sg_init_table(sg, 2);
+	sg_set_buf(&sg[0], data, n);
+	strcpy(digest_password, sysrq_password);
+	i = strlen(digest_password);
+	sg_set_buf(&sg[1], digest_password, i);
+	ret = crypto_hash_digest(&desc, sg, n + i, digest);
+	if (ret)
+		goto hash_fail;
+
+	for (i = 0; i < digestsize; i++) {
+		hexdigest[2 * i] =
+			"0123456789abcdef"[(digest[i] >> 4) & 0xf];
+		hexdigest[2 * i + 1] =
+			"0123456789abcdef"[digest[i] & 0xf];
+	}
+	hexdigest[2 * digestsize] = '\0';
+	if (len - n < digestsize) {
+		if (debug)
+			printk(KERN_INFO KBUILD_MODNAME ": Short digest,"
+				" expected %s\n", hexdigest);
+		return NF_DROP;
+	}
+	if (strncmp(data + n, hexdigest, digestsize) != 0) {
+		if (debug)
+			printk(KERN_INFO KBUILD_MODNAME ": Bad digest,"
+				" expected %s\n", hexdigest);
 		return NF_DROP;
 	}
 
+	/* Now we trust the requester */
+	seqno = new_seqno;
+	for (i = 0; i < len && data[i] != ','; i++) {
+		printk(KERN_INFO KBUILD_MODNAME ": SysRq %c\n", data[i]);
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 19)
-	handle_sysrq(c, NULL);
+		handle_sysrq(data[i], NULL);
 #else
-	handle_sysrq(c, NULL, NULL);
+		handle_sysrq(data[i], NULL, NULL);
 #endif
+	}
 	return NF_ACCEPT;
+hash_fail:
+	printk(KERN_WARNING KBUILD_MODNAME ": digest failure\n");
+	return NF_DROP;
 }
 
 static unsigned int
@@ -73,9 +170,11 @@ sysrq_tg4(struct sk_buff **pskb, const s
 	udph = (void *)iph + ip_hdrlen(skb);
 	len  = ntohs(udph->len) - sizeof(struct udphdr);
 
-	printk(KERN_INFO KBUILD_MODNAME ": " NIPQUAD_FMT ":%u -> :%u len=%u\n",
-	       NIPQUAD(iph->saddr), htons(udph->source), htons(udph->dest),
-	       len);
+	if (debug)
+		printk(KERN_INFO KBUILD_MODNAME
+			": " NIPQUAD_FMT ":%u -> :%u len=%u\n",
+			NIPQUAD(iph->saddr), htons(udph->source),
+			htons(udph->dest), len);
 	return sysrq_tg((void *)udph + sizeof(struct udphdr), len);
 }
 
@@ -94,9 +193,11 @@ sysrq_tg6(struct sk_buff **pskb, const s
 	udph = udp_hdr(skb);
 	len  = ntohs(udph->len) - sizeof(struct udphdr);
 
-	printk(KERN_INFO KBUILD_MODNAME ": " NIP6_FMT ":%hu -> :%hu len=%u\n",
-	       NIP6(iph->saddr), ntohs(udph->source),
-	       ntohs(udph->dest), len);
+	if (debug)
+		printk(KERN_INFO KBUILD_MODNAME
+			": " NIP6_FMT ":%hu -> :%hu len=%u\n",
+			NIP6(iph->saddr), ntohs(udph->source),
+			ntohs(udph->dest), len);
 	return sysrq_tg(udph + sizeof(struct udphdr), len);
 }
 
@@ -146,11 +247,55 @@ static struct xt_target sysrq_tg_reg[] _
 
 static int __init sysrq_tg_init(void)
 {
+	struct timeval now;
+	tfm = crypto_alloc_hash(sysrq_hash, 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(tfm)) {
+		printk(KERN_WARNING KBUILD_MODNAME
+			": Error: Could not find or load %s hash\n",
+			sysrq_hash);
+		tfm = NULL;
+		goto fail;
+	}
+	digestsize = crypto_hash_digestsize(tfm);
+	digest = kmalloc(digestsize, GFP_KERNEL);
+	if (!digest) {
+		printk(KERN_WARNING KBUILD_MODNAME
+			": Cannot allocate digest\n");
+		goto fail;
+	}
+	hexdigest = kmalloc(2 * digestsize + 1, GFP_KERNEL);
+	if (!hexdigest) {
+		printk(KERN_WARNING KBUILD_MODNAME
+			": Cannot allocate hexdigest\n");
+		goto fail;
+	}
+	digest_password = kmalloc(sizeof(sysrq_password), GFP_KERNEL);
+	if (!digest_password) {
+		printk(KERN_WARNING KBUILD_MODNAME
+			": Cannot allocate password digest space\n");
+		goto fail;
+	}
+	do_gettimeofday(&now);
+	seqno = now.tv_sec;
 	return xt_register_targets(sysrq_tg_reg, ARRAY_SIZE(sysrq_tg_reg));
+fail:
+	if (tfm)
+		crypto_free_hash(tfm);
+	if (digest)
+		kfree(digest);
+	if (hexdigest)
+		kfree(hexdigest);
+	if (digest_password)
+		kfree(digest_password);
+	return -EINVAL;
 }
 
 static void __exit sysrq_tg_exit(void)
 {
+	crypto_free_hash(tfm);
+	kfree(digest);
+	kfree(hexdigest);
+	kfree(digest_password);
 	return xt_unregister_targets(sysrq_tg_reg, ARRAY_SIZE(sysrq_tg_reg));
 }
 


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] More secure SYSRQ for xtables-addons

[John Haxby]

John Haxby wrote:
> Hello All,
>
> This is a patch to the SYSRQ xtables-addon that is, I believe, secure 
> enough to use in moderately untrustworthy environments.
>
> This is an updated version of the patch to address comments previously 
> received.   The main change, prompted by Patrick McHardy's question, 
> is to allow the hash algorithm to be changed at module load time.  
> Other than that I've clarified the '\n' password termination and 
> updated the man page to include using -m limit, the configurable hash 
> algorithm and made it a little that you can send multiple request keys.

I don't know if I'm being overly anxious or impatient, but is there 
anything more I need to do for this updated patch?

jch

Re: [PATCH] More secure SYSRQ for xtables-addons

[Jan Engelhardt]

On Friday 2008-12-12 09:38, John Haxby wrote:
> John Haxby wrote:
>> Hello All,
>>
>> This is a patch to the SYSRQ xtables-addon that is, I believe, secure enough
>> to use in moderately untrustworthy environments.
>>
>> This is an updated version of the patch to address comments previously
>> received.   The main change, prompted by Patrick McHardy's question, is to
>> allow the hash algorithm to be changed at module load time.  Other than that
>> I've clarified the '\n' password termination and updated the man page to
>> include using -m limit, the configurable hash algorithm and made it a little
>> that you can send multiple request keys.
>
> I don't know if I'm being overly anxious or impatient, but is there anything
> more I need to do for this updated patch?

Hm, did I miss to send an “ok”? It definitely is up in the git repo already.
Thanks again for the submission!
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] More secure SYSRQ for xtables-addons

[Jan Engelhardt]

On Saturday 2008-12-13 23:14, Jan Engelhardt wrote:

>
>On Friday 2008-12-12 09:38, John Haxby wrote:
>>>[xt_sysrq patch]
>>
>> I don't know if I'm being overly anxious or impatient, but is there anything
>> more I need to do for this updated patch?
>
>Hm, did I miss to send an “ok”? It definitely is up in the git repo already.
>Thanks again for the submission!
>
Oh yeah I noticed a dangling mail in my (as usual, overly full) "Draft"
folder. Sorry about that!
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html