From 1572b4fb1b15096780ea1b2e28f06e3d82dce227 Mon Sep 17 00:00:00 2001 From: Jonathan Lennox Date: Tue, 9 Jul 2024 10:42:11 -0400 Subject: [PATCH] Add a config option to redact remote IP addresses in logs. (#283) * Add a config option to redact remote IP addresses in logs. And other human-readable text. * Don't redact relayed candidates; do redact relAddrs. --- src/main/java/org/ice4j/TransportAddress.java | 132 +++++++++++++++++- src/main/java/org/ice4j/ice/Agent.java | 6 +- src/main/java/org/ice4j/ice/Candidate.java | 46 +++++- .../java/org/ice4j/ice/CandidatePair.java | 27 ++++ src/main/java/org/ice4j/ice/CheckList.java | 2 +- src/main/java/org/ice4j/ice/Component.java | 23 ++- .../java/org/ice4j/ice/ComponentSocket.java | 2 +- .../ice4j/ice/ConnectivityCheckClient.java | 30 ++-- .../java/org/ice4j/ice/DefaultNominator.java | 12 +- .../ice4j/socket/MergingDatagramSocket.java | 12 +- src/main/kotlin/org/ice4j/ice/AgentConfig.kt | 6 + src/main/resources/reference.conf | 3 + 12 files changed, 257 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/ice4j/TransportAddress.java b/src/main/java/org/ice4j/TransportAddress.java index f79f53cf..cbe29b44 100644 --- a/src/main/java/org/ice4j/TransportAddress.java +++ b/src/main/java/org/ice4j/TransportAddress.java @@ -147,10 +147,42 @@ public byte[] getAddressBytes() */ public String toString() { - String hostAddress = getHostAddress(); + return toString(false); + } + + public String toRedactedString() + { + return toString(true); + } + + private String toString(boolean redact) + { + String hostAddress; + if (redact) + { + hostAddress = getRedactedAddress(); + if (hostAddress == null) + { + String hostName = getHostName(); + if (hostName != null) + { + /* The transport address is an unresolved hostname. Redact the hostname. */ + hostAddress = "xxxx.xxx"; + } + } + } + else + { + hostAddress = getHostAddress(); + if (hostAddress == null) + { + hostAddress = getHostName(); + } + } if (hostAddress == null) { - hostAddress = getHostName(); + // The address has neither a hostName nor a hostAddress. Shouldn't happen, but don't NPE if it does. */ + hostAddress = "null"; } StringBuilder bldr = new StringBuilder(hostAddress); @@ -181,6 +213,27 @@ public String getHostAddress() return addressStr; } + /* Return the host address, redacted if address redaction is enabled. */ + public String getRedactedAddress() + { + if (AgentConfig.config.getRedactRemoteAddresses()) + { + InetAddress addr = getAddress(); + if (addr != null) + { + return toRedactedString(addr); + } + else + { + return null; + } + } + else + { + return getHostAddress(); + } + } + /** * The transport that this transport address is suggesting. * @@ -288,4 +341,79 @@ public boolean canReach(TransportAddress dst) return true; } + + public static String redact(InetAddress addr) + { + if (AgentConfig.config.getRedactRemoteAddresses()) + { + return toRedactedString(addr); + } + else + { + return addr.getHostAddress(); + } + } + + public static String redact(SocketAddress addr) + { + if (addr instanceof InetSocketAddress && AgentConfig.config.getRedactRemoteAddresses()) + { + InetSocketAddress iaddr = (InetSocketAddress)addr; + return toRedactedString(iaddr.getAddress()) + ":" + iaddr.getPort(); + } + else if (addr == null) + { + return null; + } + else + { + return addr.toString(); + } + } + + /** + * Return a redacted form of an InetAddress, in a form preserving its IP address family + * and (for IPv6) its highest-level bytes. + */ + public static String toRedactedString(InetAddress addr) + { + if (addr == null) + { + return null; + } + if (addr.isAnyLocalAddress() || addr.isLoopbackAddress()) + { + return addr.getHostAddress(); + } + if (addr instanceof Inet6Address) + { + StringBuilder sb = new StringBuilder(); + byte[] addrBytes = addr.getAddress(); + if ((addrBytes[0] & 0xe0) == 0x20) + { + /* Globally-routable IPv6 address; the second nybble can indicate the + * RIR that allocated the address, so don't print it. + */ + sb.append("2xxx"); + } + else if (addrBytes[0] != 0 || addrBytes[1] != 0) + { + /* Other IPv6 address; most common will be fc00:: unique-local and fe80:: link-local address where the + * first 16 bits don't leak anything but the type; all others indicate something unexpected. + */ + sb.append(Integer.toHexString(((addrBytes[0]<<8) & 0xff00) + | (addrBytes[1] & 0xff))); + } + sb.append("::xxx"); + return sb.toString(); + } + else if (addr instanceof Inet4Address) + { + return "xx.xx.xx.xx"; + } + else + { + return addr.getHostAddress(); + } + } } diff --git a/src/main/java/org/ice4j/ice/Agent.java b/src/main/java/org/ice4j/ice/Agent.java index 9ceeab23..fb007042 100644 --- a/src/main/java/org/ice4j/ice/Agent.java +++ b/src/main/java/org/ice4j/ice/Agent.java @@ -1613,7 +1613,7 @@ protected void incomingCheckReceived(TransportAddress remoteAddress, = createCandidatePair(localCandidate, remoteCandidate); logger.debug(() -> "set use-candidate " + useCandidate + " for pair " + - triggeredPair.toShortString()); + triggeredPair.toRedactedShortString()); if (useCandidate) { triggeredPair.setUseCandidateReceived(); @@ -1635,7 +1635,7 @@ else if (state == IceProcessingState.FAILED) else //Running, Connected or Terminated. { logger.debug(() -> "Received check from " - + triggeredPair.toShortString() + " triggered a check."); + + triggeredPair.toRedactedShortString() + " triggered a check."); // We have been started, and have not failed (yet). If this is // a new pair, handle it (even if we have already completed). @@ -1725,7 +1725,7 @@ private void triggerCheck(CandidatePair triggerPair) // if (triggerPair.getParentComponent().getSelectedPair() == null) logger.info("Add peer CandidatePair with new reflexive " + - "address to checkList: " + triggerPair); + "address to checkList: " + triggerPair.toRedactedString()); parentStream.addToCheckList(triggerPair); } diff --git a/src/main/java/org/ice4j/ice/Candidate.java b/src/main/java/org/ice4j/ice/Candidate.java index 7567b00a..d9e78e91 100644 --- a/src/main/java/org/ice4j/ice/Candidate.java +++ b/src/main/java/org/ice4j/ice/Candidate.java @@ -756,7 +756,23 @@ public TransportAddress getRelatedAddress() * @return a String representation of this Candidate. */ @Override - public String toString() + public String toString() { + return toString(false); + } + + /** + * Returns a String representation of this Candidate + * containing its TransportAddress, base, foundation, priority and + * whatever other properties may be relevant, but with its IP address redacted if + * redaction is enabled. + * + * @return a redacted String representation of this Candidate. + */ + public String toRedactedString() { + return toString(true); + } + + private String toString(boolean redacted) { StringBuilder buff = new StringBuilder("candidate:"); @@ -764,7 +780,14 @@ public String toString() buff.append(" ").append(getParentComponent().getComponentID()); buff.append(" ").append(getTransport()); buff.append(" ").append(getPriority()); - buff.append(" ").append(getTransportAddress().getHostAddress()); + if (redacted && getType() != CandidateType.RELAYED_CANDIDATE) + { + buff.append(" ").append(getTransportAddress().getRedactedAddress()); + } + else + { + buff.append(" ").append(getTransportAddress().getHostAddress()); + } buff.append(" ").append(getTransportAddress().getPort()); buff.append(" typ ").append(getType()); @@ -772,7 +795,15 @@ public String toString() if (relAddr != null) { - buff.append(" raddr ").append(relAddr.getHostAddress()); + buff.append(" raddr "); + if (redacted) + { + buff.append(relAddr.getRedactedAddress()); + } + else + { + buff.append(relAddr.getHostAddress()); + } buff.append(" rport ").append(relAddr.getPort()); } @@ -788,6 +819,15 @@ public String toShortString() return getTransportAddress() + "/" + getType(); } + public String toRedactedShortString() + { + if (getType() == CandidateType.RELAYED_CANDIDATE) + { + return toShortString(); + } + return getTransportAddress().toRedactedString() + "/" + getType(); + } + /** * Returns an integer indicating the preference that this Candidate * should be considered with for becoming a default candidate. diff --git a/src/main/java/org/ice4j/ice/CandidatePair.java b/src/main/java/org/ice4j/ice/CandidatePair.java index 208c6539..25c234ec 100644 --- a/src/main/java/org/ice4j/ice/CandidatePair.java +++ b/src/main/java/org/ice4j/ice/CandidatePair.java @@ -484,6 +484,20 @@ public String toString() + "\n\tRemoteCandidate=" + getRemoteCandidate(); } + /** + * Returns a String representation of this CandidatePair, with the remote + * candidate IP address redacted if redaction is enabled. + * + * @return a String representation of the object. + */ + public String toRedactedString() + { + return + "CandidatePair (State=" + getState() + " Priority=" + getPriority() + + "):\n\tLocalCandidate=" + getLocalCandidate() + + "\n\tRemoteCandidate=" + getRemoteCandidate().toRedactedString(); + } + /** * Returns a short String representation of this CandidatePair. * @@ -496,6 +510,19 @@ public String toShortString() + " (" + getParentComponent().toShortString() + ")"; } + /** + * Returns a short String representation of this CandidatePair, + * with the remote candidate IP address redacted if redaction is enabled. + * + * @return a redacted short String representation of the object. + */ + public String toRedactedShortString() + { + return getLocalCandidate().toShortString() + + " -> " + getRemoteCandidate().toRedactedShortString() + + " (" + getParentComponent().toShortString() + ")"; + } + /** * A Comparator using the compareTo method of the * CandidatePair diff --git a/src/main/java/org/ice4j/ice/CheckList.java b/src/main/java/org/ice4j/ice/CheckList.java index bf80b5ab..26071bf2 100644 --- a/src/main/java/org/ice4j/ice/CheckList.java +++ b/src/main/java/org/ice4j/ice/CheckList.java @@ -405,7 +405,7 @@ protected synchronized void handleNominationConfirmed( logger.info( "Selected pair for stream " + cmp.toShortString() + ": " - + nominatedPair.toShortString()); + + nominatedPair.toRedactedShortString()); cmp.setSelectedPair(nominatedPair); diff --git a/src/main/java/org/ice4j/ice/Component.java b/src/main/java/org/ice4j/ice/Component.java index 681a2a93..85a1693d 100644 --- a/src/main/java/org/ice4j/ice/Component.java +++ b/src/main/java/org/ice4j/ice/Component.java @@ -326,7 +326,7 @@ public int getLocalCandidateCount() public void addRemoteCandidate(RemoteCandidate candidate) { logger.info("Add remote candidate for " + toShortString() - + ": " + candidate.toShortString()); + + ": " + candidate.toRedactedShortString()); synchronized(remoteCandidates) { @@ -471,15 +471,15 @@ public void updateRemoteCandidates() if (existingPair != null) { logger.info("existing Pair updated: " + - existingPair.toShortString() + - " to " + pair.toShortString() + "."); + existingPair.toRedactedShortString() + + " to " + pair.toRedactedShortString() + "."); existingPair.setRemoteCandidate(pair.getRemoteCandidate()); existingPair.computePriority(); } else { streamCheckList.add(pair); - logger.info("new Pair added: " + pair.toShortString() + logger.info("new Pair added: " + pair.toRedactedShortString() + "."); } } @@ -599,13 +599,22 @@ public String toString() buff.append("\n") .append(remoteCandidatesCount) .append(" Remote candidates:"); - buff.append("\ndefault remote candidate: ") - .append(getDefaultRemoteCandidate()); + Candidate defaultRemoteCandidate = getDefaultRemoteCandidate(); + buff.append("\ndefault remote candidate: "); + if (defaultRemoteCandidate != null) + { + buff.append(defaultRemoteCandidate.toRedactedString()); + } + else + { + buff.append("null"); + } + synchronized(remoteCandidates) { for (RemoteCandidate cand : remoteCandidates) { - buff.append("\n").append(cand); + buff.append("\n").append(cand.toRedactedString()); } } } diff --git a/src/main/java/org/ice4j/ice/ComponentSocket.java b/src/main/java/org/ice4j/ice/ComponentSocket.java index 2befe0b9..287d2fc5 100644 --- a/src/main/java/org/ice4j/ice/ComponentSocket.java +++ b/src/main/java/org/ice4j/ice/ComponentSocket.java @@ -96,7 +96,7 @@ private void addAuthorizedAddress(SocketAddress address) return; } - logger.info("Adding allowed address: " + address); + logger.info("Adding allowed address: " + TransportAddress.redact(address)); Set newSet = new HashSet<>(); newSet.addAll(authorizedAddresses); diff --git a/src/main/java/org/ice4j/ice/ConnectivityCheckClient.java b/src/main/java/org/ice4j/ice/ConnectivityCheckClient.java index 983a72ba..6892c144 100644 --- a/src/main/java/org/ice4j/ice/ConnectivityCheckClient.java +++ b/src/main/java/org/ice4j/ice/ConnectivityCheckClient.java @@ -279,7 +279,7 @@ protected TransactionID startCheckForPair( { logger.debug(() -> "Add USE-CANDIDATE in check for: " - + candidatePair.toShortString()); + + candidatePair.toRedactedShortString()); request.putAttribute( AttributeFactory.createUseCandidateAttribute()); } @@ -324,7 +324,7 @@ protected TransactionID startCheckForPair( if (logger.isDebugEnabled()) { logger.debug( - "start check for " + candidatePair.toShortString() + " tid " + "start check for " + candidatePair.toRedactedShortString() + " tid " + tran); } try @@ -342,7 +342,7 @@ protected TransactionID startCheckForPair( maxRetransmissions); if (logger.isTraceEnabled()) { - logger.trace("checking pair " + candidatePair + " tid " + tran); + logger.trace("checking pair " + candidatePair.toRedactedString() + " tid " + tran); } } catch (NetAccessManager.SocketNotFoundException e) @@ -399,7 +399,7 @@ public void processResponse(StunResponseEvent ev) if (!checkSymmetricAddresses(ev)) { logger.info("Received a non-symmetric response for pair: " - + checkedPair.toShortString() + ". Failing."); + + checkedPair.toRedactedShortString() + ". Failing."); checkedPair.setStateFailed(); } else @@ -536,7 +536,7 @@ private void processSuccessResponse(StunResponseEvent ev) logger.debug(() -> "Received a success response with no " + "XOR_MAPPED_ADDRESS attribute."); logger.info("Pair failed (no XOR-MAPPED-ADDRESS): " - + checkedPair.toShortString() + "."); + + checkedPair.toRedactedShortString() + "."); checkedPair.setStateFailed(); return; //malformed error response } @@ -608,7 +608,7 @@ private void processSuccessResponse(StunResponseEvent ev) if (checkedPair.getParentComponent().getSelectedPair() == null) { logger.info("Receive a peer-reflexive candidate: " - + peerReflexiveCandidate.getTransportAddress() + + peerReflexiveCandidate.getTransportAddress().toRedactedString() + "."); } } @@ -646,7 +646,7 @@ private void processSuccessResponse(StunResponseEvent ev) //different than the valid pair constructed above if (checkedPair.getParentComponent().getSelectedPair() == null) { - logger.info("Pair succeeded: " + checkedPair.toShortString() + logger.info("Pair succeeded: " + checkedPair.toRedactedShortString() + "."); } checkedPair.setStateSucceeded(); @@ -655,7 +655,7 @@ private void processSuccessResponse(StunResponseEvent ev) if (!validPair.isValid()) { if (validPair.getParentComponent().getSelectedPair() == null) - logger.info("Pair validated: " + validPair.toShortString() + logger.info("Pair validated: " + validPair.toRedactedShortString() + "."); parentAgent.validatePair(validPair); } @@ -740,14 +740,14 @@ private void processSuccessResponse(StunResponseEvent ev) if (validPair.getParentComponent().getSelectedPair() == null) { logger.info("Nomination confirmed for pair: " - + validPair.toShortString() + + validPair.toRedactedShortString() + "."); parentAgent.nominationConfirmed( validPair ); } else { logger.debug(() -> - "Keep alive for pair: " + validPair.toShortString()); + "Keep alive for pair: " + validPair.toRedactedShortString()); } } //If the agent is the controlled agent, the response may be the result @@ -763,13 +763,13 @@ else if (!parentAgent.isControlling() { logger.info( "Nomination confirmed for pair: " - + validPair.toShortString()); + + validPair.toRedactedShortString()); parentAgent.nominationConfirmed(checkedPair); } else { logger.debug(() -> - "Keep alive for pair: " + validPair.toShortString()); + "Keep alive for pair: " + validPair.toRedactedShortString()); } } @@ -858,7 +858,7 @@ private void processErrorResponse(StunResponseEvent ev) String reason = errorAttr.getReasonPhrase(); String trimmedReason = reason != null ? reason.trim() : null; logger.info( - "Error response for pair: " + pair.toShortString() + + "Error response for pair: " + pair.toRedactedShortString() + ", failing. Code = " + code + "(class=" + cl + "; number=" + co + "): " + trimmedReason); pair.setStateFailed(); @@ -878,7 +878,7 @@ public void processTimeout(StunTimeoutEvent ev) CandidatePair pair = (CandidatePair) ev.getTransactionID() .getApplicationData(); - logger.info("timeout for pair: " + pair.toShortString() + ", failing."); + logger.info("timeout for pair: " + pair.toRedactedShortString() + ", failing."); pair.setStateFailed(); updateCheckListAndTimerStates(pair); } @@ -942,7 +942,7 @@ protected void run() { logger.info( "Pair failed: " - + pairToCheck.toShortString()); + + pairToCheck.toRedactedShortString()); pairToCheck.setStateFailed(); } else diff --git a/src/main/java/org/ice4j/ice/DefaultNominator.java b/src/main/java/org/ice4j/ice/DefaultNominator.java index 449cd756..2dc8a552 100644 --- a/src/main/java/org/ice4j/ice/DefaultNominator.java +++ b/src/main/java/org/ice4j/ice/DefaultNominator.java @@ -112,7 +112,7 @@ public void propertyChange(PropertyChangeEvent ev) if (parentStream.validListContainsNomineeForComponent(parentComponent)) { logger.debug(() -> - "Keep-alive for pair: " + validPair.toShortString()); + "Keep-alive for pair: " + validPair.toRedactedShortString()); return; } } @@ -140,7 +140,7 @@ private void strategyNominateFirstValid(PropertyChangeEvent evt) { CandidatePair validPair = (CandidatePair)evt.getSource(); - logger.info("Nominate (first valid): " + validPair.toShortString() + logger.info("Nominate (first valid): " + validPair.toRedactedShortString() + "."); parentAgent.nominate(validPair); } @@ -178,7 +178,7 @@ private void strategyNominateHighestPrio(PropertyChangeEvent ev) { logger.info( "Nominate (highest priority): " - + validPair.toShortString()); + + validPair.toRedactedShortString()); parentAgent.nominate(pair); } } @@ -267,7 +267,7 @@ else if (!isRelayed) logger.info( "Nominate (first highest valid): " - + validPair.toShortString()); + + validPair.toRedactedShortString()); nominate = true; } } @@ -354,7 +354,7 @@ public void propertyChange(PropertyChangeEvent evt) logger.info( "Nominate (first highest valid): " - + pair.toShortString()); + + pair.toRedactedShortString()); parentAgent.nominate(pair); } } @@ -393,7 +393,7 @@ public void run() return; logger.info( - "Nominate (first highest valid): " + pair.toShortString()); + "Nominate (first highest valid): " + pair.toRedactedShortString()); // task has not been cancelled after WAIT_TIME milliseconds so // nominate the pair diff --git a/src/main/java/org/ice4j/socket/MergingDatagramSocket.java b/src/main/java/org/ice4j/socket/MergingDatagramSocket.java index ed38629a..9077a011 100644 --- a/src/main/java/org/ice4j/socket/MergingDatagramSocket.java +++ b/src/main/java/org/ice4j/socket/MergingDatagramSocket.java @@ -525,7 +525,7 @@ public void receive(DatagramPacket p) { logger.info("Discarded " + numDiscardedPackets + " packets. Last remote address:" - + p.getSocketAddress()); + + TransportAddress.redact(p.getSocketAddress())); } // Go on and receive the next packet in p. @@ -583,7 +583,7 @@ protected void initializeActive(IceSocketWrapper socketWrapper, if (logger.isDebugEnabled()) { logger.debug("Initializing the active container, socket=" + socket - + "; remote address=" + remoteAddress); + + "; remote address=" + TransportAddress.redact(remoteAddress)); } synchronized (socketContainersSyncRoot) @@ -725,11 +725,11 @@ public void run() thread.setDaemon(true); thread.setName("MergingDatagramSocket reader thread for: " + getLocalSocketAddress() + " -> " - + getRemoteSocketAddress()); + + TransportAddress.redact(getRemoteSocketAddress())); logger.debug(() -> "Starting the thread for socket " + getLocalSocketAddress() + " -> " - + getRemoteSocketAddress()); + + TransportAddress.redact(getRemoteSocketAddress())); thread.start(); } @@ -950,12 +950,12 @@ public String toString() if (datagramSocket != null) { return datagramSocket.getLocalSocketAddress() - + " -> " + remoteAddress; + + " -> " + TransportAddress.redact(remoteAddress); } else { return delegatingSocket.getLocalSocketAddress() - + " -> " + remoteAddress; + + " -> " + TransportAddress.redact(remoteAddress); } } diff --git a/src/main/kotlin/org/ice4j/ice/AgentConfig.kt b/src/main/kotlin/org/ice4j/ice/AgentConfig.kt index bb2e835e..3b95d0e8 100644 --- a/src/main/kotlin/org/ice4j/ice/AgentConfig.kt +++ b/src/main/kotlin/org/ice4j/ice/AgentConfig.kt @@ -65,6 +65,12 @@ class AgentConfig { "ice4j.software".from(configSource) } + /** Whether remote IP addresses should be redacted in logs */ + val redactRemoteAddresses: Boolean by config { + "org.ice4j.REDACT_REMOTE_ADDRESSES".from(configSource) + "ice4j.redact-remote-addresses".from(configSource) + } + /** * Whether the per-component merging socket should be enabled by default (the default value can be * overridden with the [Agent] API). diff --git a/src/main/resources/reference.conf b/src/main/resources/reference.conf index b6de64ac..09955622 100644 --- a/src/main/resources/reference.conf +++ b/src/main/resources/reference.conf @@ -16,6 +16,9 @@ ice4j { // the socket instance from the desired [CandidatePair] must be used. use-component-socket = true + // Whether remote IP addresses should be redacted in log messages + redact-remote-addresses = false + consent-freshness { // How often a STUN Binding request used for consent freshness check will be sent. interval = 15 seconds