expired certificate / .p12 file

[matzew]

Hi,

when there is a certifacte that is expried, we noticed a good number of verbose logs, such as:

18:55:27,238 INFO  [com.turo.pushy.apns.ApnsClientHandler] (nioEventLoopGroup-16-1) Received GOAWAY from APNs server: {"reason":"Shutdown"}
18:55:28,531 WARN  [io.netty.handler.ssl.ApplicationProtocolNegotiationHandler] (nioEventLoopGroup-16-1) [id: 0xf084a1d3, L:/10.41.10.69:56147 - R:api.push.apple.com/17.188.156.33:443] TLS handshake failed:: javax.net.ssl.SSLHandshakeException: error:10000415:SSL routines:OPENSSL_internal:SSLV3_ALERT_CERTIFICATE_EXPIRED
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.shutdownWithError(ReferenceCountedOpenSslEngine.java:751) [netty-handler-4.1.11.Final.jar:4.1.11.Final]
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.sslReadErrorResult(ReferenceCountedOpenSslEngine.java:959) [netty-handler-4.1.11.Final.jar:4.1.11.Final]

and

18:55:28,536 INFO  [com.turo.pushy.apns.ApnsClient] (nioEventLoopGroup-16-1) Failed to connect.: java.lang.IllegalStateException: Channel closed before HTTP/2 preface completed.
	at com.turo.pushy.apns.ApnsClient$3.operationComplete(ApnsClient.java:395) [pushy-0.10.jar:]
	at com.turo.pushy.apns.ApnsClient$3.operationComplete(ApnsClient.java:387) [pushy-0.10.jar:]
...

and

18:55:28,541 WARN  [io.netty.channel.DefaultChannelPipeline] (nioEventLoopGroup-16-1) An exceptionCaught() event was fired, and it reached at the tail of the pipeline. It usually means the last handler in the pipeline did not handle the exception.: io.netty.handler.codec.DecoderException: javax.net.ssl.SSLHandshakeException: error:10000415:SSL routines:OPENSSL_internal:SSLV3_ALERT_CERTIFICATE_EXPIRED
	at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:459) [netty-codec-4.1.11.Final.jar:4.1.11.Final]
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:265) [netty-codec-4.1.11.Final.jar:4.1.11.Final]
...

While we now are catching this (e.g. check if the cert is expired per call), would it make sense to have some sort of handling of this in pushy itself ? E.g. that the client is not bothering w/ the HTTP2 connection in case the cert is expired ?

[jchambers]

…would it make sense to have some sort of handling of this in pushy itself?

Possibly; I'm trying to think of how we'd do it efficiently. One weird thing about SSL contexts is that it's really hard to get information out after you've put it in, so we'd need to check the expiration at construction time and then run a lot of plumbing to get it to… wherever we decide to do the check.

It sounds like there's an opportunity to do some better error reporting here at the very least, though. What version of Pushy are you using? I had hoped that #602 would cover this kind of thing, and that went out in 0.13.

Thanks!

[matzew]

No, we use 0.10.2 of the library, I think it's the last release before the refactoring you did.

Here is what we basically do: https://github.com/aerogear/aerogear-unifiedpush-server/blob/master/push-sender/src/main/java/org/jboss/aerogear/unifiedpush/message/sender/apns/ApnsUtil.java#L79-L109

@pb82 Is now the new maintainer for the code base, cc'd him here

[panchenko]

Checking certificate beforehand is surely the right thing to do. I can see you already use the checkValidity() method, there is also another one - checkValidity(Date). I would recommend calling it with new Date(System.currentTimeMillis()+TimeUnit.DAYS.toMillis(14)) and sending an alert to your monitoring if certificate expires soon. I have also developed some functions similar to yours, IMHO it would make sense to have this code in pushy.

[jchambers]

This might actually make more sense as part of (or a follow up) to #540. To get to a place where a single client handles multiple sets of credentials, we'll need to develop some notion of a credential object that we can pass around; attaching some metadata to that for an expiration time seems clean and effective.

[jchambers]

…but to be clear about the big picture: yes, I think this makes sense.

[Iris-zhang-072611]

java.util.concurrent.ExecutionException: javax.net.ssl.SSLHandshakeException: error:10000438:SSL routines:OPENSSL_internal:TLSV1_ALERT_INTERNAL_ERROR at io.netty.util.concurrent.AbstractFuture.get(AbstractFuture.java:41) at com.jxd.test.Test.main(Test.java:40)

[jchambers]

I've reflected on this a bit and, even though I recognize it might be disappointing, I don't think adding expired certificate detection to Pushy is the right way forward. In short, I don't think there's any one right way to do this that will please everybody, and the effort involved in checking certificates directly is fairly mild. With that in mind, let's close this issue.

Thank you all for your patience and understanding.