From c082fc8fa68a9d6088674ff8c5034edda64691bc Mon Sep 17 00:00:00 2001 From: zzz Date: Fri, 23 Sep 2022 09:14:03 -0400 Subject: [PATCH] SSU2: Fail handshakes while inside sync to prevent races and fail-after-fail ISE Override fail() to destroy handshake state --- .../transport/udp/EstablishmentManager.java | 6 +- .../transport/udp/InboundEstablishState2.java | 29 ++++++++- .../udp/OutboundEstablishState2.java | 60 +++++++++++++++---- 3 files changed, 79 insertions(+), 16 deletions(-) diff --git a/router/java/src/net/i2p/router/transport/udp/EstablishmentManager.java b/router/java/src/net/i2p/router/transport/udp/EstablishmentManager.java index 0d79f160a..6399339b5 100644 --- a/router/java/src/net/i2p/router/transport/udp/EstablishmentManager.java +++ b/router/java/src/net/i2p/router/transport/udp/EstablishmentManager.java @@ -767,7 +767,7 @@ class EstablishmentManager { } catch (GeneralSecurityException gse) { if (_log.shouldWarn()) _log.warn("Corrupt Session Confirmed on: " + state, gse); - state.fail(); + // state called fail() return; } InboundEstablishState.InboundState istate = state.getState(); @@ -819,7 +819,7 @@ class EstablishmentManager { } catch (GeneralSecurityException gse) { if (_log.shouldWarn()) _log.warn("Corrupt Session Created on: " + state, gse); - state.fail(); + // state called fail() return; } notifyActivity(); @@ -839,7 +839,7 @@ class EstablishmentManager { } catch (GeneralSecurityException gse) { if (_log.shouldWarn()) _log.warn("Corrupt Retry from: " + state, gse); - state.fail(); + // state called fail() return; } notifyActivity(); diff --git a/router/java/src/net/i2p/router/transport/udp/InboundEstablishState2.java b/router/java/src/net/i2p/router/transport/udp/InboundEstablishState2.java index 9329eec6d..f41b252fe 100644 --- a/router/java/src/net/i2p/router/transport/udp/InboundEstablishState2.java +++ b/router/java/src/net/i2p/router/transport/udp/InboundEstablishState2.java @@ -437,7 +437,19 @@ class InboundEstablishState2 extends InboundEstablishState implements SSU2Payloa ///////////////////////////////////////////////////////// // end payload callbacks ///////////////////////////////////////////////////////// - + + // SSU 1 overrides + + /** + * Overridden to destroy the handshake state + * @since 0.9.56 + */ + @Override + public synchronized void fail() { + _handshakeState.destroy(); + super.fail(); + } + // SSU 1 unsupported things @Override @@ -566,6 +578,21 @@ class InboundEstablishState2 extends InboundEstablishState implements SSU2Payloa * or null if more fragments to go */ public synchronized PeerState2 receiveSessionConfirmed(UDPPacket packet) throws GeneralSecurityException { + try { + return locked_receiveSessionConfirmed(packet); + } catch (GeneralSecurityException gse) { + if (_log.shouldDebug()) + _log.debug("Session confirmed error", gse); + // fail inside synch rather than have Est. Mgr. do it to prevent races + fail(); + throw gse; + } + } + + /** + * @since 0.9.56 + */ + private PeerState2 locked_receiveSessionConfirmed(UDPPacket packet) throws GeneralSecurityException { if (_currentState != InboundState.IB_STATE_CREATED_SENT && _currentState != InboundState.IB_STATE_CONFIRMED_PARTIALLY) throw new GeneralSecurityException("Bad state for Session Confirmed: " + _currentState); diff --git a/router/java/src/net/i2p/router/transport/udp/OutboundEstablishState2.java b/router/java/src/net/i2p/router/transport/udp/OutboundEstablishState2.java index 9746c8c45..ac5da3302 100644 --- a/router/java/src/net/i2p/router/transport/udp/OutboundEstablishState2.java +++ b/router/java/src/net/i2p/router/transport/udp/OutboundEstablishState2.java @@ -359,9 +359,20 @@ class OutboundEstablishState2 extends OutboundEstablishState implements SSU2Payl ///////////////////////////////////////////////////////// // end payload callbacks ///////////////////////////////////////////////////////// - + // SSU 1 overrides + /** + * Overridden to destroy the handshake state + * @since 0.9.56 + */ + @Override + public synchronized void fail() { + if (_handshakeState != null) + _handshakeState.destroy(); + super.fail(); + } + @Override public synchronized boolean validateSessionCreated() { // All validation is in receiveSessionCreated() @@ -424,6 +435,21 @@ class OutboundEstablishState2 extends OutboundEstablishState implements SSU2Payl } public synchronized void receiveRetry(UDPPacket packet) throws GeneralSecurityException { + try { + locked_receiveRetry(packet); + } catch (GeneralSecurityException gse) { + if (_log.shouldDebug()) + _log.debug("Retry error", gse); + // fail inside synch rather than have Est. Mgr. do it to prevent races + fail(); + throw gse; + } + } + + /** + * @since 0.9.56 + */ + private void locked_receiveRetry(UDPPacket packet) throws GeneralSecurityException { ////// TODO state check DatagramPacket pkt = packet.getPacket(); SocketAddress from = pkt.getSocketAddress(); @@ -452,10 +478,6 @@ class OutboundEstablishState2 extends OutboundEstablishState implements SSU2Payl chacha.decryptWithAd(data, off, LONG_HEADER_SIZE, data, off + LONG_HEADER_SIZE, data, off + LONG_HEADER_SIZE, len - LONG_HEADER_SIZE); processPayload(data, off + LONG_HEADER_SIZE, len - (LONG_HEADER_SIZE + MAC_LEN), true); - } catch (GeneralSecurityException gse) { - if (_log.shouldDebug()) - _log.debug("Retry error", gse); - throw gse; } finally { chacha.destroy(); } @@ -477,6 +499,26 @@ class OutboundEstablishState2 extends OutboundEstablishState implements SSU2Payl } public synchronized void receiveSessionCreated(UDPPacket packet) throws GeneralSecurityException { + try { + locked_receiveSessionCreated(packet); + } catch (GeneralSecurityException gse) { + if (_log.shouldDebug()) { + DatagramPacket pkt = packet.getPacket(); + byte data[] = pkt.getData(); + int off = pkt.getOffset(); + int len = pkt.getLength(); + _log.debug("Session create error, State at failure: " + _handshakeState + '\n' + net.i2p.util.HexDump.dump(data, off, len), gse); + } + // fail inside synch rather than have Est. Mgr. do it to prevent races + fail(); + throw gse; + } + } + + /** + * @since 0.9.56 + */ + private void locked_receiveSessionCreated(UDPPacket packet) throws GeneralSecurityException { if (_currentState != OutboundState.OB_STATE_REQUEST_SENT && _currentState != OutboundState.OB_STATE_REQUEST_SENT_NEW_TOKEN) { // ignore dups @@ -506,13 +548,7 @@ class OutboundEstablishState2 extends OutboundEstablishState implements SSU2Payl // _log.debug("State after mixHash 2: " + _handshakeState); // decrypt in-place - try { - _handshakeState.readMessage(data, off + LONG_HEADER_SIZE, len - LONG_HEADER_SIZE, data, off + LONG_HEADER_SIZE); - } catch (GeneralSecurityException gse) { - if (_log.shouldDebug()) - _log.debug("Session create error, State at failure: " + _handshakeState + '\n' + net.i2p.util.HexDump.dump(data, off, len), gse); - throw gse; - } + _handshakeState.readMessage(data, off + LONG_HEADER_SIZE, len - LONG_HEADER_SIZE, data, off + LONG_HEADER_SIZE); //if (_log.shouldDebug()) // _log.debug("State after sess cr: " + _handshakeState); _timeReceived = 0;