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 0207 - RPC message protection #634

Closed
jordynmackool opened this issue Dec 5, 2018 · 64 comments
Closed

SDL 0207 - RPC message protection #634

jordynmackool opened this issue Dec 5, 2018 · 64 comments

Comments

@jordynmackool
Copy link
Contributor

jordynmackool commented Dec 5, 2018

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:

  • Is the problem being addressed significant enough to warrant a change to SDL?
  • Does this proposal fit well with the feel and direction of SDL?
  • If you have used competitors with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
    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
jordyn@livio.io

@joeljfischer
Copy link
Contributor

Question 1: RPC encryption activation timing

Due to the fact that it may take a long time (up to a minute) to get RPC service encryption enabled, and there is no overhead after that, we strongly recommend the proxy enable encryption after the app gets activated and there is at least one RPC that needs protection.

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

We create a new protected RPC service 12, which is similar to the existing service 10 and service 11. We require all RPC messages that need protection (include requests, responses and notifications) must be transmitted via this new SDL 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 RPCs

I don't see a consideration in the proposal for applications that don't use the protected RPCs as seen in OnPermissionChanged. How is a mobile proxy supposed to know that the app does or does not use a particular protected RPC and therefore enable encryption? Is it expected to add a new mobile developer-facing API to ask them if they want to use RPC encryption if needed? But even that is inadequate. For example, app A uses SendLocation. Knowing that it's a protected RPC on some systems they turn on useRPCEncryptionIfNeeded. Core tells the proxy that some remote control RPC is protected, but SendLocation is not; this would still result in the app being unnecessarily protected.

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.

@yang1070
Copy link
Contributor

Question 1: RPC encryption activation timing

Due to the fact that it may take a long time (up to a minute) to get RPC service encryption enabled, and there is no overhead after that, we strongly recommend the proxy enable encryption after the app gets activated and there is at least one RPC that needs protection.

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.

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.

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?

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.

Question 3: Apps that don't use the protected RPCs
@joeljfischer I not sure I fully understand your comments.

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.

@mrapitis
Copy link
Contributor

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?

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.

@joeygrover
Copy link
Member

SDL proxy may restart service 7 with encryption enabled in several situations.

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.

SDL server updates:

The SDL server needs to support the new parameter in the policy table.

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 Service

I'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.

Question 3: Apps that don't use the protected RPCs

To Joel's question, if an app never intended to use GetVehicleData, but a permission update came through that stated that RPC needed to be encrypted, according to the proposal the app would have to restart the RPC service to align, which creates a poor UX.

No Sample Encryption library Android

I 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.

@mrapitis
Copy link
Contributor

mrapitis commented Dec 11, 2018

So the app is supposed to disconnect from the HU, reconnect with a new StartService that has the encryption bit set?

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).

@yang1070
Copy link
Contributor

yang1070 commented Dec 11, 2018

SDL proxy may restart service 7 with encryption enabled in several situations.

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.

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 restart, but it is actually not a restart, just enable additional functions.

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.

The goal is for an OEM have the ability to control the encryption requirement per RPC per app or for certain new apps. There will be no change for any existing applications that does not need encryption. It is not that complex as it looks. For example, to require all remote control apps use encryption for remote control RPCs, OEM just need to create a new encryption require remote control group and assign this group the RC apps.

@joeygrover
Copy link
Member

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.

@mrapitis
Copy link
Contributor

mrapitis commented Dec 11, 2018

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.

@joeygrover
Copy link
Member

@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.

@joeygrover
Copy link
Member

@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.

@mrapitis
Copy link
Contributor

mrapitis commented Dec 11, 2018

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.

@yang1070
Copy link
Contributor

No Sample Encryption library Android
I 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.

I agree this is a very reasonable concern. Without Sample Encryption library, this feature cannot be tested at all. But I don't think it is a reason not accepting this proposal.

@mrapitis
Copy link
Contributor

No Sample Encryption library Android
I 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.

I agree this is a very reasonable concern. Without Sample Encryption library, this feature cannot be tested at all. But I don't think it is a reason not accepting this proposal.

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.

@joeygrover
Copy link
Member

@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.

@yang1070
Copy link
Contributor

@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.

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.

@BrettyWhite
Copy link
Contributor

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.

@yang1070
Copy link
Contributor

yang1070 commented Dec 11, 2018

@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.

@mrapitis
Copy link
Contributor

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.

@BrettyWhite
Copy link
Contributor

@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.

@mrapitis
Copy link
Contributor

@BrettyWhite that’s up to each individual OEM. SDL core can be configured to not require encryption at all for any service.

@yang1070
Copy link
Contributor

@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.

@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.

@joeygrover
Copy link
Member

@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.

@yang1070
Copy link
Contributor

yang1070 commented Dec 11, 2018

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?

yes. I think so.

@yang1070
Copy link
Contributor

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?

I agree this is an option. Basically, instead of each RPC having a Boolean flag, the whole app has a Boolean flag needProtection in app policy. And it is added as a optional parameter to OnPermissionsChange RPC. OEMs can choose to do nothing about the existing apps (no app policy change or add needProtection=false). For a new app, an OEM can choose to require either all RPCs of the app or none RPCs of the app to be encryption enabled. For example, for a new RC app, an OEM say needProtection=true for this new app. SDL will get the config from policy server, and send it to the app, then the proxy can start the process of enable encryption right after it receives OnPermissionsChange message. After that, all RPC messages shall be encrypted for this new app.

@nickschwab
Copy link
Contributor

@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 of RPCs that must be encrypted would come through an OnPermissionsChange so the idea is that OEMs can change these RPCs on the fly, the app woudl receive the list of RPCs to encrypt and one of two things would happen:

  1. App is already setup to do encryption with OEM unit. The library would take the new requirements and encrypt those as well as the system is dynamic to match the module's status it connects to.
  2. App is not setup to do encryption. The app would not be able to send those RPCs. This is where the granular control would be nice for OEMs so they could give app developers time to update and become ready.

@joeygrover
Copy link
Member

@nickschwab the list supplied by the module through the OnPermissionsChange is only a list of required RPCs from the module. An app developer always has the option to encrypt any and all RPCs for themselves. The case they would run into is if a head unit supports encryption or not and at this point encryption support is not a requirement from an SDL standpoint. Therefore those apps would simply not connect to the head units it can't trust giving full control to the app developer.

@BrettyWhite
Copy link
Contributor

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.

@nickschwab
Copy link
Contributor

@BrettyWhite Correct.

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.

@joeygrover
Copy link
Member

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.

@yang1070
Copy link
Contributor

@joeygrover

Encryption/decryption is unrelated to transport. Those operations are pre and post transport so I don't believe this point is valid.

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.

  1. Loading time is a performance issue.

  2. The hypothesis of multiple apps slow registering issue is the amount of messages and data burst in a short period of time. additional messages sending at the same period can make it worse.

@smartdevicelink smartdevicelink locked and limited conversation to collaborators Jan 16, 2019
@jordynmackool
Copy link
Contributor Author

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.

@jordynmackool jordynmackool changed the title SDL 0207 - RPC message protection [Deferred] SDL 0207 - RPC message protection Jan 16, 2019
@jordynmackool
Copy link
Contributor Author

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
Appendix B.pdf
Appendix A.pdf

@nickschwab
Copy link
Contributor

nickschwab commented Jan 25, 2019

Overview

Three 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 rpcs struct within the Policy Table

This new struct would reside in the root level of a Policy Table and would define all RPCs as well as whether or not they require encryption. While providing a very clear representation of which RPCs are encrypted, this new struct would increase the size of the Policy Table considerably and introduce a new layer of Policy Table interpretation by SDL Core.

For example:

...
"rpcs": {
    "GetVehicleData": {
      "encryption_required": true
    },
    "OnVehicleData": {
      "encryption_required": true
    },
    "SubscribeVehicleData": {
      "encryption_required": true
    },
    "UnsubscribeVehicleData": {
      "encryption_required": true
    },
    "Alert": {
      "encryption_required": false
    },
    "AddCommand": {
      "encryption_required": false
    },
    "AddSubMenu": {
      "encryption_required": false
    },
    "CreateInteractionChoiceSet": {
      "encryption_required": false
    },
    "DeleteCommand": {
      "encryption_required": false
    },
    "DeleteFile": {
      "encryption_required": false
    },
    "DeleteInteractionChoiceSet": {
      "encryption_required": false
    },
    ...
},
...

Option 2: New attribute within Functional Groups

A new attribute (encryption_required: boolean`) to be added within the existing functional group struct to designate the OEM encryption enforcement for all RPCs within the group. If an app has access to multiple functional groups containing the same RPC and at least one of the groups requires encryption, then the RPC shall require encryption.

For example:

...
"functional_groupings": {
  "Location-1": {
>>>>"encryption_required": true,<<<<
    "user_consent_prompt": "Location",
    "rpcs": {
      "GetVehicleData": {
        "hmi_levels": [
          "BACKGROUND",
          "FULL",
          "LIMITED"
        ],
        "parameters": [
          "gps",
          "speed"
        ]
      },
      "OnVehicleData": {
        "hmi_levels": [
          "BACKGROUND",
          "FULL",
          "LIMITED"
        ],
        "parameters": [
          "gps",
          "speed"
        ]
      },
      "SubscribeVehicleData": {
        "hmi_levels": [
          "BACKGROUND",
          "FULL",
          "LIMITED"
        ],
        "parameters": [
          "gps",
          "speed"
        ]
      },
      "UnsubscribeVehicleData": {
        "hmi_levels": [
          "BACKGROUND",
          "FULL",
          "LIMITED"
        ],
        "parameters": [
          "gps",
          "speed"
        ]
      }
    }
  },
  "Notifications": {
>>>>"encryption_required": false,<<<<
    "user_consent_prompt": "Notifications",
    "rpcs": {
      "Alert": {
        "hmi_levels": [
          "BACKGROUND",
          "FULL",
          "LIMITED"
        ]
      }
    }
  },
  ...
},
...

Option 3: New attribute within Functional Group RPCs

Similar to Option 2, but the new attribute (encryption_required: boolean) to be added and configurable within each rpcs object of existing functional groups to designate the OEM encryption enforcement for the specific RPC.

For example:

...
"functional_groupings": {
  "Location-1": {
    "user_consent_prompt": "Location",
    "rpcs": {
      "GetVehicleData": {
>>>>>>>>"encryption_required": true,<<<<<<<<
        "hmi_levels": [
          "BACKGROUND",
          "FULL",
          "LIMITED"
        ],
        "parameters": [
          "gps",
          "speed"
        ]
      },
      "OnVehicleData": {
>>>>>>>>"encryption_required": true,<<<<<<<<
        "hmi_levels": [
          "BACKGROUND",
          "FULL",
          "LIMITED"
        ],
        "parameters": [
          "gps",
          "speed"
        ]
      },
      "SubscribeVehicleData": {
>>>>>>>>"encryption_required": true,<<<<<<<<
        "hmi_levels": [
          "BACKGROUND",
          "FULL",
          "LIMITED"
        ],
        "parameters": [
          "gps",
          "speed"
        ]
      },
      "UnsubscribeVehicleData": {
>>>>>>>>"encryption_required": true,<<<<<<<<
        "hmi_levels": [
          "BACKGROUND",
          "FULL",
          "LIMITED"
        ],
        "parameters": [
          "gps",
          "speed"
        ]
      }
    }
  },
  "Notifications": {
    "user_consent_prompt": "Notifications",
    "rpcs": {
      "Alert": {
>>>>>>>>"encryption_required": false,<<<<<<<<
        "hmi_levels": [
          "BACKGROUND",
          "FULL",
          "LIMITED"
        ]
      }
    }
  },
  ...
},
...

Conclusion

I recommend Option 2 (new attribute within Functional Groups) since it achieves the SDLC's short-term goals while:

  • Minimizing the size increase of the policy table
  • Minimizing complexity of Policy Table generation
  • Minimizing complexity of the Policy Server interface

@jordynmackool
Copy link
Contributor Author

@lauratonwe, Please see Nick's comment above.

@nickschwab
Copy link
Contributor

Would be valuable to get @JackLivio's opinion of the proposed Policy Table addition from the perspective of Core as well.

@Jack-Byrne
Copy link
Contributor

Jack-Byrne commented Jan 28, 2019

@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.

@jordynmackool jordynmackool changed the title [Deferred] SDL 0207 - RPC message protection SDL 0207 - RPC message protection Jan 30, 2019
@smartdevicelink smartdevicelink unlocked this conversation Jan 30, 2019
@jordynmackool
Copy link
Contributor Author

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.

@joeygrover
Copy link
Member

need_protection

The author didn't use the requested naming from the project maintainer. It should be encryption_required. At the very least protection_required.

SDL server updates:

The SDL server may support the new parameter in the policy table.

This is a bit lacking for a description of changes required.

App specific encryption requirement

I 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 condition

I 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 OnPermissionChange update that conveys that information. While it can be said, the app must simply try again, the logic to handle this in the managers might be a bit of work to handle it correctly.

@yang1070
Copy link
Contributor

yang1070 commented Feb 5, 2019

Sure, we can change the name from need_protection to encryption_required in function group level. And add a app level flag

    "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.

@nickschwab
Copy link
Contributor

nickschwab commented Feb 5, 2019

App specific encryption requirement

I 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.

@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 encryption_required and to default to false. This additional flag in the app_policies portion of the Policy Table should be set by the Policy Server upon the OEM's application approval, based upon a new startup configuration variable in the Policy Server named app_encryption_required to allow for a point-in-time change by the OEM. This boolean could be manually overridden by the OEM via a toggle in the Policy Server app review UI should they need to make any one-off exceptions.

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.

@yang1070 SDL Server updates include:

  • Database and API modifications to support the new encryption_required attribute of Functional Groups.
  • UI addition of a checkbox or toggle-switch to enable/disable the state of encryption_required for a Functional Group.
  • Database and API modifications to support the new encryption_required app_policy attribute.
  • New app_encryption_required server environment variable to indicate whether auto-approved applications should automatically have their encryption_required app_policy value set to true or false (default false)
  • UI addition of a checkbox or toggle-switch to enable/disable an individual application version's encryption_required app_policy value.
  • API modifications to build Policy Tables with the new encryption_required attributes in Functional Groups and App Policy structs.

@smartdevicelink smartdevicelink locked and limited conversation to collaborators Feb 6, 2019
@jordynmackool
Copy link
Contributor Author

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.
There was additional discussion in regards to whether or not this encryption will be used in core from the policy table, essentially authentication and validation or if this flag needs to be passed from core into the proxies. It was determined that it is almost inferred by the proxies if they get an on-permission change with the new RPCs that are delegating they need to be encrypted and the app itself would need encryption, so core should send the on-permission changes to an app in regards to the RPC encryptions unless that app requires an encryption.

@jordynmackool
Copy link
Contributor Author

jordynmackool commented Feb 19, 2019

Requested Revisions have been made and issues have been entered:
core
rpc_spec
ios
android
policy server

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

No branches or pull requests

8 participants