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

Support TLS max size when encrypted #1955

Merged

Conversation

joeljfischer
Copy link
Contributor

@joeljfischer joeljfischer commented Mar 31, 2021

Fixes #1954

Risk

This PR makes no API changes.

Testing Plan

  • I have verified that I have not introduced new warnings in this PR (or explain why below)
  • I have run the unit tests with this PR
  • I have tested this PR against Core and verified behavior (if applicable, if not applicable, explain why below).

Unit Tests

Tests were added for sending encryption RPCs when encryption is ready and not ready.

Core Tests

  • Tested normal non-encrypted use-case
  • Tested encryption use-case using an encryption test app to send a one-frame PutFile over encryption.
  • Tested encryption use-case using an encryption test app to send a 15-frame PutFile over encryption.
  • Tested encryption use-case using an encryption test app to receive a 3-frame OnSystemRequest over encryption

Core version / branch / commit hash / module tested against: Core v7.1.0
HMI name / version / branch / commit hash / module tested against: Generic HMI v0.10.0, sdl_hmi v5.5.0

Summary

Updates encryption to only send a max of 16384 bytes per packet due to TLS limitations when connected over encryption.

Changelog

Bug Fixes
  • Fix encrypted multi-frame messages being too large and failing to send.

Tasks Remaining:

  • Update unit tests
  • Smoke Test

CLA

@joeljfischer joeljfischer added bug A defect in the library protocol Relating to the protocol layer labels Mar 31, 2021
@joeljfischer joeljfischer self-assigned this Mar 31, 2021
@codecov
Copy link

codecov bot commented Mar 31, 2021

Codecov Report

Merging #1955 (5e0d3c1) into hotfix/7.1.1 (1fa7c01) will decrease coverage by 69.48%.
The diff coverage is 64.78%.

❗ Current head 5e0d3c1 differs from pull request most recent head 6f55654. Consider uploading reports for the commit 6f55654 to get more accurate results

@@                Coverage Diff                @@
##           hotfix/7.1.1    #1955       +/-   ##
=================================================
- Coverage         83.69%   14.21%   -69.49%     
=================================================
  Files               441      441               
  Lines             22564    22577       +13     
=================================================
- Hits              18885     3209    -15676     
- Misses             3679    19368    +15689     

…ti-frame

# Conflicts:
#	SmartDeviceLink/private/SDLProtocol.m
#	SmartDeviceLinkTests/ProtocolSpecs/MessageSpecs/SDLProtocolSpec.m
@joeljfischer joeljfischer changed the base branch from develop to hotfix/7.1.1 April 21, 2021 18:06
Copy link
Contributor

@NicoleYarroch NicoleYarroch left a comment

Choose a reason for hiding this comment

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

Some small changes requested

SmartDeviceLink/private/SDLProtocol.m Outdated Show resolved Hide resolved
SmartDeviceLink/private/SDLProtocol.m Show resolved Hide resolved
@joeljfischer joeljfischer merged commit 0b61d84 into hotfix/7.1.1 Apr 28, 2021
@joeljfischer joeljfischer deleted the bugfix/issue-1954-rpc-encryption-multi-frame branch April 28, 2021 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A defect in the library protocol Relating to the protocol layer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants