Re: [PATCH 1/2] netfilter: xtables: inclusion of xt_SYSRQ

Re: [PATCH 1/2] netfilter: xtables: inclusion of xt_SYSRQ

[Patrick McHardy]

Jan Engelhardt wrote:
> The SYSRQ target will allow to remotely invoke sysrq on the local
> machine. Authentication is by means of a pre-shared key that can
> either be transmitted plaintext or digest-secured.

I really think this is pushing what netfilter is meant for a bit
far. Its basically abusing the firewall ruleset to offer a network
service.

I can see that its useful to have this in the kernel instead of
userspace, but why isn't this implemented as a stand-alone module?
That seems like a better design to me and also makes it more useful
by not depending on netfilter.

> Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
> ---
>  net/netfilter/Kconfig    |   12 ++
>  net/netfilter/Makefile   |    1 +
>  net/netfilter/xt_SYSRQ.c |  354 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 367 insertions(+), 0 deletions(-)
>  create mode 100644 net/netfilter/xt_SYSRQ.c
> 
> diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
> index 673a6c8..bfd9b6f 100644
> --- a/net/netfilter/Kconfig
> +++ b/net/netfilter/Kconfig
> @@ -502,6 +502,18 @@ config NETFILTER_XT_TARGET_RATEEST
>  
>  	  To compile it as a module, choose M here.  If unsure, say N.
>  
> +config NETFILTER_XT_TARGET_SYSRQ
> +	tristate '"SYSRQ" - remote sysrq invocation'
> +	depends on NETFILTER_ADVANCED
> +	---help---
> +	This option enables the "SYSRQ" target which can be used to trigger
> +	sysrq from a remote machine using a magic UDP packet with a pre-shared
> +	password. This is useful when the receiving host has locked up in an
> +	Oops yet still can process incoming packets.
> +
> +	Besides plaintext packets, digest-secured SYSRQ requests will be
> +	supported when CONFIG_CRYPTO is enabled.
> +
>  config NETFILTER_XT_TARGET_TEE
>  	tristate '"TEE" - packet cloning to alternate destiantion'
>  	depends on NETFILTER_ADVANCED
> diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
> index 14e3a8f..f032195 100644
> --- a/net/netfilter/Makefile
> +++ b/net/netfilter/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_NETFILTER_XT_TARGET_NFQUEUE) += xt_NFQUEUE.o
>  obj-$(CONFIG_NETFILTER_XT_TARGET_NOTRACK) += xt_NOTRACK.o
>  obj-$(CONFIG_NETFILTER_XT_TARGET_RATEEST) += xt_RATEEST.o
>  obj-$(CONFIG_NETFILTER_XT_TARGET_SECMARK) += xt_SECMARK.o
> +obj-$(CONFIG_NETFILTER_XT_TARGET_SYSRQ) += xt_SYSRQ.o
>  obj-$(CONFIG_NETFILTER_XT_TARGET_TPROXY) += xt_TPROXY.o
>  obj-$(CONFIG_NETFILTER_XT_TARGET_TCPMSS) += xt_TCPMSS.o
>  obj-$(CONFIG_NETFILTER_XT_TARGET_TCPOPTSTRIP) += xt_TCPOPTSTRIP.o
> diff --git a/net/netfilter/xt_SYSRQ.c b/net/netfilter/xt_SYSRQ.c
> new file mode 100644
> index 0000000..929b204
> --- /dev/null
> +++ b/net/netfilter/xt_SYSRQ.c
> @@ -0,0 +1,354 @@
> +/*
> + *	"SYSRQ" target extension for Netfilter
> + *	Copyright © Jan Engelhardt <jengelh [at] medozas de>, 2008 - 2010
> + *
> + *	Based upon the ipt_SYSRQ idea by Marek Zalem <marek [at] terminus sk>
> + *
> + *	This program is free software; you can redistribute it and/or
> + *	modify it under the terms of the GNU General Public License
> + *	version 2 or later as published by the Free Software Foundation.
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +#include <linux/in.h>
> +#include <linux/ip.h>
> +#include <linux/ipv6.h>
> +#include <linux/module.h>
> +#include <linux/skbuff.h>
> +#include <linux/sysrq.h>
> +#include <linux/udp.h>
> +#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>
> +
> +#if defined(CONFIG_CRYPTO) || defined(CRYPTO_CONFIG_MODULE)
> +#	define WITH_CRYPTO 1
> +#endif
> +#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
> +#	define WITH_IPV6 1
> +#endif
> +
> +static bool sysrq_once;
> +static char sysrq_password[64];
> +static char sysrq_hash[16] = "sha1";
> +static long sysrq_seqno;
> +static int sysrq_debug;
> +module_param_string(password, sysrq_password, sizeof(sysrq_password),
> +	S_IRUSR | S_IWUSR);
> +module_param_string(hash, sysrq_hash, sizeof(sysrq_hash), S_IRUSR);
> +module_param_named(seqno, sysrq_seqno, long, S_IRUSR | S_IWUSR);
> +module_param_named(debug, sysrq_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");
> +
> +#ifdef WITH_CRYPTO
> +static struct crypto_hash *sysrq_tfm;
> +static int sysrq_digest_size;
> +static unsigned char *sysrq_digest_password;
> +static unsigned char *sysrq_digest;
> +static char *sysrq_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;
> +	int i, n;
> +	struct scatterlist sg[2];
> +	struct hash_desc desc;
> +	int ret;
> +	long new_seqno = 0;
> +
> +	if (*sysrq_password == '\0') {
> +		if (!sysrq_once)
> +			pr_info("No password set\n");
> +		sysrq_once = true;
> +		return NF_DROP;
> +	}
> +	if (len == 0)
> +		return NF_DROP;
> +
> +	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 (sysrq_debug)
> +			pr_info("badly formatted request\n");
> +		return NF_DROP;
> +	}
> +	if (sysrq_seqno >= new_seqno) {
> +		if (sysrq_debug)
> +			pr_info("old sequence number ignored\n");
> +		return NF_DROP;
> +	}
> +
> +	desc.tfm   = sysrq_tfm;
> +	desc.flags = 0;
> +	ret = crypto_hash_init(&desc);
> +	if (ret != 0)
> +		goto hash_fail;
> +	sg_init_table(sg, 2);
> +	sg_set_buf(&sg[0], data, n);
> +	strcpy(sysrq_digest_password, sysrq_password);
> +	i = strlen(sysrq_digest_password);
> +	sg_set_buf(&sg[1], sysrq_digest_password, i);
> +	ret = crypto_hash_digest(&desc, sg, n + i, sysrq_digest);
> +	if (ret != 0)
> +		goto hash_fail;
> +
> +	for (i = 0; i < sysrq_digest_size; ++i) {
> +		sysrq_hexdigest[2*i] =
> +			"0123456789abcdef"[(sysrq_digest[i] >> 4) & 0xf];
> +		sysrq_hexdigest[2*i+1] =
> +			"0123456789abcdef"[sysrq_digest[i] & 0xf];
> +	}
> +	sysrq_hexdigest[2*sysrq_digest_size] = '\0';
> +	if (len - n < sysrq_digest_size) {
> +		if (sysrq_debug)
> +			pr_info("Short digest, expected %s\n",
> +				sysrq_hexdigest);
> +		return NF_DROP;
> +	}
> +	if (strncmp(data + n, sysrq_hexdigest, sysrq_digest_size) != 0) {
> +		if (sysrq_debug)
> +			pr_info("Bad digest, expected %s\n", sysrq_hexdigest);
> +		return NF_DROP;
> +	}
> +
> +	/* Now we trust the requester */
> +	sysrq_seqno = new_seqno;
> +	for (i = 0; i < len && data[i] != ','; ++i) {
> +		pr_info("SysRq %c\n", data[i]);
> +		handle_sysrq(data[i], NULL);
> +	}
> +	return NF_ACCEPT;
> +
> + hash_fail:
> +	pr_warning("digest failure\n");
> +	return NF_DROP;
> +}
> +#else
> +static unsigned int sysrq_tg(const void *pdata, uint16_t len)
> +{
> +	const char *data = pdata;
> +	char c;
> +
> +	if (*sysrq_password == '\0') {
> +		if (!sysrq_once)
> +			pr_info("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) {
> +		pr_warning("Failed attempt - password mismatch\n");
> +		return NF_DROP;
> +	}
> +
> +	handle_sysrq(c, NULL);
> +	return NF_ACCEPT;
> +}
> +#endif
> +
> +static unsigned int
> +sysrq_tg4(struct sk_buff *skb, const struct xt_target_param *par)
> +{
> +	const struct iphdr *iph;
> +	const struct udphdr *udph;
> +	uint16_t len;
> +
> +	if (skb_linearize(skb) < 0)
> +		return NF_DROP;
> +
> +	iph = ip_hdr(skb);
> +	if (iph->protocol != IPPROTO_UDP && iph->protocol != IPPROTO_UDPLITE)
> +		return NF_DROP;
> +
> +	udph = (const void *)iph + ip_hdrlen(skb);
> +	len  = ntohs(udph->len) - sizeof(struct udphdr);
> +
> +	if (sysrq_debug)
> +		pr_info(": %pI4:%u -> :%u len=%u\n", &iph->saddr,
> +			htons(udph->source), htons(udph->dest), len);
> +	return sysrq_tg((void *)udph + sizeof(struct udphdr), len);
> +}
> +
> +#ifdef WITH_IPV6
> +static unsigned int
> +sysrq_tg6(struct sk_buff *skb, const struct xt_target_param *par)
> +{
> +	const struct ipv6hdr *iph;
> +	const struct udphdr *udph;
> +	unsigned short frag_off;
> +	unsigned int th_off;
> +	uint16_t len;
> +
> +	if (skb_linearize(skb) < 0)
> +		return NF_DROP;
> +
> +	iph = ipv6_hdr(skb);
> +	if (ipv6_find_hdr(skb, &th_off, IPPROTO_UDP, &frag_off) < 0 ||
> +	    frag_off > 0)
> +		return NF_ACCEPT; /* sink it */
> +
> +	udph = (const void *)iph + th_off;
> +	len  = ntohs(udph->len) - sizeof(struct udphdr);
> +
> +	if (sysrq_debug)
> +		pr_info("%pI6:%hu -> :%hu len=%u\n", &iph->saddr,
> +			ntohs(udph->source), ntohs(udph->dest), len);
> +	return sysrq_tg(udph + sizeof(struct udphdr), len);
> +}
> +#endif
> +
> +static int sysrq_tg_check(const struct xt_tgchk_param *par)
> +{
> +	if (par->target->family == NFPROTO_IPV4) {
> +		const struct ipt_entry *entry = par->entryinfo;
> +
> +		if ((entry->ip.proto != IPPROTO_UDP &&
> +		    entry->ip.proto != IPPROTO_UDPLITE) ||
> +		    entry->ip.invflags & XT_INV_PROTO)
> +			goto out;
> +	} else if (par->target->family == NFPROTO_IPV6) {
> +		const struct ip6t_entry *entry = par->entryinfo;
> +
> +		if ((entry->ipv6.proto != IPPROTO_UDP &&
> +		    entry->ipv6.proto != IPPROTO_UDPLITE) ||
> +		    entry->ipv6.invflags & XT_INV_PROTO)
> +			goto out;
> +	}
> +
> +	return true;
> +
> + out:
> +	pr_info("only available for UDP and UDP-Lite");
> +	return false;
> +}
> +
> +static struct xt_target sysrq_tg_reg[] __read_mostly = {
> +	{
> +		.name       = "SYSRQ",
> +		.revision   = 1,
> +		.family     = NFPROTO_IPV4,
> +		.target     = sysrq_tg4,
> +		.checkentry = sysrq_tg_check,
> +		.me         = THIS_MODULE,
> +	},
> +#ifdef WITH_IPV6
> +	{
> +		.name       = "SYSRQ",
> +		.revision   = 1,
> +		.family     = NFPROTO_IPV6,
> +		.target     = sysrq_tg6,
> +		.checkentry = sysrq_tg_check,
> +		.me         = THIS_MODULE,
> +	},
> +#endif
> +};
> +
> +static void sysrq_crypto_exit(void)
> +{
> +#ifdef WITH_CRYPTO
> +	if (sysrq_tfm)
> +		crypto_free_hash(sysrq_tfm);
> +	if (sysrq_digest)
> +		kfree(sysrq_digest);
> +	if (sysrq_hexdigest)
> +		kfree(sysrq_hexdigest);
> +	if (sysrq_digest_password)
> +		kfree(sysrq_digest_password);
> +#endif
> +}
> +
> +static int __init sysrq_crypto_init(void)
> +{
> +#if defined(WITH_CRYPTO)
> +	struct timeval now;
> +	int ret;
> +
> +	sysrq_tfm = crypto_alloc_hash(sysrq_hash, 0, CRYPTO_ALG_ASYNC);
> +	if (IS_ERR(sysrq_tfm)) {
> +		pr_err("Could not find or load %s hash\n", sysrq_hash);
> +		sysrq_tfm = NULL;
> +		ret = PTR_ERR(sysrq_tfm);
> +		goto fail;
> +	}
> +	sysrq_digest_size = crypto_hash_digestsize(sysrq_tfm);
> +	sysrq_digest = kmalloc(sysrq_digest_size, GFP_KERNEL);
> +	ret = -ENOMEM;
> +	if (sysrq_digest == NULL)
> +		goto fail;
> +	sysrq_hexdigest = kmalloc(2 * sysrq_digest_size + 1, GFP_KERNEL);
> +	if (sysrq_hexdigest == NULL)
> +		goto fail;
> +	sysrq_digest_password = kmalloc(sizeof(sysrq_password), GFP_KERNEL);
> +	if (sysrq_digest_password == NULL)
> +		goto fail;
> +	do_gettimeofday(&now);
> +	sysrq_seqno = now.tv_sec;
> +	ret = xt_register_targets(sysrq_tg_reg, ARRAY_SIZE(sysrq_tg_reg));
> +	if (ret < 0)
> +		goto fail;
> +	return ret;
> +
> + fail:
> +	sysrq_crypto_exit();
> +	return ret;
> +#else
> +	pr_info("compiled without crypto\n");
> +#endif
> +	return -EINVAL;
> +}
> +
> +static int __init sysrq_tg_init(void)
> +{
> +	if (sysrq_crypto_init() < 0)
> +		pr_info("starting without crypto\n");
> +	return xt_register_targets(sysrq_tg_reg, ARRAY_SIZE(sysrq_tg_reg));
> +}
> +
> +static void __exit sysrq_tg_exit(void)
> +{
> +	sysrq_crypto_exit();
> +	xt_unregister_targets(sysrq_tg_reg, ARRAY_SIZE(sysrq_tg_reg));
> +}
> +
> +module_init(sysrq_tg_init);
> +module_exit(sysrq_tg_exit);
> +MODULE_DESCRIPTION("Xtables: triggering SYSRQ remotely");
> +MODULE_AUTHOR("Jan Engelhardt <jengelh@medozas.de>");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("ipt_SYSRQ");
> +MODULE_ALIAS("ip6t_SYSRQ");

--
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 1/2] netfilter: xtables: inclusion of xt_SYSRQ

[Jan Engelhardt]

On Wednesday 2010-04-21 14:59, Patrick McHardy wrote:

>Jan Engelhardt wrote:
>> The SYSRQ target will allow to remotely invoke sysrq on the local
>> machine. Authentication is by means of a pre-shared key that can
>> either be transmitted plaintext or digest-secured.
>
>I really think this is pushing what netfilter is meant for a bit
>far. Its basically abusing the firewall ruleset to offer a network
>service.
>
>I can see that its useful to have this in the kernel instead of
>userspace, but why isn't this implemented as a stand-alone module?
>That seems like a better design to me and also makes it more useful
>by not depending on netfilter.

That sort of diverts from the earlier what-seemed-to-be-consensus.

Oh well, I would not mind holding the single commit up as long as the 
rest isn't blocked too :-)

Re: [PATCH 1/2] netfilter: xtables: inclusion of xt_SYSRQ

[Patrick McHardy]

Jan Engelhardt wrote:
> On Wednesday 2010-04-21 14:59, Patrick McHardy wrote:
> 
>> Jan Engelhardt wrote:
>>> The SYSRQ target will allow to remotely invoke sysrq on the local
>>> machine. Authentication is by means of a pre-shared key that can
>>> either be transmitted plaintext or digest-secured.
>> I really think this is pushing what netfilter is meant for a bit
>> far. Its basically abusing the firewall ruleset to offer a network
>> service.
>>
>> I can see that its useful to have this in the kernel instead of
>> userspace, but why isn't this implemented as a stand-alone module?
>> That seems like a better design to me and also makes it more useful
>> by not depending on netfilter.
> 
> That sort of diverts from the earlier what-seemed-to-be-consensus.
> 
> Oh well, I would not mind holding the single commit up as long as the 
> rest isn't blocked too :-)

Then lets skip this one for now.

Re: [PATCH 1/2] netfilter: xtables: inclusion of xt_SYSRQ

[Jan Engelhardt]

On Wednesday 2010-04-21 15:17, Patrick McHardy wrote:
>Jan Engelhardt wrote:
>> On Wednesday 2010-04-21 14:59, Patrick McHardy wrote:
>> 
>>> Jan Engelhardt wrote:
>>>> The SYSRQ target will allow to remotely invoke sysrq on the local
>>>> machine. Authentication is by means of a pre-shared key that can
>>>> either be transmitted plaintext or digest-secured.
>>> I really think this is pushing what netfilter is meant for a bit
>>> far. Its basically abusing the firewall ruleset to offer a network
>>> service.
>>>
>>> I can see that its useful to have this in the kernel instead of
>>> userspace, but why isn't this implemented as a stand-alone module?
>>> That seems like a better design to me and also makes it more useful
>>> by not depending on netfilter.
>> 
>> That sort of diverts from the earlier what-seemed-to-be-consensus.
>> 
>> Oh well, I would not mind holding the single commit up as long as the 
>> rest isn't blocked too :-)
>
>Then lets skip this one for now.

Well you raised the concern before -- namely that kdboe would have
the very same feature. And yet, kdboe was not part of the kernel.
Neither is the magical stand-alone module.
I really prefer to have it in rather than out, because I know
that's going to mess up maintenance-here-and-there. I'm already
having a big time with xtables-addons that still carries
xt_condition and SYSRQ for a while, and it does have some different
code lines than the kernel copy.

Re: [PATCH 1/2] netfilter: xtables: inclusion of xt_SYSRQ

[John Haxby]

On 21/04/10 14:35, Jan Engelhardt wrote:
> On Wednesday 2010-04-21 15:17, Patrick McHardy wrote:
>    
>> Jan Engelhardt wrote:
>>      
>>> On Wednesday 2010-04-21 14:59, Patrick McHardy wrote:
>>>
>>>        
>>>> Jan Engelhardt wrote:
>>>>          
>>>>> The SYSRQ target will allow to remotely invoke sysrq on the local
>>>>> machine. Authentication is by means of a pre-shared key that can
>>>>> either be transmitted plaintext or digest-secured.
>>>>>            
>>>> I really think this is pushing what netfilter is meant for a bit
>>>> far. Its basically abusing the firewall ruleset to offer a network
>>>> service.
>>>>
>>>> I can see that its useful to have this in the kernel instead of
>>>> userspace, but why isn't this implemented as a stand-alone module?
>>>> That seems like a better design to me and also makes it more useful
>>>> by not depending on netfilter.
>>>>          
>>> That sort of diverts from the earlier what-seemed-to-be-consensus.
>>>
>>> Oh well, I would not mind holding the single commit up as long as the
>>> rest isn't blocked too :-)
>>>        
>> Then lets skip this one for now.
>>      
> Well you raised the concern before -- namely that kdboe would have
> the very same feature. And yet, kdboe was not part of the kernel.
> Neither is the magical stand-alone module.
> I really prefer to have it in rather than out, because I know
> that's going to mess up maintenance-here-and-there. I'm already
> having a big time with xtables-addons that still carries
> xt_condition and SYSRQ for a while, and it does have some different
> code lines than the kernel copy.
>    

I have to agree with Jan here, but I'd like to raise some additional points.

kdboe (or kgdboe) isn't part of the kernel and I don't think it 
necessarily fits all the use cases for xt_SYSRQ.  The one I have in mind 
is where there is a non-kernel hacker whose machine has got into 
trouble.  The poor harrassed sys admin (in this case) has configured 
netconsole and knows that sysrq-t and sysrq-m are useful as a first 
attempt at passing useful information to someone who knows what might be 
going on and that sysrq-c to get a crash dump will also be useful.   
(This represents quite a few of the better sys admins that I come 
across.)   xt_SYSRQ is likewise easy to set up and easy to use.   It's 
true that k(g)dboe would provide this kind of information provided that 
the debuginfo was present on the target machine and the environment was 
such that any sort of debugging over netconsole was sufficiently secure 
... (is it at least as secure as the xt_SYSRQ controls?)

I was running over the design of a standalone module in my head on the 
way in this morning.   It seems fairly straightforward, but as I started 
adding in necessary requirements like limited IP addresses (which I know 
are not actually secure), limited interfaces (which are more secure in a 
controlled physical environment), user-space control and so on the more 
it was sounding as though it would just be a cut-down iptables.   And 
then, of course, that begs the question "why don't you leave all that 
extra stuff to iptables?"

My own interest in getting xt_SYSRQ into the mainline kernel is that it 
would then be easier to get it accepted in production kernels where it 
would make the poor beleaguered sys admin's life a little easier.  That 
is, _some_ useful information or even a crash dump could be extracted 
from the machine before it's big red button time.

jch

Re: [PATCH 1/2] netfilter: xtables: inclusion of xt_SYSRQ

[John Haxby]

On 28/04/10 15:43, John Haxby wrote:
>
> kdboe (or kgdboe) isn't part of the kernel and I don't think it 
> necessarily fits all the use cases for xt_SYSRQ.  The one I have in 
> mind is where there is a non-kernel hacker whose machine has got into 
> trouble.  The poor harrassed sys admin (in this case) has configured 
> netconsole and knows that sysrq-t and sysrq-m are useful as a first 
> attempt at passing useful information to someone who knows what might 
> be going on and that sysrq-c to get a crash dump will also be 
> useful.   (This represents quite a few of the better sys admins that I 
> come across.)   xt_SYSRQ is likewise easy to set up and easy to use.   
> It's true that k(g)dboe would provide this kind of information 
> provided that the debuginfo was present on the target machine and the 
> environment was such that any sort of debugging over netconsole was 
> sufficiently secure ... (is it at least as secure as the xt_SYSRQ 
> controls?)
>

I really must read what I've written more carefully.   I should have 
gone on to say that I don't see that k(g)dboe will be viable in this use 
case although for someone actually debugging a kernel on a machine that 
they have access to xt_SYSRQ leaves an awful lot to be desired :-)   But 
that isn't the common use-case I see -- the one I see is where the sys 
admins used to have a "crash trolley" which was a console and PS/2 
keyboard which they could plug into a machine to get some information, 
but as many rack machines no longer have anything PS/2 and USB hot plug 
is unlikely to work on a sick machine we need a sufficiently light 
mechanism that it will work in most cases (xt_SYSRQ is careful to 
pre-allocate most of the resources it will need).


And then I should have said that moving on to the possibility of a 
standalone module and that ...
> I was running over the design of a standalone module in my head on the 
> way in this morning.   It seems fairly straightforward, but as I 
> started adding in necessary requirements like limited IP addresses 
> (which I know are not actually secure), limited interfaces (which are 
> more secure in a controlled physical environment), user-space control 
> and so on the more it was sounding as though it would just be a 
> cut-down iptables.   And then, of course, that begs the question "why 
> don't you leave all that extra stuff to iptables?"

So unless I'm missing something obvious and different, I don't see that 
a standalone module is going to be lightweight enough to be acceptable.


Sorry for not making filling this parts in earlier.

jch

Re: [PATCH 1/2] netfilter: xtables: inclusion of xt_SYSRQ

[Jan Engelhardt]

On Wednesday 2010-04-28 16:54, John Haxby wrote:
>
> use-case I see -- the one I see is where the sys admins used to have a "crash
> trolley" which was a console and PS/2 keyboard which they could plug into a
> machine to get some information, but as many rack machines no longer have
> anything PS/2 and USB hot plug is unlikely to work on a sick machine

Oh I can tell you stories... sometimes it's so dead in the water that
the console unblanking would not work any more, rendering even any PS/2
useless. Stupid southbridge chipsets blowing up DMA :-)

Re: [PATCH 1/2] netfilter: xtables: inclusion of xt_SYSRQ

[John Haxby]

On 28/04/10 16:03, Jan Engelhardt wrote:
> On Wednesday 2010-04-28 16:54, John Haxby wrote:
>    
>> use-case I see -- the one I see is where the sys admins used to have a "crash
>> trolley" which was a console and PS/2 keyboard which they could plug into a
>> machine to get some information, but as many rack machines no longer have
>> anything PS/2 and USB hot plug is unlikely to work on a sick machine
>>      
> Oh I can tell you stories... sometimes it's so dead in the water that
> the console unblanking would not work any more, rendering even any PS/2
> useless. Stupid southbridge chipsets blowing up DMA :-)
>    

There's no hope in that case :-)  Just take the machine out and give it 
a decent burial.

On the other hand it's not uncommon to see reports that a machine has 
"hung totally" that include output from ping to show that it hasn't.  
Actually it's amazingly common to see this.  And in just this situation 
xt_SYSRQ is still quite likely to work.

jch

Re: [PATCH 1/2] netfilter: xtables: inclusion of xt_SYSRQ

[Jan Engelhardt]

On Wednesday 2010-04-28 17:03, Jan Engelhardt wrote:
>On Wednesday 2010-04-28 16:54, John Haxby wrote:
>>
>> use-case I see -- the one I see is where the sys admins used to have a "crash
>> trolley" which was a console and PS/2 keyboard which they could plug into a
>> machine to get some information, but as many rack machines no longer have
>> anything PS/2 and USB hot plug is unlikely to work on a sick machine
>

I still think we should merge this. A hold-up like this would have never 
happened with staging drivers!

Re: [PATCH 1/2] netfilter: xtables: inclusion of xt_SYSRQ

[John Haxby]

On 25 Jul 2010, at 17:49, Jan Engelhardt wrote:

> 
> On Wednesday 2010-04-28 17:03, Jan Engelhardt wrote:
>> On Wednesday 2010-04-28 16:54, John Haxby wrote:
>>> 
>>> use-case I see -- the one I see is where the sys admins used to have a "crash
>>> trolley" which was a console and PS/2 keyboard which they could plug into a
>>> machine to get some information, but as many rack machines no longer have
>>> anything PS/2 and USB hot plug is unlikely to work on a sick machine
>> 
> 
> I still think we should merge this. A hold-up like this would have never 
> happened with staging drivers!
> 

Me too.   I've been caught up with other things, but Patrick's suggestion of a separate module only half worked out.

Using encapsulation sockets, to get the sysrq handled in BH context works well except that there are no encapsulation sockets for IPv6.  That, for me at least was a bit of a show stopper.

In exploring this, though, I did correct one weakness in the protocol.  An opportunistic hacker could take a sysrq packet and replay it to other hosts in the LAN in the hope that they have the same password (this is a realistic weakness rather than a theoretical one).   To counter this I simply added the target IP address to the hash.

Would you like me to submit that to xt_SYSRQ anyway?   (In a couple of weeks I'm afraid, I'm out for a while.)

jch

Re: [PATCH 1/2] netfilter: xtables: inclusion of xt_SYSRQ

[Shan Wei]

Patrick McHardy wrote, at 2010/4/21 20:59:

> I really think this is pushing what netfilter is meant for a bit
> far. Its basically abusing the firewall ruleset to offer a network
> service.
> 
> I can see that its useful to have this in the kernel instead of
> userspace, but why isn't this implemented as a stand-alone module?
> That seems like a better design to me and also makes it more useful
> by not depending on netfilter.


It's very useful to remotely sysrq server machines in large data-center.
Another solution by Prarit Bhargava is denied by David Miller,
see discussion http://patchwork.ozlabs.org/patch/101284/.

xt_SYSRQ solution is more convenience for us to use in inner system.
I hope this solution can be merged to main tree. :-)
But before that, this code need to be fixed (may be need more hashing enhancement).

> 
>> Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
>> ---
>> +module_param_string(password, sysrq_password, sizeof(sysrq_password),
>> +	S_IRUSR | S_IWUSR);
>> +module_param_string(hash, sysrq_hash, sizeof(sysrq_hash), S_IRUSR);

Read only.
hash algorithm can be changed.

>> +module_param_named(seqno, sysrq_seqno, long, S_IRUSR | S_IWUSR);

sysrq_seqno is set using current time in sysrq_crypto_init.
Whatever we set, sysrq_seqno is not equal to what we set.

>> +	if (len == 0)
>> +		return NF_DROP;

malformed packet should be delivered to udp protocol handing.
 


>> +	if (i != 3) {
>> +		if (sysrq_debug)
>> +			pr_info("badly formatted request\n");
>> +		return NF_DROP;

Is there 1% possibility for user to send x,x,x,x, type data in udp payload?

>> +	do_gettimeofday(&now);
>> +	sysrq_seqno = now.tv_sec;
>> +	ret = xt_register_targets(sysrq_tg_reg, ARRAY_SIZE(sysrq_tg_reg));

register target.

>> +static int __init sysrq_tg_init(void)
>> +{
>> +	if (sysrq_crypto_init() < 0)
>> +		pr_info("starting without crypto\n");
>> +	return xt_register_targets(sysrq_tg_reg, ARRAY_SIZE(sysrq_tg_reg));

register again. This will cause target list dead loop.

--
Best Regards
Shan Wei