Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[BUG] Commissioning window timeout can be set to the value incompatible with the spec #35505

Closed
kkasperczyk-no opened this issue Sep 10, 2024 · 3 comments · Fixed by #35507
Closed

[BUG] Commissioning window timeout can be set to the value incompatible with the spec #35505

kkasperczyk-no opened this issue Sep 10, 2024 · 3 comments · Fixed by #35507
Labels
bug Something isn't working needs triage

Comments

@kkasperczyk-no
Copy link
Contributor

kkasperczyk-no commented Sep 10, 2024 •
edited
Loading

Reproduction steps

The CommissioningWindorManager::MaxCommissioningTimeout is used to determine the maximum allowed commissioning timeout used in the CommissioningWindowManager::OpenCommissioningWindow method. This method was modified btw of adding BLE extended announcement feature: ca17912#diff-d6f2798a18ed8d25dbb88df7816ca0584e4d6869dc2e4618dc74ee494db7f314R64 and if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING is enabled, the returned value is equal to 48 hours.

This makes the implementation return timeout equal to 48 hours, even for the commissioned device, and the spec does not allow the window to exceed 900 s in such scenario.

It makes the CADMIN 1.21 i 1.22 certification tests fail.

Bug prevalence

Always

GitHub hash of the SDK that was being used

83d345e (master)

Platform

core

Platform Version(s)

v1.3.0

Anything else?

No response
@kkasperczyk-no kkasperczyk-no added bug Something isn't working needs triage labels Sep 10, 2024
kkasperczyk-no added a commit to kkasperczyk-no/connectedhomeip that referenced this issue Sep 10, 2024
@kkasperczyk-no
Fix max timeout for open commissioning window
98bf9d5 
Commissioning window can be opened using timeout exceeding
the maximum value of 900 s defined by the spec. This can happen
if selected transport is IP, but the device uses BLE extended
announcement feature.

Fixes: project-chip#35505
kkasperczyk-no added a commit to kkasperczyk-no/connectedhomeip that referenced this issue Sep 10, 2024
@kkasperczyk-no
Fix max timeout for open commissioning window
de624ce 
Commissioning window can be opened using timeout exceeding
the maximum value of 900 s defined by the spec. This can happen
if selected transport is IP, but the device uses BLE extended
announcement feature.

Added isBle flag to be able to determine what max timeout should
be used for the particular scenario.

Fixes: project-chip#35505
@kkasperczyk-no kkasperczyk-no mentioned this issue Sep 10, 2024
Merged
kkasperczyk-no added a commit to kkasperczyk-no/connectedhomeip that referenced this issue Sep 10, 2024
@kkasperczyk-no
Fix max timeout for open commissioning window
aad9bd5 
Commissioning window can be opened using timeout exceeding
the maximum value of 900 s defined by the spec. This can happen
if selected transport is IP, but the device uses BLE extended
announcement feature.

Added isBle flag to be able to determine what max timeout should
be used for the particular scenario.

Fixes: project-chip#35505
@bzbarsky-apple
Copy link
Contributor

For the IP transport, the spec does not allow the window to exceed 900 s.

Yes, it does, for uncommissioned devices already on the IP network. The "Extended Announcement" feature in the spec is transport-independent.

@kkasperczyk-no
Copy link
Contributor Author

@bzbarsky-apple thank you for the clarification. We must have had some misunderstanding of this feature in the verification area. Maybe because the spec in the past mentioned about "unnecessary pollution of the crowded 2.4 GHz wireless spectrum, especially with BLE" and in our heads it turned out into BLE related :)

kkasperczyk-no added a commit to kkasperczyk-no/connectedhomeip that referenced this issue Sep 11, 2024
@kkasperczyk-no
Fix max timeout for open commissioning window
339f980 
Commissioning window can be opened using timeout exceeding
the maximum value of 900 s defined by the spec. This can happen
if selected transport is IP, but the device uses BLE extended
announcement feature.

Added checking if device is commissioned to be able to determine
what max timeout should be used for the particular scenario.

Fixes: project-chip#35505
kkasperczyk-no added a commit to kkasperczyk-no/connectedhomeip that referenced this issue Sep 11, 2024
@kkasperczyk-no
Fix max timeout for open commissioning window
870ac49 
Commissioning window can be opened using timeout exceeding
the maximum value of 900 s defined by the spec. This can happen
if selected transport is IP, but the device uses BLE extended
announcement feature.

Added checking if device is commissioned to be able to determine
what max timeout should be used for the particular scenario.

Fixes: project-chip#35505
@kkasperczyk-no
Copy link
Contributor Author

Or it's the CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING config name in the SDK that suggests it's related to BLE only.

kkasperczyk-no added a commit to kkasperczyk-no/connectedhomeip that referenced this issue Sep 11, 2024
@kkasperczyk-no
Fix max timeout for open commissioning window
51cd0a2 
Commissioning window can be opened using timeout exceeding
the maximum value of 900 s defined by the spec. This can happen
if selected transport is IP, but the device uses BLE extended
announcement feature.

Added checking if device is commissioned to be able to determine
what max timeout should be used for the particular scenario.

Fixes: project-chip#35505
kkasperczyk-no added a commit to kkasperczyk-no/connectedhomeip that referenced this issue Sep 11, 2024
@kkasperczyk-no
Fix max timeout for open commissioning window
ab896f8 
Commissioning window can be opened using timeout exceeding
the maximum value of 900 s defined by the spec. This can happen
if selected transport is IP, but the device uses BLE extended
announcement feature.

Added checking if device is commissioned to be able to determine
what max timeout should be used for the particular scenario.

Renamed CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING to the
CHIP_DEVICE_CONFIG_EXT_ADVERTISING, as config name sounds
misleading and it seems it relates only to BLE.

Fixes: project-chip#35505
@mergify mergify bot closed this as completed in #35507 Sep 16, 2024
@mergify mergify bot closed this as completed in ba9faf2 Sep 16, 2024
maciejbaczmanski pushed a commit to maciejbaczmanski/connectedhomeip that referenced this issue Nov 5, 2024
@kkasperczyk-no
[nrf fromtree] Fix max timeout for open commissioning window (project…
7c2da28 
…-chip#35507)

Commissioning window can be opened using timeout exceeding
the maximum value of 900 s defined by the spec. This can happen
if selected transport is IP, but the device uses BLE extended
announcement feature.

Added checking if device is commissioned to be able to determine
what max timeout should be used for the particular scenario.

Renamed CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING to the
CHIP_DEVICE_CONFIG_EXT_ADVERTISING, as config name sounds
misleading and it seems it relates only to BLE.

Fixes: project-chip#35505
yyzhong-g pushed a commit to yyzhong-g/connectedhomeip that referenced this issue Dec 12, 2024
@kkasperczyk-no @yyzhong-g
Fix max timeout for open commissioning window (project-chip#35507)
4220294 
Commissioning window can be opened using timeout exceeding
the maximum value of 900 s defined by the spec. This can happen
if selected transport is IP, but the device uses BLE extended
announcement feature.

Added checking if device is commissioned to be able to determine
what max timeout should be used for the particular scenario.

Renamed CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING to the
CHIP_DEVICE_CONFIG_EXT_ADVERTISING, as config name sounds
misleading and it seems it relates only to BLE.

Fixes: project-chip#35505
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working needs triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix max timeout for open commissioning window kkasperczyk-no/connectedhomeip
2 participants
@bzbarsky-apple @kkasperczyk-no