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

ALPN implementation for the updated SSLServiceDelegate #26

Merged
merged 22 commits into from
Jun 9, 2017

Conversation

ymesika
Copy link
Contributor

@ymesika ymesika commented May 24, 2017

Application-Layer Protocol Negotiation (ALPN) is a TLS extension allows the application-layer to negotiate which protocol will by used over the secured socket.
The main motivation for adding this support to BlueSSLService is to add support for HTTP/2 which uses this extension to choose the newer version of HTTP over the older ones.

Description

This PR follows the BlueSocket PR #67 that added new field/function to the SSLServiceDelegate protocol.

In this PR the implementation uses the OpenSSL API to register callback functions that will negotiate for a protocol based on ALPN information arriving from the client within the ClientHello message.
The implementation tries to match a supported protocol from the availableAlpnProtocols list with the protocols requested by the client. If it will find a match the response will be sent back to the client and the selected protocol will be set in the negotiatedAlpnProtocol field.

Because of limitations of the Apple Secure Transport it is still unclear how and if ALPN negotiation is possible on MacOS.
In this PR we only support the Linux environment by integrating with the OpenSSL API.

Motivation and Context

The main motivation for adding this support to BlueSSLService is to add support for HTTP/2 which uses this extension to choose the newer version of HTTP over the older ones.

How Has This Been Tested?

It has been tested with broader changes to Kitura-net and a new package under development for adding HTTP/2 support to Kitura.

Checklist:

  • I have submitted a CLA form
  • If applicable, I have updated the documentation accordingly.
  • If applicable, I have added tests to cover my changes.

…otiate the ALPN protocol based and select one
@ymesika
Copy link
Contributor Author

ymesika commented May 24, 2017

Closing the PR until I find a way to check the OpenSSL version and make sure it's >= 1.0.2.
Currently it fails the testings.

@ymesika ymesika closed this May 24, 2017
@ymesika
Copy link
Contributor Author

ymesika commented May 24, 2017

@shmuelk FYI

@billabt
Copy link
Collaborator

billabt commented May 24, 2017

Please note also that the macOS build failed as well.

@ymesika
Copy link
Contributor Author

ymesika commented May 25, 2017

Thanks @billabt I fixed the macOS issues.
However, I still need to figure out how to do something similar to #if OPENSSL_VERSION_NUMBER >= 0x10002000L in Swift.
Therefore not reopening yet.

@ymesika
Copy link
Contributor Author

ymesika commented May 28, 2017

I'm now testing the OpenSSL version number with the SSLeay() which can be found in any version of the library.
Reopening to make sure all build errors are now fixed.

@ymesika ymesika reopened this May 28, 2017
@ymesika
Copy link
Contributor Author

ymesika commented May 28, 2017

Logic for checking the OpenSSL version will be moved to the OpenSSL package.
Will be reopened once the OpenSSL PR (TBD) will be merged.

@ymesika ymesika closed this May 28, 2017
@ymesika
Copy link
Contributor Author

ymesika commented Jun 8, 2017

Reopening after the OpenSSL PR #3 dependency has been merged making this PR ready to go.

@ymesika ymesika reopened this Jun 8, 2017
@shmuelk shmuelk requested a review from billabt June 8, 2017 09:33
@@ -330,6 +333,20 @@ public class SSLService: SSLServiceDelegate {
public private(set) var context: SSLContext?

#endif

// MARK: ALPN
Copy link
Collaborator

Choose a reason for hiding this comment

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

The ALPN protocol is currently only for Linux. Please wrapper the implementation (where appropriate) in #if os(Linux).

@@ -330,6 +333,20 @@ public class SSLService: SSLServiceDelegate {
public private(set) var context: SSLContext?

#endif

// MARK: ALPN
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please update code to the latest version in master. I merged another PR today to correct a crash problem that changed a few things. Thanks.

niklassaers and others added 5 commits June 9, 2017 06:32
Not sure why Xcode didn’t do this by default
This is getting embarrasing. How do I make Xcode match the style / use tabs? Missed these two
@billabt billabt merged commit 3cf58c8 into Kitura:master Jun 9, 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.

None yet

3 participants