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.

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

Core does not respond to StartService with encrypted=true if PTU timed out #3440

Closed
jacobkeeler opened this issue Jun 16, 2020 · 7 comments
Closed

Comments

@jacobkeeler
Copy link
Contributor

Bug Report

If the PTU retry sequence fails when attempting to get a certificate for establishing a secured connection, Core will not respond to the original StartService or inform the HMI that the process failed.

Reproduction Steps
  1. Build Core with EXTENDED_POLICY=EXTERNAL_PROPRIETARY
  2. Set ForceProtectedService = 0x0A, 0x0B in smartDeviceLink.ini
  3. Set the following values in the preloaded PT:
            "timeout_after_x_seconds": 10,
            "seconds_between_retries": [
                1
            ],
...
            "endpoints": {
                "0x07": {
                    "default": [
                        "http://invalid.url/api/1/policies/proprietary"
                    ]
                },
  1. Start Core
  2. Connect an app and attempt to establish a secure video service
  3. Wait for the PTU sequence to time out
Expected Behavior

Core responds to the StartService with StartServiceNAK and sends a service status update to the HMI with REQUEST_REJECTED

Observed Behavior

Core does not send a StartServiceNAK or service status update, REQUEST_RECEIVED update hangs indefinitely

OS & Version Information
  • OS/Version: Ubuntu 18.04
  • SDL Core Version: develop
  • Testing Against: sdl_test_suite for Android
@dboltovskyi
Copy link
Contributor

@jacobkeeler Please notice there is an ATF script to cover this case:
https://github.com/smartdevicelink/sdl_atf_test_scripts/blob/develop/test_scripts/API/ServiceStatusUpdateToHMI/020_Video_service_protected_PTU_FAILED_TimeOut_2_failed_external.lua
It's passed on a current Core develop

Also one thing to notice there is a difference between retry sequences for PROPRIETARY and EXTERNAL_PROPRIETARY policy modes.
For EXTERNAL_PROPRIETARY HMI drives retry sequence. It means SDL waits for BC.OnSystemRequest messages from HMI and count them. If HMI doesn't send such messages SDL won't finish retry sequence by himself.

For PROPRIETARY SDL drives retry sequence hence it is able to finish it by himself.

@jacobkeeler
Copy link
Contributor Author

@dboltovskyi Note that the test case that you reference goes through an extra retry in each PTU retry sequence. 3 attempts should be made, but BC.OnSystemRequest is sent by ATF 4 times, so ATF is not emulating a realistic setup in this case.

@Jack-Byrne
Copy link
Collaborator

@dboltovskyi
Copy link
Contributor

dboltovskyi commented Jun 17, 2020

@jacobkeeler @JackLivio That was initially by design due to differences between policy flows.
As far as I remember EXT flow was originated from Sync project
But I'm not quite sure that this number of retries should be differ for OpenSDL and may be this is a good idea to make it the same.
If so we need to do more updates in ATF scripts:

./test_scripts/API/ServiceStatusUpdateToHMI/020_Video_service_protected_PTU_FAILED_TimeOut_2_failed_external.lua
./test_scripts/API/ServiceStatusUpdateToHMI/021_Video_service_protected_PTU_FAILED_TimeOut_failed_success_failed_external.lua
./test_scripts/API/ServiceStatusUpdateToHMI/022_Video_service_protected_PTU_FAILED_TimeOut_failed_success_2rd_try_failed_external.lua
./test_scripts/Defects/6_1/3136_02_External_Proprietary.lua
./test_scripts/Defects/6_1/3136_05_External_Proprietary_3apps.lua
./test_scripts/Policies/HMI_PTU/009_PTU_retry_sequence_HMI_has_timeout_more_then_SDL_ext.lua
./test_scripts/Policies/HMI_PTU/014_Successful_PTU_after_failed_retry_consequtive_trigger_ext.lua
./test_scripts/Policies/HMI_PTU/016_Successful_PTU_after_failed_retry_parallel_trigger_ext.lua

This list may be not complete, so full regression is required to be sure.

Also Web-HMI (sdl_hmi) needs to be updated accordingly to reflect this change.

@jacobkeeler
Copy link
Contributor Author

jacobkeeler commented Jun 17, 2020

@dboltovskyi The sdl_hmi already follows this flow, I originally encountered the issue while testing against it.

@dboltovskyi
Copy link
Contributor

@jacobkeeler Seems you're right since issue smartdevicelink/sdl_hmi#339 and fix for it smartdevicelink/sdl_hmi#315 are still open.

@jacobkeeler
Copy link
Contributor Author

Closing with the merge of #3441

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants