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

Unable to enable TLS if using v2 protocol #2300

Merged
merged 3 commits into from
Mar 30, 2020

Conversation

sijie
Copy link
Member

@sijie sijie commented Mar 28, 2020

Descriptions of the changes in this PR:

Motivation

TLS is enabled using startTLS but the startTLS
is a v3 protobuf request. So if the client is
configured to use v2 protocol, the client is not
able to decode startTLSResponse.

Modifications

This change is to improve the v2 protocol response handling for
handling startTLS response.


In order to uphold a high standard for quality for code contributions, Apache BookKeeper runs various precommit
checks for pull requests. A pull request can only be merged when it passes precommit checks.


Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

If this PR is a BookKeeper Proposal (BP):

  • Make sure the PR title is formatted like:
    <BP-#>: Description of bookkeeper proposal
    e.g. BP-1: 64 bits ledger is support
  • Attach the master issue link in the description of this PR.
  • Attach the google doc link if the BP is written in Google Doc.

Otherwise:

  • Make sure the PR title is formatted like:
    <Issue #>: Description of pull request
    e.g. Issue 123: Description ...
  • Make sure tests pass via mvn clean apache-rat:check install spotbugs:check.
  • Replace <Issue #> in the title with the actual Issue number.

*Motivation*

TLS is enabled using `startTLS` but the `startTLS`
is a v3 protobuf request. So if the client is
configured to use v2 protocol, the client is not
able to decode `startTLSResponse`.

*Modifications*

This change is to improve the v2 protocol response handling for
handling `startTLS` response.
@sijie sijie added this to the 4.11.0 milestone Mar 28, 2020
@sijie sijie requested review from jiazhai and eolivelli March 28, 2020 09:08
@sijie sijie self-assigned this Mar 28, 2020
@sijie sijie added the type/bug label Mar 28, 2020
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM

@sijie sijie merged commit 87981d4 into apache:master Mar 30, 2020
sijie added a commit that referenced this pull request Mar 30, 2020
Descriptions of the changes in this PR:

*Motivation*

TLS is enabled using `startTLS` but the `startTLS`
is a v3 protobuf request. So if the client is
configured to use v2 protocol, the client is not
able to decode `startTLSResponse`.

*Modifications*

This change is to improve the v2 protocol response handling for
handling `startTLS` response.

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #2300 from sijie/fix_tls_v2_protocol

(cherry picked from commit 87981d4)
Signed-off-by: Sijie Guo <sijie@apache.org>
Ghatage pushed a commit to Ghatage/bookkeeper that referenced this pull request Jun 19, 2020
Descriptions of the changes in this PR:

*Motivation*

TLS is enabled using `startTLS` but the `startTLS`
is a v3 protobuf request. So if the client is
configured to use v2 protocol, the client is not
able to decode `startTLSResponse`.

*Modifications*

This change is to improve the v2 protocol response handling for
handling `startTLS` response.



Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes apache#2300 from sijie/fix_tls_v2_protocol
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants