-
Notifications
You must be signed in to change notification settings - Fork 121
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 0207 - RPC message protection #634
Comments
Question 1: RPC encryption activation timing
This is concerning. The fact that RPC service encryption may take up to a minute, then any app a user activates would appear to the user to be non-responsive. I feel that this would effectively cripple SDL. It is, of course, the prerogative of the OEM to cripple their own system, but nonetheless, we should avoid that if possible. I think the option specified by this proposal (3) is the worst option. It would be better to either (option 2) determine if the app needs RPC protection and to get it that protection before the user interacts with it if possible (and if it's possible for multiple apps to do it at the same time), or (option 1) for the app to be functional immediately, up to the point that the protected RPC is needed. I think that we need to not only consider the technical ramifications of this proposal, but also the user UX ramifications, and that such considerations should be contained within the proposal (e.g. a flow diagram showing the expected user-facing flow). Question 2: Alternative Encrypted RPC Service
I actually like this alternative quite a bit as it provides better separation and cleaner code. Why was the existing solution chosen over this solution? Question 3: Apps that don't use the protected RPCsI don't see a consideration in the proposal for applications that don't use the protected RPCs as seen in The only solution I can think of is for the app to be informed, at runtime, of protected RPCs, and for the app to respond with whether they use any of them. This kind of solution would be rather difficult to use. Let me know if this was covered in the proposal and I missed it, it's quite a large proposal. |
When the app moves from NONE to other HMI states (get activated), it just triggers the process to enable the RPC encryption. For the first time, there are some extra steps for the app to finish, this process needs some time. However, at the same time the un-protected RPC shall be still usable. So app shall not be non-responsive if the app does it in a background task. The first option may cause a delay. Because the app wants to send an encrypted message, but the channel is not ready, it must wait the encryption gets enabled to continue. The second option looks good to me. But for the case that an app get used once or twice, later the app will get registered every time the device get connected, but it never get activated, enable encryption seems a waste of time to me. But it is acceptable. In the third option, when an app get activated, it is highly possible the app will use the RPCs that need encryption. It seems the best time to me.
Personally, I actually like this alternative too. We didn't choose it because it requires a new RPC service. We are afraid of the push back of adding a new service.
It is the policy (OEM or agreement between app developer and OEM) to specify which RPC(s) of which app need protection. The app cannot make a decision by itself. Once policy says a RPC for an app need protection, the app must follow the rule. Otherwise the SDL rejects the RPCs. If the app does not need to protect any RPCs and OEM agrees it, the policy server shall be configured to say the RPCs for the app do not need protection at all. |
The capability of using an encrypted RPC service already exists in sdl core, I'm not sure if it makes sense to create a new service for the same purpose. |
So the app is supposed to disconnect from the HU, reconnect with a new StartService that has the encryption bit set? If that is the case, that will be a very poor UX as hash resumption is not currently implemented into the library and the burden on app developers to do so is extremely high. I would rather see a new control message that allowed a service to enable encryption that starting a service over.
These changes will be both front and and back end changes that will take some time to support. Is the intention to allow an OEM to generally say which RPCs need encryption for all apps, or should an OEM have the ability to control the encryption requirement per RPC per app? The latter is much more complex to implement. Alternative New ServiceI'm torn on this one. I don't like creating a new service just for the sake of encryption when we already have the ability to flag it in the protocol. However, it does make the logic necessary to handle this feature much simpler. In my opinion if my previous comment was addressed (new control message to enable encryption on an established service) that would also simplify the logic.
To Joel's question, if an app never intended to use No Sample Encryption library AndroidI would warn that there is currently no example library of how to build an encryption library for Android. I understand the author stated the "how" is out of scope, but I see that as a concern that should be addressed before this proposal is accepted or implemented. |
The app would open service type 7 unencrypted, followed by a request to open service 7 encrypted. After receiving an ACK from the HU for the protected service type 7, the app can freely send encrypted / unencrytpted RPC's at will. (this is functionality built into sdl core today, no disconnect is required). |
No. The app does not need to disconnect first. The existing unprotected service is still working. We do need a StartService that has the encryption bit set on the existing service. That how it is today. I called it
The goal is for |
So the app sends a StartService unencrypted then leaves the service open but sends another StartService with the encryption bit set? I'm not sure that is covered in the protocol spec. The service remains open and each app should only have a single service of a single type, so sending the StartService again seems very strange. |
Correct as currently implemented in sdl core and the Android proxy. I believe some of the documentation may need to be updated to reflect this. |
@yang1070 The update to the policy server is complex for per app control of RPC per encryption requirement. While the policy table seems straightforward, enabling such control into a UX and database for the policy server will be complex. |
@mrapitis Yea that is not defined that way in the protocol spec. Which is more than documentation, it is the spec and if the code doesn't follow it or operates outside of it, that is a bit of a concern. |
Actually service 10 and 11 can also behave in the same way, if the force protection ini keyword is not turned on for the service, the app can send data encrypted or unencrypted for that service and the service can be started multiple times with encryption on or off. |
I agree this is a very reasonable concern. Without |
It should also be noted that the sample android security library is needed for OEM's making use of any encrypted service including service 10 and 11 -- it is not specific to service type 7 or this proposal. |
@mrapitis You are correct. It is still a concern as it stands. However I believe a proposal like this that would affect all SDL apps vs only VPM the concern grows. I don't believe we need an example before the proposal is accepted, but I would recommend a plan of action on how and when it would be contributed. |
The simplest way to me is to create a new group for each new app (certainly it will increase the db size and the amount of traffic, thus it is not a good solution), in this way for each app, all RPCs are listed and there is a check box to say the PRC needs protection or not. The UI shall be simple and clean. On the other hand, we can relax the encryption requirement to all new apps to certain RPC groups, for example (1) remote control RPCs for security (2) vehicle data related RPCs for privacy (3) onKeyboardInput RPC for users input of passwords. |
Is there a reason to do per RPC encryption? It is stated in your proposal that the web moving from HTTP to HTTPS is an example. Current standards are to encrypt everything on the web, even static pages that have no logins, etc. It seems that either: 1. requiring an app to use an encrypted service or 2. don't require the app to use an encrypted service would be much simpler than defining per-RPC requirements for individual apps. Additionally, as encryption requirements get stricter in the changing web / vehicle / connected everything landscape, I feel an all or nothing approach (production should always be all) is a better system than creating unneeded complexity that also leaves some aspects unencrypted. Think of this as a bit of future proofing. |
@BrettyWhite We have considered all or nothing solution, and we would like to see that all RPC messages are encrypted. The primary reasons we choose this are we want (1) the new apps or update versions of existing apps work with the old head units (that we cannot update and old sw does not require encryption) (2) the old apps (that does not have a security lib, thus cannot do encryption) works with the new head units For example, in the case of an application with different versions (old versions that does not need protection, new version that have new functions, it uses new RPCs and needs protection), if we have just one flag to say this app need protection or not, old versions will not work with new head units. |
It can also be an expensive operation to encrypt every last message. The cloud typically has the luxury of a higher performance server while a vehicle hu may be much more conservative. |
@yang1070 The encryption library might need to be able to detect the head unit then and force encryption on modern units and allow plaintext on old ones. I imagine this is an issue as well even with the selective approach. It seems the OEM would have to maintain the list and enforce that for individual car models. @mrapitis Aren't nav apps required to use an encrypted transport currently? I can't imagine templated apps creating anywhere near the traffic that something that streams video does. |
@BrettyWhite that’s up to each individual OEM. SDL core can be configured to not require encryption at all for any service. |
@BrettyWhite What encryption library does is to obtain a certificate from OEM's server and start DTLS/TLS handshake with head units with the downloaded cert, it cannot detect the head unit. OEM does not need to maintain the list for individual car models, it maintains the app list and the new RPCs that need encryption. Let's say all remote control RPCs need encryption, an update of the existing apps add a new feature of climate remote control to the app, on old head unit, the update version of the app will only lose the new feature but keep other features working with the selective approach. But with all/nothing approach, other feature will be affected too. |
@nickschwab @crokita Can you comment on the work that would be necessary to the policy server to add this level of functionality? It's not so much what the UI could look like, but how to actual code it into what we have today as well as being able to adjust the database to handle it. Each update to the policy server needs to be backwards compatible so creating scripts to migrate from one database scheme to the next is time consuming and can be error prone. I believe if the HU receives a StartService with encryption bit set and no encryption is set for that service would it not just remain unencrypted? I think adding the RPC by RPC approach is dynamic, but the complexity might not be worth it. Why not allow OEMs to slowly roll out encryption requirements for the RPC service and warn app developers they need to update or they will lose features? Or Core could recognize older RPC/protocol versions and enforce the encryption based on a threshold. |
yes. I think so. |
I agree this is an option. Basically, instead of each RPC having a Boolean flag, the whole app has a Boolean flag |
@joeygrover I see, but that still seems like technical debt and unnecessary complication within the library. As @BrettyWhite mentioned, I think the most important issue here is that there should be consistency across OEMs regarding what is encrypted (and what better way to do that than to encrypt everything?), otherwise app developers and consumers will be left vulnerable to having different portions of their apps insecure across different OEMs. As an app developer or consumer, I shouldn't have to worry about whether or not certain RPCs/parts of an app are encrypted or not depending on which vehicle I'm in... That would be like me walking into car dealerships and needing to ask if their vehicles have steering wheel airbags installed so I know whether or not my safety is at risk.
|
@nickschwab the list supplied by the module through the |
I think Nick's point in that last post is that while technical debt is bad and this definitely adds it - That our greater concern should be unity in the platform and concern for our partner apps and end users (the vehicle owners). They deserve current best practices and we owe it to them to provide it. |
@BrettyWhite Correct.
|
I think that goes to a larger question of should all data sent through SDL be encrypted or not, not just relating to RPCs. Some transports already provide encryption around everything sent, bluetooth, and MFi chip (USB and bluetooth) for example. So some of this would be encrypted traffic that goes through an encrypted transport as well. I would have to double check on AOA, but obviously TCP is not encrypted by default. |
Encrypted data message is transmitted over a transport. If Encryption add more data to the the message, or more messages to the traffic, then it get related. It may not be a problem on high band width transport, but it will be a problem for a low band with transport.
|
The Steering Committee voted to defer this proposal on 2019-01-15. A SDLC workshop will be scheduled to further discuss encrypting all RPCs or encrypting individual RPCs only. |
The Steering Committee took part in a workshop regarding this proposal on 2019-01-24. Meeting minutes from the workshop are attached with action items in red. 2019-01-24- SDL -0207- RPC Message Protection Workshop Minutes.pdf |
OverviewThree modifications to the Policy Table were evaluated with the goal of allowing an OEM to designate which RPCs are required to be encrypted/protected in order to be used by an app developer. This has been identified as a short-term solution, while the long-term solution is to support and require the encryption of all SDL communication at the transport level. Option 1: New
|
@lauratonwe, Please see Nick's comment above. |
Would be valuable to get @JackLivio's opinion of the proposed Policy Table addition from the perspective of Core as well. |
@nickschwab I agree that option 2 is the best approach. The main use case wants to force encryption for a specific group of RPCs (ie Remote Control). In option 3, adding the flag to each RPC entry would bloat the policy table. |
Revisions have been made by the author based on the outcome of the workshop that was held on 2019-01-24. The new review of "SDL 0207 - RPC message protection" begins now and runs through February 5, 2019. |
The author didn't use the requested naming from the project maintainer. It should be
This is a bit lacking for a description of changes required. App specific encryption requirementI would also like to see a flag added in the policy table per app that describes if they are required to comply with the encryption or not. This might be redundant due the ability to make near duplicate functional groups with the only change being the required encryption flag set to true, however I believe the flag will be useful in the future as we discussed the desire and plan to move to full transport encryption. So if the app entry had the flag set to true, Core would honor the requirements of the subsequent functional groups, but if the flag was absent or set to false, Core would ignore the requirement for that app. Race conditionI believe we have missed the possible race condition between an app using an RPC that requires an encrypted service vs when the app receives the |
Sure, we can change the name from "default": {
+ "encryption_required": false,
"keep_context": false,
"steal_focus": false,
"priority": "NONE",
"default_hmi": "NONE",
"groups": [
"Base-4"
]
} Regarding SDL server update, I'm not aware how sdl server is designed and what the implementation is, as long as it sends the PTU with added parameters, it is good to me. |
@joeygrover Good point. This is required to ensure backward compatibility with applications already in production so they can continue to work without encryption. I suggest this new flag to also be named
@yang1070 SDL Server updates include:
|
The Steering Committee voted to accept this proposal with revisions. The revisions will include adding a flag in the policy table per app that describes if they are required to comply with the encryption or not as noted in this comment. |
Requested Revisions have been made and issues have been entered: |
Hello SDL community,
The review of "SDL 0207 - RPC message protection" begins now and runs through February 5, 2019. The proposal is available here:
https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0207-rpc-message-protection.md
Reviews are an important part of the SDL evolution process. All reviews should be sent to the associated Github issue at:
#634
What goes into a review?
The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of SDL. When writing your review, here are some questions you might want to answer in your review:
Please state explicitly whether you believe that the proposal should be accepted into SDL.
More information about the SDL evolution process is available at
https://github.com/smartdevicelink/sdl_evolution/blob/master/process.md
Thank you,
Jordyn Mackool
Program Manager - Livio
[email protected]
The text was updated successfully, but these errors were encountered: