jchambers / pushy

A Java library for sending APNs (iOS/macOS/Safari) push notifications
https://pushy-apns.org/
MIT License
1.8k stars 450 forks source link

MITM attack vulnerability #884

Closed Drauggy closed 3 years ago

Drauggy commented 3 years ago

Hey! I ran into vulnerability like man in the middle attack in Pushy code. Pushy uses the Netty library under the hood, which does not include hostname verification logic by default (see https://github.com/netty/netty/issues/9930). This looks logical, since SSLContext does not know anything about the type of protocol used and, accordingly, does not know how to check the host correctly. Pushy does not validate the hostname of remote connection and does not validate it against alternative name in certificate from peer. I tested this vulnerability myself. This can be done most simply by replacing dns -name in / etc / hosts Thus, if an attacker organizes a spoofing attack (arp-spoofing, dns -spoofing ...) and stands in the middle, then using a certificate issued by a trusted certification authority, but in a name different from the name of the target address (api.push.apple.com, api .development.push.apple.com), then it will be able to receive automatically decrypted push messages. A logical solution to this problem, as I see it, may be to add the default HostnameVerifier class to the utils package. And to make it possible for the end user to change this verifier as the strategy pattern does. For version 0.13.10, which I am using in my project, you can do this as follows: Insert the following code into the ApnsChannelFactory class in the body of the initChannel method:

.....

 final HostnameVerifier v = new DefaultHostnameVerifier ();

                sslHandler.handshakeFuture (). addListener (new GenericFutureListener <Future <Channel>> () {
                    @Override
                    public void operationComplete (final Future <Channel> handshakeFuture) throws ConnectException, ExecutionException, InterruptedException {
                        if (handshakeFuture.isSuccess ()) {
                            if (! v.verify (apnsServerAddress.getHostName (),
                                          sslHandler.engine (). getSession ())) {
                                handshakeFuture.get (). close ();
                                throw new ConnectException ("HostnameVerifier exception.");

                            }

.... The implementation of the DefaultHostnameVerifier can be seen from the appache http client.

An example of a trace stack for a verifier's error when making a mitm attack:

11:33:10,418 ERROR [tech.paycon.server.pusher.service.AppleSender] (pusher_task_executor_thread1) Error while sending push for device 698603fd8ad3c937baa50813bdc7ca289dc02390e32de365338bed16cda77d70: java.util.concurrent.ExecutionException: java.lang.IllegalStateException: Channel closed before HTTP/2 preface completed.
    at io.netty.util.concurrent.DefaultPromise.get(DefaultPromise.java:350)
    at tech.paycon.server.pusher.service.AppleSender.pushIOSPushy(AppleSender.java:235)
    at tech.paycon.server.pusher.task.Task.run(Task.java:53)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.IllegalStateException: Channel closed before HTTP/2 preface completed.
    at com.turo.pushy.apns.ApnsChannelFactory$3$2.operationComplete(ApnsChannelFactory.java:247)
    at com.turo.pushy.apns.ApnsChannelFactory$3$2.operationComplete(ApnsChannelFactory.java:241)
    at io.netty.util.concurrent.DefaultPromise.notifyListener0(DefaultPromise.java:578)
    at io.netty.util.concurrent.DefaultPromise.notifyListenersNow(DefaultPromise.java:552)
    at io.netty.util.concurrent.DefaultPromise.notifyListeners(DefaultPromise.java:491)
    at io.netty.util.concurrent.DefaultPromise.setValue0(DefaultPromise.java:616)
    at io.netty.util.concurrent.DefaultPromise.setSuccess0(DefaultPromise.java:605)
    at io.netty.util.concurrent.DefaultPromise.trySuccess(DefaultPromise.java:104)
    at io.netty.channel.DefaultChannelPromise.trySuccess(DefaultChannelPromise.java:84)
    at io.netty.channel.AbstractChannel$CloseFuture.setClosed(AbstractChannel.java:1186)
    at io.netty.channel.AbstractChannel$AbstractUnsafe.doClose0(AbstractChannel.java:773)
    at io.netty.channel.AbstractChannel$AbstractUnsafe.close(AbstractChannel.java:749)
    at io.netty.channel.AbstractChannel$AbstractUnsafe.close(AbstractChannel.java:620)
    at io.netty.channel.DefaultChannelPipeline$HeadContext.close(DefaultChannelPipeline.java:1352)
    at io.netty.channel.AbstractChannelHandlerContext.invokeClose(AbstractChannelHandlerContext.java:622)
    at io.netty.channel.AbstractChannelHandlerContext.close(AbstractChannelHandlerContext.java:606)
    at io.netty.handler.ssl.SslHandler$8.operationComplete(SslHandler.java:2140)
    at io.netty.handler.ssl.SslHandler$8.operationComplete(SslHandler.java:2129)
    at io.netty.util.concurrent.DefaultPromise.notifyListener0(DefaultPromise.java:578)
    at io.netty.util.concurrent.DefaultPromise.notifyListenersNow(DefaultPromise.java:552)
    at io.netty.util.concurrent.DefaultPromise.notifyListeners(DefaultPromise.java:491)
    at io.netty.util.concurrent.DefaultPromise.addListener(DefaultPromise.java:184)
    at io.netty.channel.DefaultChannelPromise.addListener(DefaultChannelPromise.java:95)
    at io.netty.channel.DefaultChannelPromise.addListener(DefaultChannelPromise.java:30)
    at io.netty.handler.ssl.SslHandler.safeClose(SslHandler.java:2129)
    at io.netty.handler.ssl.SslHandler.closeOutboundAndChannel(SslHandler.java:1900)
    at io.netty.handler.ssl.SslHandler.close(SslHandler.java:745)
    at io.netty.channel.AbstractChannelHandlerContext.invokeClose(AbstractChannelHandlerContext.java:622)
    at io.netty.channel.AbstractChannelHandlerContext.close(AbstractChannelHandlerContext.java:606)
    at io.netty.channel.AbstractChannelHandlerContext.close(AbstractChannelHandlerContext.java:472)
    at io.netty.channel.DefaultChannelPipeline.close(DefaultChannelPipeline.java:957)
    at io.netty.channel.AbstractChannel.close(AbstractChannel.java:244)
    at com.turo.pushy.apns.ApnsChannelFactory$1$1.operationComplete(ApnsChannelFactory.java:143)
    at io.netty.util.concurrent.DefaultPromise.notifyListener0(DefaultPromise.java:578)
    at io.netty.util.concurrent.DefaultPromise.notifyListeners0(DefaultPromise.java:571)
    at io.netty.util.concurrent.DefaultPromise.notifyListenersNow(DefaultPromise.java:550)
    at io.netty.util.concurrent.DefaultPromise.notifyListeners(DefaultPromise.java:491)
    at io.netty.util.concurrent.DefaultPromise.setValue0(DefaultPromise.java:616)
    at io.netty.util.concurrent.DefaultPromise.setSuccess0(DefaultPromise.java:605)
    at io.netty.util.concurrent.DefaultPromise.trySuccess(DefaultPromise.java:104)
    at io.netty.handler.ssl.SslHandler.setHandshakeSuccess(SslHandler.java:1780)
    at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1438)
    at io.netty.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1253)
    at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1300)
    at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:508)
    at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:447)
    at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:276)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
    at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
    at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
    at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
    at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:719)
    at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:655)
    at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:581)
    at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:493)
    at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:989)
    at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
    at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
    ... 1 more

11:33:10,419 WARN [io.netty.util.concurrent.DefaultPromise] (nioEventLoopGroup-2-1) An exception was thrown by com.turo.pushy.apns.ApnsChannelFactory$1$1.operationComplete(): java.net.ConnectException: HostnameVerifier exception.

    at io.netty.util.concurrent.DefaultPromise.notifyListener0(DefaultPromise.java:578)
    at io.netty.util.concurrent.DefaultPromise.notifyListeners0(DefaultPromise.java:571)
    at io.netty.util.concurrent.DefaultPromise.notifyListenersNow(DefaultPromise.java:550)
    at io.netty.util.concurrent.DefaultPromise.notifyListeners(DefaultPromise.java:491)
    at io.netty.util.concurrent.DefaultPromise.setValue0(DefaultPromise.java:616)
    at io.netty.util.concurrent.DefaultPromise.setSuccess0(DefaultPromise.java:605)
    at io.netty.util.concurrent.DefaultPromise.trySuccess(DefaultPromise.java:104)
    at io.netty.handler.ssl.SslHandler.setHandshakeSuccess(SslHandler.java:1780)
    at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1438)
    at io.netty.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1253)
    at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1300)
    at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:508)
    at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:447)
    at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:276)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
    at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
    at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
    at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
    at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:719)
    at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:655)
    at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:581)
    at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:493)
    at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:989)
    at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
    at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
    at java.lang.Thread.run(Thread.java:748)

11:33:10,419 ERROR [stderr] (pusher_task_executor_thread1) java.util.concurrent.ExecutionException: java.lang.IllegalStateException: Channel closed before HTTP/2 preface completed.

jchambers commented 3 years ago

Thank you for the report. I'll look into it.

It's my fault for not having clear instructions, but in the future, please do not report security vulnerabilities in public issues. Doing so advertises the vulnerabilities to bad actors until a new release is out and a majority of users have updated to the new release. Again, I emphasize that this is on me for not publishing clear instructions for what to do instead, and I'll make sure to get those instructions published as part of this issue.

Drauggy commented 3 years ago

Sorry for that!

jchambers commented 3 years ago

The core issue is addressed in #889. Feedback on the implementation is certainly welcome.