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

SDL applies PT file received when PTU procedure is not in progress #3076

Closed
aderiabin opened this issue Oct 15, 2019 · 10 comments
Closed

SDL applies PT file received when PTU procedure is not in progress #3076

aderiabin opened this issue Oct 15, 2019 · 10 comments
Assignees
Labels
Projects

Comments

@aderiabin
Copy link

Bug Report

SDL applies PT file received when PTU procedure is not in progress

Precondition
  1. Policy table contains permissions for application (App)
    GetVehicleData RPC is not allowed for App.
  2. HMI and SDL are started
  3. App is registered and activated (PTU is not triggered)
Reproduction Steps
  1. App sends valid "SystemRequest" with PT file which allows GetVehicleData RPC for App
  2. App sends valid GetVehicleData RPC
Expected Behavior

SDL responds on GetVehicleData with DISALLOWED resultCode

Observed Behavior

SDL responds on GetVehicleData with SUCCESS resultCode

OS & Version Information

Logs:
hmi.log
SmartDeviceLinkCore.log

@aderiabin
Copy link
Author

This is not a regression issue and reproduced on a previous SDL release (5.1.3)

@aderiabin
Copy link
Author

@theresalech, Please triage this issue.
Luxoft team is going to prepare a fix.

@theresalech
Copy link
Contributor

@aderiabin we've noted that this is not a regression issue but will work to prioritize reviewing the PR once submitted. Thanks!

@theresalech theresalech added this to Bug Fixes in 6.0.0 Oct 16, 2019
@LitvinenkoIra
Copy link
Contributor

According to current behavior for PROPRIETARY policy flow:

  1. App -> SDL: SystemRequest (PROPRIETARY, fileName) // valid and binary data is not empty.
  2. SDL reads data and stores it by the path in smartDeviceLink.ini file under "SystemFilesPath" parameter.
  3. SDL -> HMI: SystemRequest (PROPRIETARY, fileName, appID)

The diagram: https://smartdevicelink.com/en/guides/hmi/basiccommunication/policyupdate/

But the question is should SDL process SystemRequest and apply the PT snapshot in case if there are no PTU in progress and App in UP_TO_DATE state?

Or SDL should process SystemRequest according to the requirements above even if App in UP_TO_DATE state?

@JackLivio @jacobkeeler @dboltovskyi @aderiabin Would be grateful for any thoughts on this issue. Thank you.

@Jack-Byrne
Copy link
Collaborator

I agree that there is an issue present. Allowing an unsolicited PTU SystemRequest from an app could raise some security concerns if there is no PTU encryption in place.

We could update Core to keep track of the appID it sent the onSystemRequest to and only accept the PTU SystemRequest from that app id. Its not a 100% fix because a bad actor could still receive that first OnSystemRequest.

Even though I agree with this issue, I still want to submit an evolution proposal for a fix to ensure SystemRequests are only accepted from apps during PTU in progress state. The proposal will be for visibility to other SDLC members in case someone's policy flow was defined around being able to send unsolicited PTUs.

@LitvinenkoIra
Copy link
Contributor

@JackLivio thank you for your response. I agree that this issue requires proposal and we from the Luxoft side can prepare it.

@theresalech theresalech removed this from Bug Fixes in 6.0.0 Oct 21, 2019
@LitvinenkoIra
Copy link
Contributor

Closed since Luxoft will prepare a proposal for this issue.

@Jack-Byrne
Copy link
Collaborator

Reopening since a proposal has been entered by Livio and is in review @LitvinenkoIra

@jordynmackool
Copy link

The proposal mentioned in this issue was accepted (#3715)

@iCollin
Copy link
Collaborator

iCollin commented Feb 11, 2022

Closed via #3853

@iCollin iCollin closed this as completed Feb 11, 2022
@theresalech theresalech added this to Features in 8.1.0 via automation Feb 14, 2022
@theresalech theresalech moved this from Features to Completed Bug Fixes/ Enhancements in 8.1.0 Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
8.1.0
Completed Bug Fixes/ Enhancements
Development

No branches or pull requests

6 participants