Skip to content

bpo-30622: Change NPN detection: #2079

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

Merged
merged 2 commits into from
Sep 4, 2017
Merged

bpo-30622: Change NPN detection: #2079

merged 2 commits into from
Sep 4, 2017

Conversation

melvyn-sopacua
Copy link
Contributor

Bpo follows.

Changes NPN detection to match semantics of OpenSSL 1.1.

Version breakdown, support disabled (pre-patch/post-patch):
- pre-1.0.1: OPENSSL_NPN_NEGOTIATED will not be defined -> False/False
- 1.0.1 and 1.0.2: OPENSSL_NPN_NEGOTIATED will not be defined ->
False/False
- 1.1.0+: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will be defined -> True/False

Version breakdown support enabled (pre-patch/post-patch):
- pre-1.0.1: OPENSSL_NPN_NEGOTIATED will not be defined -> False/False
- 1.0.1 and 1.0.2: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will not be defined -> True/True
- 1.1.0+: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will not be defined -> True/True
@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

@melvyn-sopacua melvyn-sopacua changed the title Change NPN detection: bpo-30622: Change NPN detection: Jun 10, 2017
@melvyn-sopacua
Copy link
Contributor Author

Needs some more refinement. do_protocol_selection is used when ALPN is enabled. When NPN is disabled and ALPN is enabled, this function doesn't exist.

- If NPN is disabled, but ALPN is available we need our callback
- Make clinic's ssl behave the same way

This created a working ssl module for me, with NPN disabled and ALPN
enabled for OpenSSL 1.1.0f.

Concerns to address:
The initial commit for NPN support into OpenSSL [1], had the
OPENSSL_NPN_* variables defined inside the OPENSSL_NO_NEXTPROTONEG
guard. The question is if that ever made it into a release.
This would need an ugly hack, something like:

	#if defined(OPENSSL_NO_NEXTPROTONEG) && \
		!defined(OPENSSL_NPN_NEGOTIATED)
	#	define OPENSSL_NPN_UNSUPPORTED 0
	#	define OPENSSL_NPN_NEGOTIATED 1
	#	define OPENSSL_NPN_NO_OVERLAP 2
	#endif

[1] openssl/openssl@68b33cc5c7
Copy link
Member

@tiran tiran left a comment

Choose a reason for hiding this comment

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

LGTM

@tiran tiran merged commit b2d096b into python:master Sep 4, 2017
@bedevere-bot
Copy link

GH-3314 is a backport of this pull request to the 3.6 branch.

tiran pushed a commit to tiran/cpython that referenced this pull request Sep 4, 2017
* Change NPN detection:

Version breakdown, support disabled (pre-patch/post-patch):
- pre-1.0.1: OPENSSL_NPN_NEGOTIATED will not be defined -> False/False
- 1.0.1 and 1.0.2: OPENSSL_NPN_NEGOTIATED will not be defined ->
False/False
- 1.1.0+: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will be defined -> True/False

Version breakdown support enabled (pre-patch/post-patch):
- pre-1.0.1: OPENSSL_NPN_NEGOTIATED will not be defined -> False/False
- 1.0.1 and 1.0.2: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will not be defined -> True/True
- 1.1.0+: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will not be defined -> True/True

* Refine NPN guard:

- If NPN is disabled, but ALPN is available we need our callback
- Make clinic's ssl behave the same way

This created a working ssl module for me, with NPN disabled and ALPN
enabled for OpenSSL 1.1.0f.

Concerns to address:
The initial commit for NPN support into OpenSSL [1], had the
OPENSSL_NPN_* variables defined inside the OPENSSL_NO_NEXTPROTONEG
guard. The question is if that ever made it into a release.
This would need an ugly hack, something like:

	GH-if defined(OPENSSL_NO_NEXTPROTONEG) && \
		!defined(OPENSSL_NPN_NEGOTIATED)
	GH-	define OPENSSL_NPN_UNSUPPORTED 0
	GH-	define OPENSSL_NPN_NEGOTIATED 1
	GH-	define OPENSSL_NPN_NO_OVERLAP 2
	GH-endif

[1] openssl/openssl@68b33cc5c7.
(cherry picked from commit b2d096b)
@bedevere-bot
Copy link

GH-3316 is a backport of this pull request to the 2.7 branch.

tiran pushed a commit to tiran/cpython that referenced this pull request Sep 4, 2017
* Change NPN detection:

Version breakdown, support disabled (pre-patch/post-patch):
- pre-1.0.1: OPENSSL_NPN_NEGOTIATED will not be defined -> False/False
- 1.0.1 and 1.0.2: OPENSSL_NPN_NEGOTIATED will not be defined ->
False/False
- 1.1.0+: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will be defined -> True/False

Version breakdown support enabled (pre-patch/post-patch):
- pre-1.0.1: OPENSSL_NPN_NEGOTIATED will not be defined -> False/False
- 1.0.1 and 1.0.2: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will not be defined -> True/True
- 1.1.0+: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will not be defined -> True/True

* Refine NPN guard:

- If NPN is disabled, but ALPN is available we need our callback
- Make clinic's ssl behave the same way

This created a working ssl module for me, with NPN disabled and ALPN
enabled for OpenSSL 1.1.0f.

Concerns to address:
The initial commit for NPN support into OpenSSL [1], had the
OPENSSL_NPN_* variables defined inside the OPENSSL_NO_NEXTPROTONEG
guard. The question is if that ever made it into a release.
This would need an ugly hack, something like:

	GH-if defined(OPENSSL_NO_NEXTPROTONEG) && \
		!defined(OPENSSL_NPN_NEGOTIATED)
	GH-	define OPENSSL_NPN_UNSUPPORTED 0
	GH-	define OPENSSL_NPN_NEGOTIATED 1
	GH-	define OPENSSL_NPN_NO_OVERLAP 2
	GH-endif

[1] openssl/openssl@68b33cc5c7
(cherry picked from commit b2d096b)
zware added a commit to zware/cpython that referenced this pull request Sep 4, 2017
zware added a commit that referenced this pull request Sep 4, 2017
tiran added a commit that referenced this pull request Sep 4, 2017
* Change NPN detection:

Version breakdown, support disabled (pre-patch/post-patch):
- pre-1.0.1: OPENSSL_NPN_NEGOTIATED will not be defined -> False/False
- 1.0.1 and 1.0.2: OPENSSL_NPN_NEGOTIATED will not be defined ->
False/False
- 1.1.0+: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will be defined -> True/False

Version breakdown support enabled (pre-patch/post-patch):
- pre-1.0.1: OPENSSL_NPN_NEGOTIATED will not be defined -> False/False
- 1.0.1 and 1.0.2: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will not be defined -> True/True
- 1.1.0+: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will not be defined -> True/True

* Refine NPN guard:

- If NPN is disabled, but ALPN is available we need our callback
- Make clinic's ssl behave the same way

This created a working ssl module for me, with NPN disabled and ALPN
enabled for OpenSSL 1.1.0f.

Concerns to address:
The initial commit for NPN support into OpenSSL [1], had the
OPENSSL_NPN_* variables defined inside the OPENSSL_NO_NEXTPROTONEG
guard. The question is if that ever made it into a release.
This would need an ugly hack, something like:

	GH-if defined(OPENSSL_NO_NEXTPROTONEG) && \
		!defined(OPENSSL_NPN_NEGOTIATED)
	GH-	define OPENSSL_NPN_UNSUPPORTED 0
	GH-	define OPENSSL_NPN_NEGOTIATED 1
	GH-	define OPENSSL_NPN_NO_OVERLAP 2
	GH-endif

[1] openssl/openssl@68b33cc5c7.
(cherry picked from commit b2d096b)
tiran added a commit that referenced this pull request Sep 5, 2017
* Change NPN detection:

Version breakdown, support disabled (pre-patch/post-patch):
- pre-1.0.1: OPENSSL_NPN_NEGOTIATED will not be defined -> False/False
- 1.0.1 and 1.0.2: OPENSSL_NPN_NEGOTIATED will not be defined ->
False/False
- 1.1.0+: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will be defined -> True/False

Version breakdown support enabled (pre-patch/post-patch):
- pre-1.0.1: OPENSSL_NPN_NEGOTIATED will not be defined -> False/False
- 1.0.1 and 1.0.2: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will not be defined -> True/True
- 1.1.0+: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will not be defined -> True/True

* Refine NPN guard:

- If NPN is disabled, but ALPN is available we need our callback
- Make clinic's ssl behave the same way

This created a working ssl module for me, with NPN disabled and ALPN
enabled for OpenSSL 1.1.0f.

Concerns to address:
The initial commit for NPN support into OpenSSL [1], had the
OPENSSL_NPN_* variables defined inside the OPENSSL_NO_NEXTPROTONEG
guard. The question is if that ever made it into a release.
This would need an ugly hack, something like:

	GH-if defined(OPENSSL_NO_NEXTPROTONEG) && \
		!defined(OPENSSL_NPN_NEGOTIATED)
	GH-	define OPENSSL_NPN_UNSUPPORTED 0
	GH-	define OPENSSL_NPN_NEGOTIATED 1
	GH-	define OPENSSL_NPN_NO_OVERLAP 2
	GH-endif

[1] openssl/openssl@68b33cc5c7
(cherry picked from commit b2d096b)
GadgetSteve pushed a commit to GadgetSteve/cpython that referenced this pull request Sep 10, 2017
* Change NPN detection:

Version breakdown, support disabled (pre-patch/post-patch):
- pre-1.0.1: OPENSSL_NPN_NEGOTIATED will not be defined -> False/False
- 1.0.1 and 1.0.2: OPENSSL_NPN_NEGOTIATED will not be defined ->
False/False
- 1.1.0+: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will be defined -> True/False

Version breakdown support enabled (pre-patch/post-patch):
- pre-1.0.1: OPENSSL_NPN_NEGOTIATED will not be defined -> False/False
- 1.0.1 and 1.0.2: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will not be defined -> True/True
- 1.1.0+: OPENSSL_NPN_NEGOTIATED will be defined and
OPENSSL_NO_NEXTPROTONEG will not be defined -> True/True

* Refine NPN guard:

- If NPN is disabled, but ALPN is available we need our callback
- Make clinic's ssl behave the same way

This created a working ssl module for me, with NPN disabled and ALPN
enabled for OpenSSL 1.1.0f.

Concerns to address:
The initial commit for NPN support into OpenSSL [1], had the
OPENSSL_NPN_* variables defined inside the OPENSSL_NO_NEXTPROTONEG
guard. The question is if that ever made it into a release.
This would need an ugly hack, something like:

	#if defined(OPENSSL_NO_NEXTPROTONEG) && \
		!defined(OPENSSL_NPN_NEGOTIATED)
	#	define OPENSSL_NPN_UNSUPPORTED 0
	#	define OPENSSL_NPN_NEGOTIATED 1
	#	define OPENSSL_NPN_NO_OVERLAP 2
	#endif

[1] openssl/openssl@68b33cc5c7
GadgetSteve pushed a commit to GadgetSteve/cpython that referenced this pull request Sep 10, 2017
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.

6 participants