Skip to content

schannel: small simplifications#20569

Closed
bagder wants to merge 1 commit intomasterfrom
bagder/schannel-tidy
Closed

schannel: small simplifications#20569
bagder wants to merge 1 commit intomasterfrom
bagder/schannel-tidy

Conversation

@bagder
Copy link
Member

@bagder bagder commented Feb 12, 2026

Refactor and simplify the Schannel code, primarily by reducing
duplicated buffer-management and credential-setup logic.

  • split client certificate selection into get_client_cert() and SSPI
    credential acquisition into acquire_sspi_handle()
  • introduce a struct sbuffer for encrypted/decrypted buffering
  • Add ensure_encoding_size() and ensure_decoding_size() helpers to
    centralize buffer growth/realloc decisions
  • Tighten variable scopes and tidy indentation/logging in the handshake
    and receive/decrypt loops.
  • Update comments and adjusts some receive error-condition handling to
    better preserve buffered-data behavior.

@github-actions github-actions bot added TLS Windows Windows-specific labels Feb 12, 2026
@bagder
Copy link
Member Author

bagder commented Feb 12, 2026

augment review

@augmentcode
Copy link

augmentcode bot commented Feb 12, 2026

🤖 Augment PR Summary

Summary: This PR refactors and simplifies the Schannel TLS backend, primarily by reducing duplicated buffer-management and credential-setup logic.

Changes:

  • Splits client certificate selection into get_client_cert() and SSPI credential acquisition into acquire_sspi_handle().
  • Introduces a shared struct sbuffer for encrypted/decrypted buffering (length/offset/buffer) and updates handshake/recv code to use it.
  • Adds ensure_encoding_size() and ensure_decoding_size() helpers to centralize buffer growth/realloc decisions.
  • Tightens variable scopes and tidies indentation/logging in the handshake and receive/decrypt loops.
  • Updates comments and adjusts some receive error-condition handling for clearer state management.
Technical Notes: Centralizing buffer sizing reduces repeated arithmetic and makes Schannel record handling easier to reason about across handshake and data paths.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 3 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the Schannel TLS backend internals to simplify buffer management and tighten up local variable scoping, while also cleaning up indentation/comments and adjusting an error-handling path.

Changes:

  • Replace separate encdata_* / decdata_* fields with a shared struct sbuffer for encrypted/decrypted buffers.
  • Introduce ensure_encoding_size() / ensure_decoding_size() helpers and use them in handshake and receive paths.
  • Minor cleanups: request-flag expression simplification, indentation fixes, reduced variable scopes, and a small recv error-path adjustment.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
lib/vtls/schannel_int.h Introduces struct sbuffer and updates backend state to use it; moves the encdata_is_incomplete comment.
lib/vtls/schannel.c Refactors handshake/recv buffer logic to use struct sbuffer and new ensure-size helpers; scope/indent fixes and a recv error-path tweak.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bagder bagder requested a review from Copilot February 12, 2026 13:45
@bagder bagder marked this pull request as ready for review February 12, 2026 13:48
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bagder
Copy link
Member Author

bagder commented Feb 12, 2026

augment review

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

bagder added a commit that referenced this pull request Feb 12, 2026
Refactor and simplift the Schannel code, primarily by reducing
duplicated buffer-management and credential-setup logic.

- split client certificate selection into get_client_cert() and SSPI credential acquisition into
  acquire_sspi_handle()
- introduce a struct sbuffer for encrypted/decrypted buffering
- Add ensure_encoding_size() and ensure_decoding_size() helpers to
  centralize buffer growth/realloc decisions
- Tighten variable scopes and tidy indentation/logging in the handshake and receive/decrypt loops.
- Update comments and adjusts some receive error-condition handling to
  better preserve buffered-data behavior.

Closes #20569
@bagder bagder force-pushed the bagder/schannel-tidy branch from 814629a to 06c8642 Compare February 12, 2026 14:01
Refactor and simplift the Schannel code, primarily by reducing
duplicated buffer-management and credential-setup logic.

- split client certificate selection into get_client_cert() and SSPI
  credential acquisition into acquire_sspi_handle()
- introduce a struct sbuffer for encrypted/decrypted buffering
- Add ensure_encoding_size() and ensure_decoding_size() helpers to
  centralize buffer growth/realloc decisions
- Tighten variable scopes and tidy indentation/logging in the handshake
  and receive/decrypt loops.
- Update comments and adjusts some receive error-condition handling to
  better preserve buffered-data behavior.

Closes #20569
@bagder bagder force-pushed the bagder/schannel-tidy branch from 06c8642 to b0e91a3 Compare February 12, 2026 14:40
@bagder
Copy link
Member Author

bagder commented Feb 12, 2026

@aisle-analyzer give a look as well

@aisle-research-bot
Copy link

aisle-research-bot bot commented Feb 12, 2026

🔒 Aisle Security Analysis

✅ We scanned this PR and did not find any security vulnerabilities.
Aisle supplements but does not replace security review.


Analyzed PR: #20569 at commit b0e91a3

@bagder bagder closed this in 10bb489 Feb 12, 2026
@bagder bagder deleted the bagder/schannel-tidy branch February 12, 2026 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

TLS Windows Windows-specific

Development

Successfully merging this pull request may close these issues.

2 participants