Bug 254924

Summary:

stp does not validate timer values in config BPDU

Product:

Base System

Reporter:

jcaplan

Component:

kern

Assignee:

Kristof Provost <kp>

Status:

Closed FIXED

Severity:

Affects Only Me

CC:

Priority:

---

Version:

CURRENT

Hardware:

Any

OS:

Any

Attachments:

Description	Flags
proposed patch	none

Description jcaplan 2021-04-09 17:24:03 UTC

Created attachment 223952 [details]
proposed patch

Overview
--------

IEEE Std 802.1D-2004 Section 17.14 defines permitted ranges for timers. Incoming BPDU messages should be checked against the permitted ranges. The rest of 17.14 appears to be enforced already.

Steps to Reproduce
------------------

Send an invalid config with scapy (maxage > 40, fwddelay > 30):

>>stp = Ether(src="00:0c:29:0b:91:0a",dst="01:80:C2:00:00:00")/LLC()/STP(proto=0,rootid=32768,rootmac="00:0c:29:01:01:01",bridgeid=32768,bridgemac="00:0c:29:01:01:01",portid=0x8007,maxage=50,hellotime=2,fwddelay=40)

>>sendp(stp,inter=1./1,iface="em1",loop=1)

Actual Results
--------------
tcpdump shows configuration is accepted and forwarded by other ports:

tcpdump: listening on em1, link-type EN10MB (Ethernet), capture size 262144 bytes
17:35:34.930786 STP 802.1d, Config, Flags [none], bridge-id 8000.00:0c:29:c8:34:91.8002, length 43
message-age 2.00s, max-age 50.00s, hello-time 2.00s, forwarding-delay 40.00s


Expected Results
----------------
The invalid config is discarded

Comment 1 Kristof Provost freebsd_committer

2021-04-14 21:39:49 UTC

Is there a public version of the relevant standard anywhere?

Comment 2 Kristof Provost freebsd_committer

2021-04-15 08:30:02 UTC

(In reply to Kristof Provost from comment #1)
Never mind, I've found it on https://ieeexplore.ieee.org/document/1309630

(Elide rant about IEEE hiding their standards.)

Comment 3 Kristof Provost freebsd_committer

2021-04-15 17:37:22 UTC

Slightly altered patch: https://reviews.freebsd.org/D29782
And test: https://reviews.freebsd.org/D29783

Comment 4 commit-hook freebsd_committer

2021-04-19 12:10:49 UTC

A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=0e4025bffa2bab3461b72b40d0b1468722ff76e6

commit 0e4025bffa2bab3461b72b40d0b1468722ff76e6
Author:     Jonah Caplan <jcaplan@blackberry.com>
AuthorDate: 2021-04-15 09:28:42 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-04-19 10:09:18 +0000

    bridgestp: validate timer values in config BPDU

    IEEE Std 802.1D-2004 Section 17.14 defines permitted ranges for timers.
    Incoming BPDU messages should be checked against the permitted ranges.
    The rest of 17.14 appears to be enforced already.

    PR:             254924
    Reviewed by:    kp, donner
    Differential Revision:  https://reviews.freebsd.org/D29782

 sys/net/bridgestp.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comment 5 commit-hook freebsd_committer

2021-04-19 12:10:50 UTC

A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=4ae3a97e127cea14277b904af31483af7e6e2891

commit 4ae3a97e127cea14277b904af31483af7e6e2891
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2021-04-15 12:55:00 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-04-19 10:09:35 +0000

    bridge tests: Test STP config BPDU validation

    PR:             254924
    Reviewed by:    donner
    Differential Revision:  https://reviews.freebsd.org/D29783

 tests/sys/net/Makefile          |   7 ++-
 tests/sys/net/if_bridge_test.sh |  45 ++++++++++++++++
 tests/sys/net/stp.py (new)      | 112 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 163 insertions(+), 1 deletion(-)

Comment 6 commit-hook freebsd_committer

2021-05-18 12:19:20 UTC

A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e8889c723a816c1407c5de7cc812dcd5fce42c34

commit e8889c723a816c1407c5de7cc812dcd5fce42c34
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2021-04-15 12:55:00 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-05-18 12:17:47 +0000

    bridge tests: Test STP config BPDU validation

    PR:             254924
    Reviewed by:    donner
    Differential Revision:  https://reviews.freebsd.org/D29783

    (cherry picked from commit 4ae3a97e127cea14277b904af31483af7e6e2891)

 tests/sys/net/Makefile          |   7 ++-
 tests/sys/net/if_bridge_test.sh |  45 ++++++++++++++++
 tests/sys/net/stp.py (new)      | 112 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 163 insertions(+), 1 deletion(-)

Comment 7 commit-hook freebsd_committer

2021-05-18 12:19:21 UTC

A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=ad27abda399a103276a04c0a17f966c861a4e836

commit ad27abda399a103276a04c0a17f966c861a4e836
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2021-04-15 12:55:00 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-05-18 10:00:38 +0000

    bridge tests: Test STP config BPDU validation

    PR:             254924
    Reviewed by:    donner
    Differential Revision:  https://reviews.freebsd.org/D29783

    (cherry picked from commit 4ae3a97e127cea14277b904af31483af7e6e2891)

 tests/sys/net/Makefile          |   7 ++-
 tests/sys/net/if_bridge_test.sh |  45 ++++++++++++++++
 tests/sys/net/stp.py (new)      | 112 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 163 insertions(+), 1 deletion(-)

Comment 8 commit-hook freebsd_committer

2021-05-18 12:19:22 UTC

A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=dbfd8660a96df693b66e9f13c70ca4302d2bfa84

commit dbfd8660a96df693b66e9f13c70ca4302d2bfa84
Author:     Jonah Caplan <jcaplan@blackberry.com>
AuthorDate: 2021-04-15 09:28:42 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-05-18 12:17:46 +0000

    bridgestp: validate timer values in config BPDU

    IEEE Std 802.1D-2004 Section 17.14 defines permitted ranges for timers.
    Incoming BPDU messages should be checked against the permitted ranges.
    The rest of 17.14 appears to be enforced already.

    PR:             254924
    Reviewed by:    kp, donner
    Differential Revision:  https://reviews.freebsd.org/D29782

    (cherry picked from commit 0e4025bffa2bab3461b72b40d0b1468722ff76e6)

 sys/net/bridgestp.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comment 9 commit-hook freebsd_committer

2021-05-18 12:19:23 UTC

A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=61d771b63df62e4e8764b187c1307a87933248ef

commit 61d771b63df62e4e8764b187c1307a87933248ef
Author:     Jonah Caplan <jcaplan@blackberry.com>
AuthorDate: 2021-04-15 09:28:42 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-05-18 10:00:38 +0000

    bridgestp: validate timer values in config BPDU

    IEEE Std 802.1D-2004 Section 17.14 defines permitted ranges for timers.
    Incoming BPDU messages should be checked against the permitted ranges.
    The rest of 17.14 appears to be enforced already.

    PR:             254924
    Reviewed by:    kp, donner
    Differential Revision:  https://reviews.freebsd.org/D29782

    (cherry picked from commit 0e4025bffa2bab3461b72b40d0b1468722ff76e6)

 sys/net/bridgestp.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)