Implement TLS1.3 for windows client side #641
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #641 +/- ##
=======================================
Coverage 80.39% 80.39%
=======================================
Files 28 28
Lines 5992 5992
=======================================
Hits 4817 4817
Misses 1175 1175 ☔ View full report in Codecov by Sentry. |
| return grbitEnabledProtocols; | ||
| } | ||
|
|
||
| static struct aws_channel_handler *s_tls_handler_new_win10_plus( |
There was a problem hiding this comment.
I'd rather these were named based on what we're checking for semantically, ie the win32 feature and not just a windows version.
| memmove( | ||
| sc_handler->buffered_read_in_data_buf.buffer, | ||
| (sc_handler->buffered_read_in_data_buf.buffer + read_len) - input_buffers[3].cbBuffer, | ||
| input_buffers[3].cbBuffer); | ||
| sc_handler->buffered_read_in_data_buf.len = input_buffers[3].cbBuffer; |
There was a problem hiding this comment.
Prior to this PR, an offset was used to pass the extra data to the decrypt operation (see old line 1069). Now, this extra data is being moved to the beginning of the buffer, but the offset logic is still present. Looks incorrect.
Also, a followup question/concern. We added additional memory copying. Do you know how frequent is this case when input_buffers[3].BufferType == SECBUFFER_EXTRA? If it appears all the time, it might affect the performance.
| sc_handler->read_extra = input_buffers2[1].cbBuffer; | ||
| continue; | ||
| } | ||
| break; |
There was a problem hiding this comment.
I can't figure out why we don't want to try to decrypt extra data when sc_handler->buffered_read_in_data_buf.len == input_buffers2[1].cbBuffer. I think, a comment explaining this would be useful.
…s already occurred on assignment of status
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.