Repository: netty/netty
Issue: OpenSslSession#initPeerCerts creates too long almost empty arrays
Labels: defect

netty 4.1.6 

io.netty.handler.ssl.ReferenceCountedOpenSslEngine.OpenSslSession#initPeerCerts

```
final byte[] clientCert;
...
int len = clientCert.length + 1; // <------ BUG: raw certificate byte array size is used
peerCerts = new Certificate[len];
x509PeerCerts = new X509Certificate[len];

```

But should probably be

`int len = chain.length + 1; // <------ so we have first entry as client cert + chain.length`


Issue comments:
Maybe want to provide a PR with a fix?

> Am 26.10.2016 um 21:25 schrieb ichaki5748 notifications@github.com:
> 
> netty 4.1.6
> 
> io.netty.handler.ssl.ReferenceCountedOpenSslEngine.OpenSslSession#initPeerCerts
> 
> final byte[] clientCert;
> ...
> int len = clientCert.length + 1; // <------ BUG: raw certificate byte array size is used
> peerCerts = new Certificate[len];
> x509PeerCerts = new X509Certificate[len];
> 
> But should probably be
> 
> int len = chain.length + 1; // <------ so we have first entry as client cert + chain.length
> 
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub, or mute the thread.

Yes, sure, will try to do it this week 

Thx

> Am 26.10.2016 um 23:54 schrieb ichaki5748 notifications@github.com:
> 
> Yes, sure, will try to do it this week
> 
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or mute the thread.

Also `io.netty.handler.ssl.OpenSslJavaxX509Certificate.*`
could theoretically blow in concurrent usage because of lazy init in `io.netty.handler.ssl.OpenSslJavaxX509Certificate#unwrap` because 
`io.netty.handler.ssl.OpenSslJavaxX509Certificate#wrapped` is not `volatile` (certificate would not be fully initialized)? 

@iainmcgin please open separate PRs for the separate issues and ping me once you are ready. I will review then

You probably meant to reference @ichaki5748

@iainmcgin ups yes... sorry I need more ☕️ 

@normanmaurer Hi, please see https://github.com/netty/netty/pull/5967 https://github.com/netty/netty/pull/5966

Fixed by https://github.com/netty/netty/pull/5967


Pull request review comments:

Patch that resolved the issue:
diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslJavaxX509Certificate.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslJavaxX509Certificate.java
index da10dedf0c1..ad4447f34b7 100644
--- a/handler/src/main/java/io/netty/handler/ssl/OpenSslJavaxX509Certificate.java
+++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslJavaxX509Certificate.java
@@ -30,7 +30,7 @@
 
 final class OpenSslJavaxX509Certificate extends X509Certificate {
     private final byte[] bytes;
-    private X509Certificate wrapped;
+    private volatile X509Certificate wrapped;
 
     public OpenSslJavaxX509Certificate(byte[] bytes) {
         this.bytes = bytes;
diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslX509Certificate.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslX509Certificate.java
index 77d0713613a..faaac551a41 100644
--- a/handler/src/main/java/io/netty/handler/ssl/OpenSslX509Certificate.java
+++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslX509Certificate.java
@@ -34,7 +34,7 @@
 final class OpenSslX509Certificate extends X509Certificate {
 
     private final byte[] bytes;
-    private X509Certificate wrapped;
+    private volatile X509Certificate wrapped;
 
     public OpenSslX509Certificate(byte[] bytes) {
         this.bytes = bytes;


Test patch: