diff mbox series

[net-next] ipv4: Add support to disable icmp timestamp

Message ID 1557711193-7284-1-git-send-email-chenweilong@huawei.com
State Deferred
Delegated to: David Miller
Headers show
Series [net-next] ipv4: Add support to disable icmp timestamp | expand

Commit Message

chenweilong May 13, 2019, 1:33 a.m. UTC
The remote host answers to an ICMP timestamp request.
This allows an attacker to know the time and date on your host.

This path is an another way contrast to iptables rules:
iptables -A input -p icmp --icmp-type timestamp-request -j DROP
iptables -A output -p icmp --icmp-type timestamp-reply -j DROP

Default is disabled to improve security.

enable:
	sysctl -w net.ipv4.icmp_timestamp_enable=1
disable
	sysctl -w net.ipv4.icmp_timestamp_enable=0
testing:
	hping3 --icmp --icmp-ts -V $IPADDR

Signed-off-by: Weilong Chen <chenweilong@huawei.com>
---
 include/net/ip.h           | 2 ++
 net/ipv4/icmp.c            | 5 +++++
 net/ipv4/sysctl_net_ipv4.c | 8 ++++++++
 3 files changed, 15 insertions(+)

Comments

Michal Kubecek May 13, 2019, 7:49 a.m. UTC | #1
On Mon, May 13, 2019 at 09:33:13AM +0800, Weilong Chen wrote:
> The remote host answers to an ICMP timestamp request.
> This allows an attacker to know the time and date on your host.

Why is that a problem? If it is, does it also mean that it is a security
problem to have your time in sync (because then the attacker doesn't
even need ICMP timestamps to know the time and date on your host)?

> This path is an another way contrast to iptables rules:
> iptables -A input -p icmp --icmp-type timestamp-request -j DROP
> iptables -A output -p icmp --icmp-type timestamp-reply -j DROP
> 
> Default is disabled to improve security.

If we need a sysctl for this (and I'm not convinced we do), I would
prefer preserving current behaviour by default.

Michal Kubecek
chenweilong May 13, 2019, 11:38 a.m. UTC | #2
On 2019/5/13 15:49, Michal Kubecek wrote:
> On Mon, May 13, 2019 at 09:33:13AM +0800, Weilong Chen wrote:
>> The remote host answers to an ICMP timestamp request.
>> This allows an attacker to know the time and date on your host.
>
> Why is that a problem? If it is, does it also mean that it is a security
> problem to have your time in sync (because then the attacker doesn't
> even need ICMP timestamps to know the time and date on your host)?
>
It's a low risk vulnerability(CVE-1999-0524). TCP has 
net.ipv4.tcp_timestamps = 0 to disable it.

>> This path is an another way contrast to iptables rules:
>> iptables -A input -p icmp --icmp-type timestamp-request -j DROP
>> iptables -A output -p icmp --icmp-type timestamp-reply -j DROP
>>
>> Default is disabled to improve security.
>
> If we need a sysctl for this (and I'm not convinced we do), I would
> prefer preserving current behaviour by default.
>
Firewall is not applied to all scenarios.

> Michal Kubecek
>
> .
>

thanks.
Michal Kubecek May 13, 2019, 11:49 a.m. UTC | #3
On Mon, May 13, 2019 at 07:38:37PM +0800, Weilong Chen wrote:
> 
> On 2019/5/13 15:49, Michal Kubecek wrote:
> > On Mon, May 13, 2019 at 09:33:13AM +0800, Weilong Chen wrote:
> > > The remote host answers to an ICMP timestamp request.
> > > This allows an attacker to know the time and date on your host.
> > 
> > Why is that a problem? If it is, does it also mean that it is a security
> > problem to have your time in sync (because then the attacker doesn't
> > even need ICMP timestamps to know the time and date on your host)?
> > 
> It's a low risk vulnerability(CVE-1999-0524). TCP has
> net.ipv4.tcp_timestamps = 0 to disable it.

That does not really answer my question. Even if "CVE" meant much more
back in 1999 than it does these days, none of the CVE-1999-0524
descriptions I found cares to explain why it's considered a problem that
an attacker knows time on your machine. They just claim it is. If we
assume it is a security problem, then we would have to consider having
correct time a security problem which is something I certainly don't
agree with.

One idea is that there may be applications using current time as a seed
for random number generator - but then such application is the real
problem, not having correct time.

Michal Kubecek
chenweilong May 13, 2019, 12:06 p.m. UTC | #4
On 2019/5/13 19:49, Michal Kubecek wrote:
> On Mon, May 13, 2019 at 07:38:37PM +0800, Weilong Chen wrote:
>>
>> On 2019/5/13 15:49, Michal Kubecek wrote:
>>> On Mon, May 13, 2019 at 09:33:13AM +0800, Weilong Chen wrote:
>>>> The remote host answers to an ICMP timestamp request.
>>>> This allows an attacker to know the time and date on your host.
>>>
>>> Why is that a problem? If it is, does it also mean that it is a security
>>> problem to have your time in sync (because then the attacker doesn't
>>> even need ICMP timestamps to know the time and date on your host)?
>>>
>> It's a low risk vulnerability(CVE-1999-0524). TCP has
>> net.ipv4.tcp_timestamps = 0 to disable it.
>
> That does not really answer my question. Even if "CVE" meant much more
> back in 1999 than it does these days, none of the CVE-1999-0524
> descriptions I found cares to explain why it's considered a problem that
> an attacker knows time on your machine. They just claim it is. If we
> assume it is a security problem, then we would have to consider having
> correct time a security problem which is something I certainly don't
> agree with.
>
> One idea is that there may be applications using current time as a seed
> for random number generator - but then such application is the real
> problem, not having correct time.
>
Yes, the target computer responded to an ICMP timestamp request. By 
accurately determining the target's clock state, an attacker can more 
effectively attack certain time-based pseudorandom number generators 
(PRNGs) and the authentication systems that rely on them.

So, the 'time' may become sensitive information. The OS should not leak 
it out.

> Michal Kubecek
>
> .
>
Michal Kubecek May 13, 2019, 12:11 p.m. UTC | #5
On Mon, May 13, 2019 at 08:06:37PM +0800, Weilong Chen wrote:
> On 2019/5/13 19:49, Michal Kubecek wrote:
> > One idea is that there may be applications using current time as a seed
> > for random number generator - but then such application is the real
> > problem, not having correct time.
> > 
> Yes, the target computer responded to an ICMP timestamp request. By
> accurately determining the target's clock state, an attacker can more
> effectively attack certain time-based pseudorandom number generators (PRNGs)
> and the authentication systems that rely on them.
> 
> So, the 'time' may become sensitive information. The OS should not leak it
> out.

So you are effectively saying that having correct time is a security
vulnerability?

I'm sorry but I cannot agree with that. Seeding PRNG with current time
is known to be a bad practice and if some application does it, the
solution is to fix the application, not obfuscating system time.

Michal Kubecek
Florian Westphal May 13, 2019, 12:22 p.m. UTC | #6
Weilong Chen <chenweilong@huawei.com> wrote:
> On 2019/5/13 15:49, Michal Kubecek wrote:
> > On Mon, May 13, 2019 at 09:33:13AM +0800, Weilong Chen wrote:
> > > The remote host answers to an ICMP timestamp request.
> > > This allows an attacker to know the time and date on your host.
> > 
> > Why is that a problem? If it is, does it also mean that it is a security
> > problem to have your time in sync (because then the attacker doesn't
> > even need ICMP timestamps to know the time and date on your host)?
> > 
> It's a low risk vulnerability(CVE-1999-0524). TCP has
> net.ipv4.tcp_timestamps = 0 to disable it.

These are not the same.

TCP timestamps (before pseudo-randomized offset were added) used to leaked
system uptime.

ICMP timestamps "leak" milliseconds since midnight. Don't see how thats
a problem.
chenweilong May 13, 2019, 12:26 p.m. UTC | #7
On 2019/5/13 20:11, Michal Kubecek wrote:
> On Mon, May 13, 2019 at 08:06:37PM +0800, Weilong Chen wrote:
>> On 2019/5/13 19:49, Michal Kubecek wrote:
>>> One idea is that there may be applications using current time as a seed
>>> for random number generator - but then such application is the real
>>> problem, not having correct time.
>>>
>> Yes, the target computer responded to an ICMP timestamp request. By
>> accurately determining the target's clock state, an attacker can more
>> effectively attack certain time-based pseudorandom number generators (PRNGs)
>> and the authentication systems that rely on them.
>>
>> So, the 'time' may become sensitive information. The OS should not leak it
>> out.
>
> So you are effectively saying that having correct time is a security
> vulnerability?
No, I mean that a server should not provide time to others if not necessary.

>
> I'm sorry but I cannot agree with that. Seeding PRNG with current time
> is known to be a bad practice and if some application does it, the
> solution is to fix the application, not obfuscating system time.
>
As I said, users can use Firewall to achieve the purpose. This patch 
just provide a simple way.

I can resubmit this patch and set default to 1, preserving current 
behaviour by default. Does that OK for you?

> Michal Kubecek
>
> .
>
Michal Kubecek May 13, 2019, 12:32 p.m. UTC | #8
On Mon, May 13, 2019 at 08:26:18PM +0800, Weilong Chen wrote:
> On 2019/5/13 20:11, Michal Kubecek wrote:
> > On Mon, May 13, 2019 at 08:06:37PM +0800, Weilong Chen wrote:
> > > On 2019/5/13 19:49, Michal Kubecek wrote:
> > > > One idea is that there may be applications using current time as a seed
> > > > for random number generator - but then such application is the real
> > > > problem, not having correct time.
> > > > 
> > > Yes, the target computer responded to an ICMP timestamp request. By
> > > accurately determining the target's clock state, an attacker can more
> > > effectively attack certain time-based pseudorandom number generators (PRNGs)
> > > and the authentication systems that rely on them.
> > > 
> > > So, the 'time' may become sensitive information. The OS should not leak it
> > > out.
> > 
> > So you are effectively saying that having correct time is a security
> > vulnerability?
> 
> No, I mean that a server should not provide time to others if not necessary.

If the system time is in sync, the attacker knows host's system time
even without ICMP timestamp messages. So if leaking system time via ICMP
timestamp replies is considered a security issue, so must be running
with properly synced system time.

> > I'm sorry but I cannot agree with that. Seeding PRNG with current time
> > is known to be a bad practice and if some application does it, the
> > solution is to fix the application, not obfuscating system time.
> > 
> As I said, users can use Firewall to achieve the purpose. This patch just
> provide a simple way.
> 
> I can resubmit this patch and set default to 1, preserving current behaviour
> by default. Does that OK for you?

I would be less opposed to such version but I'm still not convinced we
do need another sysctl for this. But it's not up to me to decide, it's
just my opinion.

Michal Kubecek
David Miller May 13, 2019, 4:07 p.m. UTC | #9
From: Weilong Chen <chenweilong@huawei.com>
Date: Mon, 13 May 2019 20:06:37 +0800

> So, the 'time' may become sensitive information. The OS should not
> leak it out.

The current time of day is a globally synchronized value everyone on
the planet has access to.

I don't buy this line of reasoning at all, time is not sensitive
information.

I agree with Michal, the apps should not be using time as a seed for
critical calculations that have security implications.

There is no way I am applying this patch, especially as-is.
David Miller May 13, 2019, 4:07 p.m. UTC | #10
From: Michal Kubecek <mkubecek@suse.cz>
Date: Mon, 13 May 2019 14:11:45 +0200

> I'm sorry but I cannot agree with that. Seeding PRNG with current time
> is known to be a bad practice and if some application does it, the
> solution is to fix the application, not obfuscating system time.

+1 +1 +1
diff mbox series

Patch

diff --git a/include/net/ip.h b/include/net/ip.h
index 2d3cce7..71840e4 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -718,6 +718,8 @@  bool icmp_global_allow(void);
 extern int sysctl_icmp_msgs_per_sec;
 extern int sysctl_icmp_msgs_burst;
 
+extern int sysctl_icmp_timestamp_enable;
+
 #ifdef CONFIG_PROC_FS
 int ip_misc_proc_init(void);
 #endif
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index f3a5893..d302189 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -232,6 +232,7 @@  static inline void icmp_xmit_unlock(struct sock *sk)
 
 int sysctl_icmp_msgs_per_sec __read_mostly = 1000;
 int sysctl_icmp_msgs_burst __read_mostly = 50;
+int sysctl_icmp_timestamp_enable __read_mostly;
 
 static struct {
 	spinlock_t	lock;
@@ -953,6 +954,10 @@  static bool icmp_echo(struct sk_buff *skb)
 static bool icmp_timestamp(struct sk_buff *skb)
 {
 	struct icmp_bxm icmp_param;
+
+	if (!sysctl_icmp_timestamp_enable)
+		goto out_err;
+
 	/*
 	 *	Too short.
 	 */
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 875867b..1fe467e 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -544,6 +544,14 @@  static struct ctl_table ipv4_table[] = {
 		.extra1		= &zero,
 	},
 	{
+		.procname	= "icmp_timestamp_enable",
+		.data		= &sysctl_icmp_timestamp_enable,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &zero,
+	},
+	{
 		.procname	= "udp_mem",
 		.data		= &sysctl_udp_mem,
 		.maxlen		= sizeof(sysctl_udp_mem),