Skip to content

Commit

Permalink
Minor fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
marianobarrios committed May 10, 2024
1 parent ac3c4f8 commit e6db1d0
Show file tree
Hide file tree
Showing 21 changed files with 56 additions and 59 deletions.
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Being an API layer, TLS Channel *delegates all cryptographic operations to SSLEn

## Rationale

The world's most used encryption protocol is TLS. Created by Netscape in 1994 as SSL (Secure Socket Layer), it experimented widespread adoption, which eventually let to its standardization. TLS works on top of the Transport Control Protocol (TCP), maintaining its core abstractions: two independent byte streams, one in each direction, with ordered at-most-once delivery. It can be argued that part of the success of TLS was due to its convenient programming interface, similar to the highly successful and familiar Berkeley Sockets. Currenty, there exist a few widely-used implementations:
The world's most used encryption protocol is TLS. Created by Netscape in 1994 as SSL (Secure Socket Layer), it experimented widespread adoption, which eventually let to its standardization. TLS works on top of the Transport Control Protocol (TCP), maintaining its core abstractions: two independent byte streams, one in each direction, with ordered at-most-once delivery. It can be argued that part of the success of TLS was due to its convenient programming interface, similar to the highly successful and familiar Berkeley Sockets. Currently, there exist a few widely-used implementations:

- The most used TLS library is [OpenSSL](https://www.openssl.org/). Written in C and (along with some forks) the *de facto* standard for C and C++. Also widely used in Python, PHP, Ruby and Node.js.
- The Go language has its own implementation, package [crypto/tls](https://golang.org/pkg/crypto/tls/).
Expand All @@ -59,8 +59,8 @@ But no TLS support, which was only available in old-style sockets.

Version 1.5 saw the advent of [SSLEngine](https://docs.oracle.com/javase/8/docs/api/javax/net/ssl/SSLEngine.html) as the official way of doing TLS over NIO sockets. This API has been the official option for more than a decade. However, it has severe shortcomings:

- No streaming support. SSLEngine does not do any I/O, or keep any buffers. It does all cryptographic operations on user-managed buffers (but, confusingly, at the same time keeps internal state associated with the TLS connection). This no-data but stateful API is just not what users expect or are used to, and indeed not what the rest of the industry has standarized on.
- Even considering the constrains, the API is unnecessarily convoluted, with too big a surface, and many incorrect interactions just not prevented at compile-time. It's just extremely hard to use correctly.
- No streaming support. SSLEngine does not do any I/O, or keep any buffers. It does all cryptographic operations on user-managed buffers (but, confusingly, at the same time keeps internal state associated with the TLS connection). This no-data but stateful API is just not what users expect or are used to, and indeed not what the rest of the industry has standardized on.
- Even considering the constraints, the API is unnecessarily convoluted, with too big a surface, and many incorrect interactions just not prevented at compile-time. It's just extremely hard to use correctly.
- No support for server-side SNI handling.

#### What to do
Expand Down Expand Up @@ -132,7 +132,7 @@ Standard ByteChannel instances communicate the fact that operations would block

Ideally, a more complex return type would suffice—not merely an `int` but some object including more information. For instance, OpenSSL uses special error codes for these conditions: `SSL_ERROR_WANT_READ` and `SSL_ERROR_WANT_WRITE`.

In the case of TLS Channel, it is in practice necessary to maintain compatibility with the existing ByteChannel interface. That's why an somewhat unorthodox approach is used: when the operation would block, special exceptions are thrown: [NeedsReadException](https://oss.sonatype.org/service/local/repositories/releases/archive/com/github/marianobarrios/tls-channel/0.1.0/tls-channel-0.1.0-javadoc.jar/!/index.html?tlschannel/NeedsReadException.html) and [NeedsWriteException](https://oss.sonatype.org/service/local/repositories/releases/archive/com/github/marianobarrios/tls-channel/0.1.0/tls-channel-0.1.0-javadoc.jar/!/index.html?tlschannel/NeedsWriteException.html), meaning that the operation should be retried when the underlying channel is ready for reading or writing, respectively.
In the case of TLS Channel, it is in practice necessary to maintain compatibility with the existing ByteChannel interface. That's why a somewhat unorthodox approach is used: when the operation would block, special exceptions are thrown: [NeedsReadException](https://oss.sonatype.org/service/local/repositories/releases/archive/com/github/marianobarrios/tls-channel/0.1.0/tls-channel-0.1.0-javadoc.jar/!/index.html?tlschannel/NeedsReadException.html) and [NeedsWriteException](https://oss.sonatype.org/service/local/repositories/releases/archive/com/github/marianobarrios/tls-channel/0.1.0/tls-channel-0.1.0-javadoc.jar/!/index.html?tlschannel/NeedsWriteException.html), meaning that the operation should be retried when the underlying channel is ready for reading or writing, respectively.

Typical usage inside a selector loop looks like this:

Expand Down
4 changes: 2 additions & 2 deletions src/main/java/tlschannel/ClientTlsChannel.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ private static SSLEngine defaultSSLEngineFactory(SSLContext sslContext) {
}

/**
* Create a new {@link Builder}, configured with a underlying {@link Channel} and a fixed {@link
* Create a new {@link Builder}, configured with an underlying {@link Channel} and a fixed {@link
* SSLEngine}.
*
* @param underlying a reference to the underlying {@link ByteChannel}
Expand All @@ -68,7 +68,7 @@ public static Builder newBuilder(ByteChannel underlying, SSLEngine sslEngine) {
}

/**
* Create a new {@link Builder}, configured with a underlying {@link Channel} and a {@link
* Create a new {@link Builder}, configured with an underlying {@link Channel} and a {@link
* SSLContext}.
*
* @param underlying a reference to the underlying {@link ByteChannel}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/tlschannel/NeedsWriteException.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

/**
* This exception signals the caller that the operation cannot continue because bytesProduced need
* to be write to the underlying {@link ByteChannel}, the channel is non-blocking and there are no
* to be written to the underlying {@link ByteChannel}, the channel is non-blocking and there are no
* buffer space available. The caller should try the operation again, either with the channel in
* blocking mode of after ensuring that buffer space exists.
*
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/tlschannel/ServerTlsChannel.java
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public ServerTlsChannel build() {
}

/**
* Create a new {@link Builder}, configured with a underlying {@link Channel} and a fixed {@link
* Create a new {@link Builder}, configured with an underlying {@link Channel} and a fixed {@link
* SSLContext}, which will be used to create the {@link SSLEngine}.
*
* @param underlying a reference to the underlying {@link ByteChannel}
Expand All @@ -144,9 +144,9 @@ public static Builder newBuilder(ByteChannel underlying, SSLContext sslContext)
}

/**
* Create a new {@link Builder}, configured with a underlying {@link Channel} and a custom {@link
* Create a new {@link Builder}, configured with an underlying {@link Channel} and a custom {@link
* SSLContext} factory, which will be used to create the context (in turn used to create the
* {@link SSLEngine}, as a function of the SNI received at the TLS connection start.
* {@link SSLEngine}), as a function of the SNI received at the TLS connection start.
*
* <p><b>Implementation note:</b><br>
* Due to limitations of {@link SSLEngine}, configuring a {@link ServerTlsChannel} to select the
Expand Down Expand Up @@ -220,7 +220,7 @@ public ByteChannel getUnderlying() {
/**
* Return the used {@link SSLContext}.
*
* @return if context if present, of null if the TLS connection as not been initializer, or the
* @return context if present, or null if the TLS connection as not been initializer, or the
* SNI not received yet.
*/
public SSLContext getSslContext() {
Expand Down
20 changes: 10 additions & 10 deletions src/main/java/tlschannel/TlsChannel.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* <p>In other words, an interface that allows the programmer to have TLS using the same standard
* socket API used for plaintext, just like OpenSSL does for C, only for Java.
*
* <p>Note that this is an API adapter, not a cryptographic implementation: with the exception of a
* <p>Note that this is an API adapter, not a cryptographic implementation: except for a
* few bytesProduced of parsing at the beginning of the connection, to look for the SNI, the whole
* protocol implementation is done by the SSLEngine. Both the SSLContext and SSLEngine are supplied
* by the client; these classes are the ones responsible for protocol configuration, including
Expand Down Expand Up @@ -114,9 +114,9 @@ public interface TlsChannel extends ByteChannel, GatheringByteChannel, Scatterin
* its limit will not have changed.
*
* <p>A read operation might not fill the buffer, and in fact it might not read any bytesProduced
* at all. Whether or not it does so depends upon the nature and state of the underlying channel.
* at all. Whether it does so depends upon the nature and state of the underlying channel.
* It is guaranteed, however, that if a channel is in blocking mode and there is at least one byte
* remaining in the buffer then this method will block until at least one byte is read. On the
* remaining in the buffer, then this method will block until at least one byte is read. On the
* other hand, if the underlying channel is in non-blocking mode then a {@link
* WouldBlockException} may be thrown. Note that this also includes the possibility of a {@link
* NeedsWriteException}, due to the fact that, during a TLS handshake, bytesProduced need to be
Expand Down Expand Up @@ -252,7 +252,7 @@ public interface TlsChannel extends ByteChannel, GatheringByteChannel, Scatterin
* <p>See {@link GatheringByteChannel#write(ByteBuffer[], int, int)} for more details of the
* meaning of this signature.
*
* <p>This method behaves slightly different than the interface specification, with respect to
* <p>This method behaves slightly different from the interface specification, with respect to
* non-blocking responses, see {@link #write(ByteBuffer)} for more details.
*
* @param srcs The buffers from which bytesProduced are to be retrieved
Expand Down Expand Up @@ -287,7 +287,7 @@ public interface TlsChannel extends ByteChannel, GatheringByteChannel, Scatterin
*
* </blockquote>
*
* This method behaves slightly different than the interface specification, with respect to
* This method behaves slightly different from the interface specification, with respect to
* non-blocking responses, see {@link #write(ByteBuffer)} for more details.
*
* @param srcs The buffers from which bytesProduced are to be retrieved
Expand All @@ -310,7 +310,7 @@ public interface TlsChannel extends ByteChannel, GatheringByteChannel, Scatterin
* <p>See {@link ScatteringByteChannel#read(ByteBuffer[], int, int)} for more details of the
* meaning of this signature.
*
* <p>This method behaves slightly different than the interface specification, with respect to
* <p>This method behaves slightly different from the interface specification, with respect to
* non-blocking responses, see {@link #read(ByteBuffer)} for more details.
*
* @param dsts The buffers into which bytesProduced are to be transferred
Expand Down Expand Up @@ -346,7 +346,7 @@ public interface TlsChannel extends ByteChannel, GatheringByteChannel, Scatterin
*
* </blockquote>
*
* <p>This method behaves slightly different than the interface specification, with respect to
* <p>This method behaves slightly different from the interface specification, with respect to
* non-blocking responses, see {@link #read(ByteBuffer)} for more details.
*
* @param dsts The buffers into which bytesProduced are to be transferred
Expand All @@ -369,7 +369,7 @@ public interface TlsChannel extends ByteChannel, GatheringByteChannel, Scatterin
* done. The exact behavior can be configured using the {@link
* TlsChannelBuilder#withWaitForCloseConfirmation}.
*
* <p>The default behavior mimics what happens in a normal (that is, non layered) {@link
* <p>The default behavior mimics what happens in a normal (that is, non-layered) {@link
* javax.net.ssl.SSLSocket#close()}.
*
* <p>For finer control of the TLS close, use {@link #shutdown()}
Expand All @@ -390,7 +390,7 @@ public interface TlsChannel extends ByteChannel, GatheringByteChannel, Scatterin
* used for more communications, the complete shutdown procedure (bidirectional "close notify"
* alerts) must be performed, so that the peers stay synchronized.
*
* <p>This class supports both uni- and bidirectional shutdown by its 2 step behavior, using this
* <p>This class supports both uni- and bidirectional shutdown by its 2-step behavior, using this
* method.
*
* <p>When this is the first party to send the "close notify" alert, this method will only send
Expand All @@ -400,7 +400,7 @@ public interface TlsChannel extends ByteChannel, GatheringByteChannel, Scatterin
* for the peer's "close notify" shutdown alert. On success, the second call will return <code>
* true</code>.
*
* <p>If the peer already sent the "close notify" alert and it was already processed implicitly
* <p>If the peer already sent the "close notify" alert, and it was already processed implicitly
* inside a read operation, the {@link #shutdownReceived()} flag is already set. This method will
* then send the "close notify" alert, set the {@link #shutdownSent()} flag and immediately return
* <code>true</code>. It is therefore recommended to check the return value of this method and
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/tlschannel/TlsChannelBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ public T withRunTasks(boolean runTasks) {
}

/**
* Set the {@link BufferAllocator} to use for unencrypted data. By default a {@link
* HeapBufferAllocator} is used, as this buffers are used to supplement user-supplied ones when
* Set the {@link BufferAllocator} to use for unencrypted data. By default, a {@link
* HeapBufferAllocator} is used, as these buffers are used to supplement user-supplied ones when
* dealing with too big a TLS record, that is, they operate entirely inside the JVM.
*
* @param bufferAllocator the buffer allocator
Expand All @@ -52,7 +52,7 @@ public T withPlainBufferAllocator(BufferAllocator bufferAllocator) {
}

/**
* Set the {@link BufferAllocator} to use for encrypted data. By default a {@link
* Set the {@link BufferAllocator} to use for encrypted data. By default, a {@link
* DirectBufferAllocator} is used, as this data is usually read from or written to native sockets.
*
* @param bufferAllocator the buffer allocator
Expand Down Expand Up @@ -80,7 +80,7 @@ public T withSessionInitCallback(Consumer<SSLSession> sessionInitCallback) {
* Whether to release unused buffers in the mid of connections. Equivalent to OpenSSL's
* SSL_MODE_RELEASE_BUFFERS.
*
* <p>Default is to release. Releasing unused buffers is specially effective in the case case of
* <p>Default is to release. Releasing unused buffers is specially effective in the case of
* idle long-lived connections, when the memory footprint can be reduced significantly. A
* potential reason for setting this value to <code>false</code> is performance, since more
* releases means more allocations, which have a cost. This is effectively a memory-time
Expand All @@ -98,7 +98,7 @@ public T withReleaseBuffers(boolean releaseBuffers) {
* Whether to wait for TLS close confirmation when executing a local {@link TlsChannel#close()} on
* the channel. If the underlying channel is blocking, setting this to <code>true</code> will
* block (potentially until it times out, or indefinitely) the close operation until the
* counterpart confirms the close on their side (sending a close_notify alert. If the underlying
* counterpart confirms the close on their side (sending a close_notify alert). If the underlying
* channel is non-blocking, setting this parameter to true is ineffective.
*
* <p>Setting this value to <code>true</code> emulates the behavior of {@link SSLSocket} when used
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/tlschannel/WouldBlockException.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package tlschannel;

/**
* Signals that some IO operation cannot continue because the channel is in non blocking mode and
* Signals that some IO operation cannot continue because the channel is in non-blocking mode and
* some blocking would otherwise happen.
*/
public class WouldBlockException extends TlsChannelFlowControlException {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/tlschannel/async/AsynchronousTlsChannel.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public boolean cancel(boolean mayInterruptIfRunning) {
/**
* Initializes a new instance of this class.
*
* @param channelGroup group to associate new new channel to
* @param channelGroup group to associate new channel to
* @param tlsChannel existing TLS channel to be used asynchronously
* @param socketChannel underlying socket
* @throws ClosedChannelException if any of the underlying channels are closed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class RegisteredSocket {
final SocketChannel socketChannel;

/**
* Used to wait until the channel is effectively in the selector (which happens asynchronously
* Used to wait until the channel is effectively in the selector (which happens asynchronously)
* to the initial registration.
*/
final CountDownLatch registered = new CountDownLatch(1);
Expand Down Expand Up @@ -185,7 +185,7 @@ public AsynchronousTlsChannelGroup(int nThreads) {
selectorThread.start();
}

/** Creates an instance of this class, using as many thread as available processors. */
/** Creates an instance of this class, using as many threads as available processors. */
public AsynchronousTlsChannelGroup() {
this(Runtime.getRuntime().availableProcessors());
}
Expand Down Expand Up @@ -356,7 +356,7 @@ private void loop() {
while (shutdown == Shutdown.No
|| shutdown == Shutdown.Wait && (!pendingRegistrations.isEmpty() || !registrations.isEmpty())) {
// most state-changing operations will wake the selector up, however, asynchronous closings
// of the channels won't, so we have to timeout to allow checking those cases
// of the channels won't, so we have to time out to allow checking those cases
int c = selector.select(100); // block
selectionCount.increment();
// avoid unnecessary creation of iterator object
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/tlschannel/impl/TlsChannelImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ public void renegotiate() throws IOException {
* Renegotiation was removed in TLS 1.3. We have to do the check at this level because SSLEngine will not
* check that, and just enter into undefined behavior.
*/
// relying in hopefully-robust lexicographic ordering of protocol names
// relying on hopefully-robust lexicographic ordering of protocol names
if (engine.getSession().getProtocol().compareTo("TLSv1.3") >= 0) {
throw new SSLException("renegotiation not supported in TLS 1.3 or latter");
}
Expand Down
Loading

0 comments on commit e6db1d0

Please sign in to comment.