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

RPC Message Protection Implementation #1187

Merged
merged 50 commits into from
Oct 10, 2019
Merged

RPC Message Protection Implementation #1187

merged 50 commits into from
Oct 10, 2019

Conversation

bilal-alsharifi
Copy link
Contributor

@bilal-alsharifi bilal-alsharifi commented Oct 2, 2019

Fixes #978 (based on PR #1103)

This PR is ready for review.

Risk

This PR makes minor API changes.

Testing Plan

Unit tests are added. Manual tests can be done by requiring some RPC to be encrypted in policy table.

Summary

This PR adds RPC encryption to Android and JavaSE libraries as described in the proposal.

CLA

Tnnnguyen added 30 commits June 26, 2019 09:08
- Add getter/setter for 'requireEncryption' field in PermissionItem and OnPermissionChange
… for app level and individual rpc accordingly.
- Reset mRPCSecuredServiceStarted on service error, on service ended and during proxy dispose
- Improve log info
Change method signature to getter and setter match
- Add comments, more unit tests and rename getter/setter method
- Add BaseEncryptionLifecycleManager
* develop_mainRepo: (262 commits)
  Fix error on FunctionID
  Update predefined window to be a enum Update the validations for the ONHMIStatus Update tests
  Change to int enum
  Validation of the manager on the onHMIStatus
  change to set it as Object as suggested
  Update checkLifecycleConfiguration logic
  fix failing tests
  removed more references to 'default'
  fix improper tab
  dont require Menu config params and remove defaults
  add some javadocs for migrating to newer SDL Versions
  fix tabulation and position
  change back to previous way
  revert last commit
  Add javadoc for scale, ppi and diagonalscreensize
  Dont allow user to modify scale value
  Call sendRPC method
  Fix documentation Fix indentation for
  Convert odd resolution to pair resolution.
  Deprecate old proxy classes
  ...
Move encryption related public methods back to encryption manager
Refractor to proper directory to match current structure of Managers
Create hook to lifecycle manager
Add method to prepare message payload
* develop_mainRepo_local: (22 commits)
  Fixed unit test.
  Update VehicleDataTypeTests.java
  Update VehicleDataTypeTests.java
  Update VehicleDataType.java
  Update VehicleDataType.java
  Simplify if conditions LockScreenManager
  Address review comments
  fix review comments
  some of the fixes requested
  clarify javadocs
  merge variable declarations
  lock screen changes to match iOS
  Additional update for adding VEHICLEDATA_OEM_VEHICLE_DATA_TYPE in VehicleDataType as well as making oemCustomDataType as Non-Mandatory. Also, updating test cases.
  Fix some spacing issues
  Updating test cases for Generic Network Signal data Proposal changes.
  Fix #1102: add StreamingStateMachine transition and SdlRemoteDisplay condition
  Additional changes to method names as per revised proposal.
  Add ability to use TCP transport with new layers
  Make TCP write thread actually not on main thread
  Create TCPTransportManager
  ...
- Notify developer on encryption service status change
- Check app level encryption in the other loop, in SdlProxyBase
* develop_mainRepo_local:
  fix bracket
  Fix per code review recommendations: remove unused imports and attempt to fix indentation. Fix function id to match mobile API spec.
  Removed unused import.
  Revert "Attempt a work-around to fix failed test suite on server."
  Attempt a work-around to fix failed test suite on server.
  Resolve more conflict.
  Merge new changes from develop branch
  Merge new changes from develop branch
  Add more unit tests
  Add unit tests for ReleaseInteriorVehicleDataModule and ReleaseInteriorVehicleDataModuleResponse
  Add unit tests for ReleaseInteriorVehicleDataModule and ReleaseInteriorVehicleDataModuleResponse
  Add test for GetInteriorVehicleDataConsentResponse
  Add unit tests for Grid, SeatLocation, GetInteriorVehicleDataConsent Fix comment in SystemCapabilityType
  Add unit tests for Grid, SeatLocation, GetInteriorVehicleDataConsent Fix comment in SystemCapabilityType
  Add unit tests for SeatLocationCapability
  Add tests for ModuleInfo
  Fix test cases, add more enum to enum tests and refractor functionId names.
  Add overridden methods onGetInteriorVehicleDataConsentResponse and onReleaseInteriorVehicleDataModuleResponse to ProxyBridge
  - handle new response msg for GetInteriorVehicleDataConsentResponse and ReleaseInteriorVehicleDataModuleResponse in SdlProxyBase - change hash keys in Grid and ModuleInfo - refractor method names and constants
  Multiple Module, 1st batch
…curity with encryption listener

- In SdlProxyBase: also listen to HMI level when starting secured service, make call back to app on all statuses of service listener
- Match method name changes for PermissionItem in JavaSE
- In BasePermissionManager, also listen for HMI Level
- Provide public method to start secured service in SdlManager
- Provide public methods to check encryption requirement and requirement for individual RPC in both SdlManager and SdlProxyBase
…no secured service, notify the caller using the OnRPCResponseListener instead of propagating to SdlManager.
…service is not secured, also clear resources on dispose.
- Change logic to decide if encryption is needed base on new requirement
- Decide on encryption when hmi level is not NONE, not only when FULL
- Improve ISdlServiceListener logs
- Improve readability when setting payload protection for message
Tnnnguyen and others added 12 commits September 13, 2019 11:27
* develop_mainRepo_local: (60 commits)
  fix is paused logic
  Make ChoiceSetManager operations blocking (#1155)
  handle case where TM is null in protocolbase
  add a null check to proxylistener
  fix thread lock issue
  notify iProxyListenerBase of service ending
  remove deprecation and add javadoc
  fix incorrect method logic
  add one more check
  allow videos to be stopped properly with secondary transport disconnection
  fix older video stream restarts
  Removed unused code
  Refactored Thread.currentThread().isInterrupted()
  Removed uneeded deprecated presentKeyboard()
  dismissKeyboard() now takes a nonnull Integer
  Attempting to fix spacing issue in tests
  Update base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java
  Update base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java
  Update base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java
  Update base/src/main/java/com/smartdevicelink/proxy/rpc/ScrollableMessage.java
  ...
- Remove local rpc names set and use instance variable
- startRPCEncryptionService method should be void type
- Remove redundant empty rpc names set check in SdlProxyBase
@bilal-alsharifi bilal-alsharifi self-assigned this Oct 2, 2019
@codecov-io
Copy link

codecov-io commented Oct 2, 2019

Codecov Report

Merging #1187 into develop will decrease coverage by 0.68%.
The diff coverage is 25%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #1187      +/-   ##
=============================================
- Coverage      47.36%   46.68%   -0.69%     
- Complexity      4323     4382      +59     
=============================================
  Files            481      481              
  Lines          23338    24090     +752     
  Branches        2648     2900     +252     
=============================================
+ Hits           11055    11247     +192     
- Misses         11604    12162     +558     
- Partials         679      681       +2
Impacted Files Coverage Δ Complexity Δ
...com/smartdevicelink/SdlConnection/SdlSession2.java 19.32% <0%> (-0.34%) 9 <0> (ø)
...smartdevicelink/proxy/rpc/OnPermissionsChange.java 100% <100%> (ø) 7 <2> (+2) ⬆️
...va/com/smartdevicelink/proxy/rpc/enums/Result.java 100% <100%> (ø) 2 <0> (ø) ⬇️
.../com/smartdevicelink/proxy/rpc/PermissionItem.java 90% <100%> (+1.76%) 10 <2> (+2) ⬆️
...n/java/com/smartdevicelink/proxy/SdlProxyBase.java 8.56% <17.91%> (+0.36%) 29 <0> (+2) ⬆️
...ink/managers/permission/BasePermissionManager.java 70.19% <30%> (-2.86%) 43 <1> (ø)
...icelink/proxy/rpc/GetSystemCapabilityResponse.java 80.95% <0%> (-19.05%) 8% <0%> (+3%)
...link/proxy/rpc/SetInteriorVehicleDataResponse.java 80.95% <0%> (-19.05%) 8% <0%> (+3%)
...evicelink/proxy/rpc/DiagnosticMessageResponse.java 80.95% <0%> (-19.05%) 8% <0%> (+3%)
...link/proxy/rpc/GetInteriorVehicleDataResponse.java 83.33% <0%> (-16.67%) 11% <0%> (+4%)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e62d723...e8a335a. Read the comment docs.

@bilal-alsharifi bilal-alsharifi changed the title [WIP] RPC Message Protection Implementation RPC Message Protection Implementation Oct 4, 2019
@@ -2447,6 +2447,10 @@ public static boolean validatePermissionItem(PermissionItem item1, PermissionIte
return false;
}

if(item1.getRequireEncryption() != item2.getRequireEncryption()) {
return false;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent

*
* @return true if encryption is required for app level; false otherwise
*/
boolean getRequiresEncryption() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could this be private?

/**
* Checks the current state and make the call back to initiate secured service flow
*/
void checkStatusAndInitSecuredService() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could this be private?

@BrettyWhite BrettyWhite merged commit cc4e472 into develop Oct 10, 2019
@BrettyWhite BrettyWhite deleted the feature/issue_978 branch October 10, 2019 18:53
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

Successfully merging this pull request may close these issues.

4 participants