bgpd: Ensure stream received has enough data #12454
Conversation
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6U18AMD64-8716/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 6 Topotests Ubuntu 18.04 arm8 part 5: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 5: No useful log foundTopotests Ubuntu 18.04 i386 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6U18I386-8716/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 6 Topotests Ubuntu 18.04 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5U18AMD64-8716/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 5 Topotests debian 10 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6DEB10AMD64-8716/test Topology Tests failed for Topotests debian 10 amd64 part 6 Topotests Ubuntu 18.04 arm8 part 6: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 6: No useful log foundTopotests debian 10 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5DEB10AMD64-8716/test Topology Tests failed for Topotests debian 10 amd64 part 5 Topotests Ubuntu 18.04 i386 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5U18I386-8716/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 5 Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6U18AMD64-8716/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 6 Topotests Ubuntu 18.04 arm8 part 5: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 5: No useful log foundTopotests Ubuntu 18.04 i386 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6U18I386-8716/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 6 Topotests Ubuntu 18.04 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5U18AMD64-8716/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 5 Topotests debian 10 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6DEB10AMD64-8716/test Topology Tests failed for Topotests debian 10 amd64 part 6 Topotests Ubuntu 18.04 arm8 part 6: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 6: No useful log foundTopotests debian 10 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5DEB10AMD64-8716/test Topology Tests failed for Topotests debian 10 amd64 part 5 Topotests Ubuntu 18.04 i386 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5U18I386-8716/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 5 |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6U18AMD64-8720/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 6 Topotests Ubuntu 18.04 arm8 part 5: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 5: No useful log foundTopotests Ubuntu 18.04 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5U18AMD64-8720/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 5 Topotests debian 10 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5DEB10AMD64-8720/test Topology Tests failed for Topotests debian 10 amd64 part 5 Topotests Ubuntu 18.04 i386 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6U18I386-8720/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 6 Topotests Ubuntu 18.04 arm8 part 6: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 6: No useful log foundTopotests debian 10 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6DEB10AMD64-8720/test Topology Tests failed for Topotests debian 10 amd64 part 6 Topotests Ubuntu 18.04 i386 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5U18I386-8720/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 5 Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6U18AMD64-8720/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 6 Topotests Ubuntu 18.04 arm8 part 5: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 5: No useful log foundTopotests Ubuntu 18.04 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5U18AMD64-8720/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 5 Topotests debian 10 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5DEB10AMD64-8720/test Topology Tests failed for Topotests debian 10 amd64 part 5 Topotests Ubuntu 18.04 i386 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6U18I386-8720/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 6 Topotests Ubuntu 18.04 arm8 part 6: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 6: No useful log foundTopotests debian 10 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6DEB10AMD64-8720/test Topology Tests failed for Topotests debian 10 amd64 part 6 Topotests Ubuntu 18.04 i386 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5U18I386-8720/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 5 |
bgpd/bgp_attr.c
Outdated
| else if (type == BGP_PREFIX_SID_SRV6_L3_SERVICE) { | ||
| if (STREAM_READABLE(peer->curr) < length) { | ||
| } else if (type == BGP_PREFIX_SID_SRV6_L3_SERVICE) { | ||
| if (STREAM_READABLE(peer->curr) != 1) { |
There was a problem hiding this comment.
Shouldn't be <1 here?
We are processing the BGP_PREFIX_SID_SRV6_L3_SERVICE TLV, which has a 1-byte RESERVED field and a variable-length SRv6 Service Sub-TLVs field.
Here we ensure that we have at least 1 byte (for the RESERVED field). And then we call bgp_attr_srv6_service() to validate and parse the SRv6 Service Sub-TLVs.
There was a problem hiding this comment.
I'm wondering if this is related to the failing topo tests, as well ... it looks like routes are missing, which makes me think we're not processing all the routes being sent to us (?)
|
rerunning to see if we can clean this up ... |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 arm8 part 6: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 6: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-9275/artifact/TOPO6U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 6: No useful log foundTopotests debian 10 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6DEB10AMD64-9275/test Topology Tests failed for Topotests debian 10 amd64 part 6 Topotests Ubuntu 18.04 i386 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5U18I386-9275/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 5 Topotests debian 10 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5DEB10AMD64-9275/test Topology Tests failed for Topotests debian 10 amd64 part 5 Topotests Ubuntu 18.04 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6U18AMD64-9275/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 6 Topotests debian 10 amd64 part 9: Failed (click for details)Topotests debian 10 amd64 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-9275/artifact/TOPO9DEB10AMD64/TopotestLogs/ Topotests debian 10 amd64 part 9: No useful log foundTopotests Ubuntu 18.04 i386 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6U18I386-9275/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 6 Topotests Ubuntu 18.04 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5U18AMD64-9275/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 5 Topotests Ubuntu 18.04 arm8 part 5: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 5: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-9275/artifact/TOPO5U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 5: No useful log foundSuccessful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 arm8 part 6: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 6: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-9275/artifact/TOPO6U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 6: No useful log foundTopotests debian 10 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6DEB10AMD64-9275/test Topology Tests failed for Topotests debian 10 amd64 part 6 Topotests Ubuntu 18.04 i386 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5U18I386-9275/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 5 Topotests debian 10 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5DEB10AMD64-9275/test Topology Tests failed for Topotests debian 10 amd64 part 5 Topotests Ubuntu 18.04 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6U18AMD64-9275/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 6 Topotests debian 10 amd64 part 9: Failed (click for details)Topotests debian 10 amd64 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-9275/artifact/TOPO9DEB10AMD64/TopotestLogs/ Topotests debian 10 amd64 part 9: No useful log foundTopotests Ubuntu 18.04 i386 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6U18I386-9275/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 6 Topotests Ubuntu 18.04 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO5U18AMD64-9275/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 5 Topotests Ubuntu 18.04 arm8 part 5: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 5: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-9275/artifact/TOPO5U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 5: No useful log found |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDTest incomplete. See below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: IncompleteAddresssanitizer topotests part 3: Incomplete(check logs for details)Topotests debian 10 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-10388/test Topology Tests failed for Topotests debian 10 amd64 part 9 Successful on other platforms/tests
|
bgpd/bgp_attr.c
Outdated
| if (STREAM_READABLE(peer->curr) < length) { | ||
| flog_err( | ||
| EC_BGP_ATTR_LEN, | ||
| "Prefix SID Originator SRGB specifies length %hu, but only %zu bytes remain", |
There was a problem hiding this comment.
Isn't this a generic error message, not only Originator SRGB-specific?
There was a problem hiding this comment.
I'm not sure what you are asking/implying here. It's a specific error message to when we are parsing the Prefix SID hence the message that way
There was a problem hiding this comment.
I mean this error message is generic and valid not only for BGP_PREFIX_SID_ORIGINATOR_SRGB, but for BGP_PREFIX_SID_IPV6 or any other type?
There was a problem hiding this comment.
Agree with Donatas. Here we are validating a generic TLV of the BGP Prefix-SID attribute, not only the BGP_PREFIX_SID_ORIGINATOR_SRGB TLV.
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-10388/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
bgpd/bgp_attr.c
Outdated
| if (STREAM_READABLE(peer->curr) < length) { | ||
| flog_err( | ||
| EC_BGP_ATTR_LEN, | ||
| "Prefix SID Originator SRGB specifies length %hu, but only %zu bytes remain", |
There was a problem hiding this comment.
Agree with Donatas. Here we are validating a generic TLV of the BGP Prefix-SID attribute, not only the BGP_PREFIX_SID_ORIGINATOR_SRGB TLV.
bgpd/bgp_attr.c
Outdated
| return bgp_attr_malformed(args, | ||
| BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, | ||
| args->total); | ||
| "Prefix SID SRV6 L3 Service claims %hu bytes, but it must be 1 byte", |
There was a problem hiding this comment.
Here the error message seems to be misleading.
We are verifying that we have at least 1 readable byte.
So, shouldn't it be something like Not enough data left. Expected at least 1 byte, available XXX ?
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-10408/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
BGP_PREFIX_SID_SRV6_L3_SERVICE attributes must not fully trust the length value specified in the nlri. Always ensure that the amount of data we need to read can be fullfilled. Reported-by: Iggy Frankovic <iggyfran@amazon.com> Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 i386 part 2: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO2U18I386-10437/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 2 Successful on other platforms/tests
|
|
ci:rerun |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 amd64 part 7: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO7U18AMD64-10443/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 7 Successful on other platforms/tests
|
|
ci:rerun another stupid ci failure |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests debian 10 amd64 part 3: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO3DEB10AMD64-10447/test Topology Tests failed for Topotests debian 10 amd64 part 3 Successful on other platforms/tests
|
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-10447/ This is a comment from an automated CI system. |
|
@Mergifyio backport stable/8.5 stable/8.4 |
✅ Backports have been created
|
bgpd: Ensure stream received has enough data (backport #12454)
bgpd: Ensure stream received has enough data (backport #12454)
BGP_PREFIX_SID_SRV6_L3_SERVICE attributes must not fully trust the length value specified in the nlri. Always ensure that the amount of data we need to read can be fullfilled.
Reported-by: Iggy Frankovic iggyfran@amazon.com
Signed-off-by: Donald Sharp sharpd@nvidia.com
Closes #13099