Bug 254924 - stp does not validate timer values in config BPDU
Summary: stp does not validate timer values in config BPDU
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Kristof Provost
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-04-09 17:24 UTC by jcaplan
Modified: 2021-05-18 12:19 UTC (History)
1 user (show)

See Also:


Attachments
proposed patch (657 bytes, patch)
2021-04-09 17:24 UTC, jcaplan
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 freebsd_triage 2021-04-14 21:39:49 UTC
Is there a public version of the relevant standard anywhere?
Comment 2 Kristof Provost freebsd_committer freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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(+)