-
Notifications
You must be signed in to change notification settings - Fork 13
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 0317] SDL Protocol Security Specification #41
[SDL 0317] SDL Protocol Security Specification #41
Conversation
@theresalech this PR is ready for Livio review |
@AKalinich-Luxoft , can you please advise if this PR has already been reviewed and approved by a member of the Ford team? We will begin our review after a Ford team member has provided their approval on the PR. Thank you! |
@theresalech Ford has reviewed and approved of this PR. Thanks! |
Co-authored-by: Jacob Keeler <[email protected]>
Co-authored-by: Jacob Keeler <[email protected]>
Co-authored-by: Jacob Keeler <[email protected]>
@jacobkeeler all comments have been processed. Would you have a chance to proceed with the review here? |
@AKalinich-Luxoft Ah, so we've been holding off on officially approving this because there is some testing we need to do to verify there aren't any legacy issues with this specification. Currently this spec isn't properly implemented in either of the mobile libraries (see smartdevicelink/sdl_ios#2024 and smartdevicelink/sdl_java_suite#1720), and we're in the process of fixing this. As a result, we are currently planning to test these updates against a number of legacy systems to verify that everything still works properly (and if not, we would submit a revision to the proposal which accounts for this). I can approve this PR for now if we need to, but we will need to hold off on merging until this testing is performed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added suggestions for rewriting the Send Handshake Data
description for clarity, in addition to a note describing incorrect behavior in older versions of Core.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed some small formatting issues
Co-authored-by: Jacob Keeler <[email protected]>
Co-authored-by: Jacob Keeler <[email protected]>
Implements #40