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

New BoringSSL context option - SSL_CTX_set1_sigalgs #765

Merged
merged 19 commits into from
Dec 23, 2024

Conversation

jaymansfield
Copy link
Contributor

My use case of this library had the requirement to use a different signing algorithm and certificate type (ed25519 to be specific).

Changes:

  • New BoringSSL context option - SSL_CTX_set1_sigalgs (this expands on Allow to configure groups via BoringSSLContextOption #755 )
  • Allowing certificate types in certificate callback to be modified (allows the implementer to override the defaults and use any certificate type they want now)

Result:

This increases the overall flexibility of the quic implementation.

jaymansfield and others added 3 commits December 7, 2024 20:52
- Allowing certificate types in certificate callback to be modified (allows for any certificate type to be used now)
jaymansfield and others added 2 commits December 11, 2024 21:11
…/BoringSSL.java


Removed null check as per normanmaurer

Co-authored-by: Norman Maurer <[email protected]>
…/BoringSSL.java


Code style fix as per normanmaurer

Co-authored-by: Norman Maurer <[email protected]>
@jaymansfield
Copy link
Contributor Author

@normanmaurer Two new SSLContextOption's have been added for client and server certificate/key types

Copy link
Member

@normanmaurer normanmaurer left a comment

Choose a reason for hiding this comment

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

LGTM @jaymansfield did you sign our icla yet ? https://netty.io/s/icla

@normanmaurer normanmaurer added this to the 0.0.70.Final milestone Dec 12, 2024
@jaymansfield
Copy link
Contributor Author

LGTM @jaymansfield did you sign our icla yet ? https://netty.io/s/icla

Yes just now.

@normanmaurer
Copy link
Member

@jaymansfield thanks a lot... will merge when the build passed.

@normanmaurer
Copy link
Member

@jaymansfield would it be possible to add a unit test as well ?

@normanmaurer
Copy link
Member

I think this would fit well in QuicChannelConnectTest

@jaymansfield
Copy link
Contributor Author

jaymansfield commented Dec 13, 2024

I think this would fit well in QuicChannelConnectTest

I added two very basic ones for the key type change. One that passes by allowing only RSA keys (using your existing self signed test certificate which is RSA), and one that is expected to fail since the self signed certificate doesn't match the newly specific key type (EdDSA) in the test case

@normanmaurer
Copy link
Member

@jaymansfield please apply this change:

diff --git a/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java b/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java
index 099d244..c78436c 100644
--- a/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java
+++ b/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java
@@ -27,6 +27,7 @@ import io.netty.channel.ChannelOutboundHandlerAdapter;
 import io.netty.channel.ChannelPromise;
 import io.netty.channel.ConnectTimeoutException;
 import io.netty.channel.socket.ChannelInputShutdownEvent;
+import io.netty.handler.codec.ByteToMessageDecoder;
 import io.netty.handler.ssl.ClientAuth;
 import io.netty.handler.ssl.SniCompletionEvent;
 import io.netty.handler.ssl.SslHandshakeCompletionEvent;
@@ -69,8 +70,18 @@ import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
 import java.security.spec.MGF1ParameterSpec;
 import java.security.spec.PSSParameterSpec;
-import java.util.*;
-import java.util.concurrent.*;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Executor;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
@@ -741,24 +752,16 @@ public class QuicChannelConnectTest extends AbstractQuicTest {
                     public boolean isSharable() {
                         return true;
                     }
-                }, new ChannelInboundHandlerAdapter() {
+                }, new ByteToMessageDecoder() {
                     @Override
-                    public boolean isSharable() {
-                        return true;
-                    }
-
-                    @Override
-                    public void channelRead(ChannelHandlerContext ctx, Object msg) {
-                        ByteBuf buffer = (ByteBuf) msg;
-                        try {
-                            assertEquals(4, buffer.readableBytes());
-                            assertEquals(5, buffer.readInt());
-                            readLatch.countDown();
-
-                            ctx.close();
-                        } finally {
-                            buffer.release();
+                    protected void decode(ChannelHandlerContext ctx, ByteBuf msg, List<Object> out) throws Exception {
+                        if (msg.readableBytes() < 4) {
+                            return;
                         }
+                        assertEquals(5, msg.readInt());
+                        readLatch.countDown();
+
+                        ctx.close();
                     }
                 });
 

@jaymansfield
Copy link
Contributor Author

@jaymansfield please apply this change:

diff --git a/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java b/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java
index 099d244..c78436c 100644
--- a/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java
+++ b/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java
@@ -27,6 +27,7 @@ import io.netty.channel.ChannelOutboundHandlerAdapter;
 import io.netty.channel.ChannelPromise;
 import io.netty.channel.ConnectTimeoutException;
 import io.netty.channel.socket.ChannelInputShutdownEvent;
+import io.netty.handler.codec.ByteToMessageDecoder;
 import io.netty.handler.ssl.ClientAuth;
 import io.netty.handler.ssl.SniCompletionEvent;
 import io.netty.handler.ssl.SslHandshakeCompletionEvent;
@@ -69,8 +70,18 @@ import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
 import java.security.spec.MGF1ParameterSpec;
 import java.security.spec.PSSParameterSpec;
-import java.util.*;
-import java.util.concurrent.*;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Executor;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
@@ -741,24 +752,16 @@ public class QuicChannelConnectTest extends AbstractQuicTest {
                     public boolean isSharable() {
                         return true;
                     }
-                }, new ChannelInboundHandlerAdapter() {
+                }, new ByteToMessageDecoder() {
                     @Override
-                    public boolean isSharable() {
-                        return true;
-                    }
-
-                    @Override
-                    public void channelRead(ChannelHandlerContext ctx, Object msg) {
-                        ByteBuf buffer = (ByteBuf) msg;
-                        try {
-                            assertEquals(4, buffer.readableBytes());
-                            assertEquals(5, buffer.readInt());
-                            readLatch.countDown();
-
-                            ctx.close();
-                        } finally {
-                            buffer.release();
+                    protected void decode(ChannelHandlerContext ctx, ByteBuf msg, List<Object> out) throws Exception {
+                        if (msg.readableBytes() < 4) {
+                            return;
                         }
+                        assertEquals(5, msg.readInt());
+                        readLatch.countDown();
+
+                        ctx.close();
                     }
                 });
 

Thanks for this! Done.

Copy link
Member

@normanmaurer normanmaurer left a comment

Choose a reason for hiding this comment

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

Almost there... two more things. After this we can merge

jaymansfield and others added 2 commits December 22, 2024 22:16
@normanmaurer normanmaurer merged commit c20187b into netty:main Dec 23, 2024
14 checks passed
@normanmaurer
Copy link
Member

@jaymansfield thanks a lot again

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.

2 participants