Skip to content

fix: restart reader after server declines StartTLS - #645

Open
netliomax25-code wants to merge 1 commit into
go-ldap:masterfrom
netliomax25-code:starttls-declined-reader
Open

netliomax25-code wants to merge 1 commit into
go-ldap:masterfrom
netliomax25-code:starttls-declined-reader

Conversation

@netliomax25-code

Copy link
Copy Markdown
Contributor
  1. StartTLS sends its request with the startTLS flag, so the reader goroutine stops as soon as it has delivered the response and the TLS handshake can take over the socket. The reader is only restarted on the success path, after the handshake.
  2. When the server declines StartTLS (unwillingToPerform, protocolError, unavailable, or an error an on-path attacker injects to keep the session in the clear), the error is returned but nothing reads the socket any more: the next Bind, Search or WhoAmI on that connection is written and its reply is never delivered, so the caller blocks until SetTimeout fires, or forever without one.
  3. Restart the reader in the declined branch. RFC 4511 section 4.14.2.2 leaves the session usable without a TLS layer in that case, which is what the code already intends by not closing the connection there; the handshake-failure branch, which closes the connection, is unchanged.

Validation: TestStartTLSRejectedKeepsReader drives StartTLS over the in-package fake connection, has the server decline it and checks a following WhoAmI gets its reply. It fails on master and passes with the fix. go vet, golangci-lint and go test -race ./... against the test directory server pass.

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.

1 participant