From 87715450ee20e5ba34a4883ab7b5bf3ff1fcbdcc Mon Sep 17 00:00:00 2001 From: Mohit Date: Wed, 14 Oct 2026 21:30:22 +0530 Subject: [PATCH] Remove deprecated BlowfishTupleSerializer (#8994) --- docs/SECURITY.md | 2 +- .../BlowfishTupleSerializer.java | 106 ------------------ .../BlowfishTupleSerializerTest.java | 100 ----------------- .../SerializationFactoryTest.java | 20 +++- .../apache/storm/daemon/nimbus/Nimbus.java | 13 ++- .../nimbus/NimbusGetTopologyConfTest.java | 7 +- .../nimbus/NimbusGetTopologyPageInfoTest.java | 5 +- 7 files changed, 28 insertions(+), 225 deletions(-) delete mode 100644 storm-client/src/jvm/org/apache/storm/security/serialization/BlowfishTupleSerializer.java delete mode 100644 storm-client/test/jvm/org/apache/storm/security/serialization/BlowfishTupleSerializerTest.java diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 36154271f13..498650df1d8 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -677,7 +677,7 @@ The size limits need a caveat. `maxbytes` counts stream bytes per object, not pe An invalid pattern is rejected when the topology is submitted, and otherwise fails worker startup. -For tuple encryption, use TLS-based transport encryption (`storm.messaging.netty.tls.enable`) instead of the deprecated `BlowfishTupleSerializer`, which uses a 64-bit block cipher vulnerable to birthday attacks. +For tuple encryption, use TLS-based transport encryption (`storm.messaging.netty.tls.enable`). The legacy `BlowfishTupleSerializer` has been removed because it uses a 64-bit block cipher vulnerable to birthday attacks. ### Log Cleanup The Logviewer daemon now is also responsible for cleaning up old log files for dead topologies. diff --git a/storm-client/src/jvm/org/apache/storm/security/serialization/BlowfishTupleSerializer.java b/storm-client/src/jvm/org/apache/storm/security/serialization/BlowfishTupleSerializer.java deleted file mode 100644 index 185817b8225..00000000000 --- a/storm-client/src/jvm/org/apache/storm/security/serialization/BlowfishTupleSerializer.java +++ /dev/null @@ -1,106 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one or more contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. The ASF licenses this file to you under the Apache License, Version - * 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions - * and limitations under the License. - */ - -package org.apache.storm.security.serialization; - -import com.esotericsoftware.kryo.Kryo; -import com.esotericsoftware.kryo.Serializer; -import com.esotericsoftware.kryo.io.Input; -import com.esotericsoftware.kryo.io.Output; -import com.esotericsoftware.kryo.serializers.BlowfishSerializer; -import java.util.Map; -import javax.crypto.KeyGenerator; -import javax.crypto.SecretKey; -import org.apache.storm.Config; -import org.apache.storm.serialization.types.ListDelegateSerializer; -import org.apache.storm.shade.org.apache.commons.codec.DecoderException; -import org.apache.storm.shade.org.apache.commons.codec.binary.Hex; -import org.apache.storm.utils.ListDelegate; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Apply Blowfish encryption for tuple communication to bolts. - * - * @deprecated since 2.8.6. Blowfish uses a 64-bit block size which is vulnerable to birthday attacks (Sweet32). - * Use TLS-based transport encryption instead (see storm.messaging.netty.tls.enable). - */ -@Deprecated(since = "2.8.6", forRemoval = true) -public class BlowfishTupleSerializer extends Serializer { - /** - * The secret key (if any) for data encryption by blowfish payload serialization factory (BlowfishSerializationFactory). You should use - * in via: - * - *

```storm -c topology.tuple.serializer.blowfish.key=YOURKEY -c topology.tuple.serializer=org.apache.storm.security.serialization - * .BlowfishTupleSerializer - * jar ...``` - */ - public static final String SECRET_KEY = "topology.tuple.serializer.blowfish.key"; - private static final Logger LOG = LoggerFactory.getLogger(BlowfishTupleSerializer.class); - private BlowfishSerializer serializer; - - public BlowfishTupleSerializer(Kryo unused, Map topoConf) { - String encryptionkey; - try { - encryptionkey = (String) topoConf.get(SECRET_KEY); - LOG.warn("BlowfishTupleSerializer is deprecated and will be removed in a future release. " - + "Blowfish uses a 64-bit block size which is vulnerable to birthday attacks (Sweet32). " - + "Use TLS-based transport encryption instead (storm.messaging.netty.tls.enable)."); - - byte[] bytes; - if (encryptionkey != null) { - bytes = Hex.decodeHex(encryptionkey.toCharArray()); - } else { - // try to use zookeeper secret - String payload = (String) topoConf.get(Config.STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD); - if (payload != null) { - LOG.debug("{} is not present. Use {} as Blowfish encryption key", SECRET_KEY, - Config.STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD); - bytes = payload.getBytes(); - } else { - throw new RuntimeException("Blowfish encryption key not specified"); - } - } - serializer = new BlowfishSerializer(new ListDelegateSerializer(), bytes); - } catch (DecoderException ex) { - throw new RuntimeException("Blowfish encryption key invalid", ex); - } - } - - /** - * Produce a blowfish key to be used in "Storm jar" command. - */ - public static void main(String[] args) { - try { - KeyGenerator kgen = KeyGenerator.getInstance("Blowfish"); - kgen.init(256); - SecretKey skey = kgen.generateKey(); - byte[] raw = skey.getEncoded(); - String keyString = new String(Hex.encodeHex(raw)); - System.out.println("storm -c " + SECRET_KEY - + "=" + keyString + " -c " + Config.TOPOLOGY_TUPLE_SERIALIZER - + "=" + BlowfishTupleSerializer.class.getName() + " ..."); - } catch (Exception ex) { - LOG.error(ex.getMessage()); - } - } - - @Override - public void write(Kryo kryo, Output output, ListDelegate object) { - kryo.writeObject(output, object, serializer); - } - - @Override - public ListDelegate read(Kryo kryo, Input input, Class type) { - return kryo.readObject(input, ListDelegate.class, serializer); - } -} diff --git a/storm-client/test/jvm/org/apache/storm/security/serialization/BlowfishTupleSerializerTest.java b/storm-client/test/jvm/org/apache/storm/security/serialization/BlowfishTupleSerializerTest.java deleted file mode 100644 index 8ab530f0d98..00000000000 --- a/storm-client/test/jvm/org/apache/storm/security/serialization/BlowfishTupleSerializerTest.java +++ /dev/null @@ -1,100 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one or more contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. The ASF licenses this file to you under the Apache License, Version - * 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions - * and limitations under the License. - */ - -package org.apache.storm.security.serialization; - -import com.esotericsoftware.kryo.Kryo; -import com.esotericsoftware.kryo.io.Input; -import com.esotericsoftware.kryo.io.Output; -import java.util.Arrays; -import java.util.HashMap; -import java.util.Map; -import org.apache.storm.Config; -import org.apache.storm.shade.com.google.common.base.Joiner; -import org.apache.storm.shade.com.google.common.collect.ImmutableMap; -import org.apache.storm.utils.ListDelegate; -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; - -public class BlowfishTupleSerializerTest { - - /** - * Throws RuntimeException when no encryption key is given. - */ - @Test - public void testConstructorThrowsOnNullKey() { - assertThrows(RuntimeException.class, () -> new BlowfishTupleSerializer(null, new HashMap<>())); - } - - /** - * Throws RuntimeException when an invalid encryption key is given. - */ - @Test - public void testConstructorThrowsOnInvalidKey() { - // The encryption key must be hexadecimal. - assertThrows(RuntimeException.class, - () -> new BlowfishTupleSerializer(null, ImmutableMap.of(BlowfishTupleSerializer.SECRET_KEY, "0123456789abcdefg"))); - } - - /** - * Test using {@link org.apache.storm.security.serialization.BlowfishTupleSerializer#SECRET_KEY}. - */ - @Test - public void testUseBlowfishKey() { - String arbitraryKey = "7dd6fb3203878381b08f9c89d25ed105"; - Map topoConf = ImmutableMap.of(BlowfishTupleSerializer.SECRET_KEY, arbitraryKey); - testEncryptsAndDecryptsMessage(topoConf); - } - - /** - * Test using {@link org.apache.storm.Config#STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD} - * when {@link org.apache.storm.security.serialization.BlowfishTupleSerializer#SECRET_KEY} is not present. - */ - @Test - public void testUseZookeeperSecret() { - Map topoConf = ImmutableMap.of(Config.STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD, "user:password"); - testEncryptsAndDecryptsMessage(topoConf); - } - - /** - * Reads a string encrypted by another instance with a shared key - */ - private void testEncryptsAndDecryptsMessage(Map topoConf) { - String testText = "Tetraodontidae is a family of primarily marine and estuarine fish of the order" + - " Tetraodontiformes. The family includes many familiar species, which are" + - " variously called pufferfish, puffers, balloonfish, blowfish, bubblefish," + - " globefish, swellfish, toadfish, toadies, honey toads, sugar toads, and sea" + - " squab.[1] They are morphologically similar to the closely related" + - " porcupinefish, which have large external spines (unlike the thinner, hidden" + - " spines of Tetraodontidae, which are only visible when the fish has puffed up)." + - " The scientific name refers to the four large teeth, fused into an upper and" + - " lower plate, which are used for crushing the shells of crustaceans and" + - " mollusks, their natural prey."; - Kryo kryo = new Kryo(); - kryo.setRegistrationRequired(false); - BlowfishTupleSerializer writerBTS = new BlowfishTupleSerializer(null, topoConf); - BlowfishTupleSerializer readerBTS = new BlowfishTupleSerializer(null, topoConf); - int bufferSize = 1024; - Output output = new Output(bufferSize, bufferSize); - Input input = new Input(bufferSize); - String[] stringList = testText.split(" "); - ListDelegate delegate = new ListDelegate(); - delegate.addAll(Arrays.asList(stringList)); - - writerBTS.write(kryo, output, delegate); - input.setBuffer(output.getBuffer()); - ListDelegate outDelegate = readerBTS.read(kryo, input, ListDelegate.class); - assertEquals(testText, Joiner.on(" ").join(outDelegate.toArray())); - } -} diff --git a/storm-client/test/jvm/org/apache/storm/serialization/SerializationFactoryTest.java b/storm-client/test/jvm/org/apache/storm/serialization/SerializationFactoryTest.java index 998098643cc..a5667b1626d 100644 --- a/storm-client/test/jvm/org/apache/storm/serialization/SerializationFactoryTest.java +++ b/storm-client/test/jvm/org/apache/storm/serialization/SerializationFactoryTest.java @@ -13,9 +13,11 @@ package org.apache.storm.serialization; import com.esotericsoftware.kryo.Kryo; +import com.esotericsoftware.kryo.Serializer; +import com.esotericsoftware.kryo.io.Input; +import com.esotericsoftware.kryo.io.Output; import java.util.Map; import org.apache.storm.Config; -import org.apache.storm.security.serialization.BlowfishTupleSerializer; import org.apache.storm.testing.TestWordSpout; import org.apache.storm.topology.TopologyBuilder; import org.apache.storm.utils.ListDelegate; @@ -48,14 +50,11 @@ public void test_throws_runtimeexception_when_no_such_class() { @Test public void test_registers_when_valid_class_name() { - Class arbitraryClass = BlowfishTupleSerializer.class; - String secretKey = "0123456789abcdef"; + Class arbitraryClass = MockTupleSerializer.class; Map conf = Utils.readDefaultConfig(); conf.put(Config.TOPOLOGY_TUPLE_SERIALIZER, arbitraryClass.getName()); - conf.put(BlowfishTupleSerializer.SECRET_KEY, secretKey); Kryo kryo = SerializationFactory.getKryo(conf); assertEquals(arbitraryClass, kryo.getSerializer(ListDelegate.class).getClass()); - } @Test @@ -89,4 +88,15 @@ public void decorate(Kryo k, Map conf) { public static class MockSerObject { } + public static class MockTupleSerializer extends Serializer { + @Override + public void write(Kryo kryo, Output output, ListDelegate object) { + } + + @Override + public ListDelegate read(Kryo kryo, Input input, Class type) { + return null; + } + } + } diff --git a/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java b/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java index 9e184feb695..9a7d6bedaf1 100644 --- a/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java +++ b/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java @@ -200,7 +200,6 @@ import org.apache.storm.security.auth.ThriftConnectionType; import org.apache.storm.security.auth.ThriftServer; import org.apache.storm.security.auth.workertoken.WorkerTokenManager; -import org.apache.storm.security.serialization.BlowfishTupleSerializer; import org.apache.storm.shade.com.google.common.annotations.VisibleForTesting; import org.apache.storm.shade.com.google.common.base.Strings; import org.apache.storm.shade.com.google.common.collect.ImmutableMap; @@ -806,20 +805,22 @@ private static StormTopology readStormTopologyAsNimbus(String topoId, TopoCache return tc.readTopology(topoId, NIMBUS_SUBJECT); } + @VisibleForTesting + static final String LEGACY_BLOWFISH_SECRET_KEY = "topology.tuple.serializer.blowfish.key"; + /** * Mask the credential values in a config map that is about to be serialized to a client. Nimbus serves config over * several read-only operations, and a caller authorized for those is not necessarily authorized to hold the cluster's - * or the topology's secrets. This covers what {@link ConfigUtils#maskCredentials(Map)} covers, plus the Blowfish - * tuple-serializer key, whose constant lives outside the config classes so the annotation scan cannot see it and - * whose name matches no credential pattern. + * or the topology's secrets. This covers what {@link ConfigUtils#maskCredentials(Map)} covers, plus the legacy Blowfish + * tuple-serializer key, so that legacy stored configs do not leak the key. * * @param conf the config about to be served * @return a copy of the config with credential values replaced */ private static Map maskCredentialsForApi(Map conf) { Map masked = new HashMap<>(ConfigUtils.maskCredentials(conf)); - if (masked.get(BlowfishTupleSerializer.SECRET_KEY) instanceof String) { - masked.put(BlowfishTupleSerializer.SECRET_KEY, "*****"); + if (masked.get(LEGACY_BLOWFISH_SECRET_KEY) instanceof String) { + masked.put(LEGACY_BLOWFISH_SECRET_KEY, "*****"); } return masked; } diff --git a/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusGetTopologyConfTest.java b/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusGetTopologyConfTest.java index fae4ad566b3..0121527add2 100644 --- a/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusGetTopologyConfTest.java +++ b/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusGetTopologyConfTest.java @@ -29,7 +29,6 @@ import org.apache.storm.cluster.IStormClusterState; import net.minidev.json.JSONValue; -import org.apache.storm.security.serialization.BlowfishTupleSerializer; import org.junit.jupiter.api.Test; import org.mockito.ArgumentMatchers; import org.mockito.Mockito; @@ -56,7 +55,7 @@ public void getTopologyConfMasksCredentialsAndKeepsOtherValues() throws Exceptio Map storedConf = new HashMap<>(); storedConf.put(Config.TOPOLOGY_NAME, TOPO_NAME); storedConf.put(Config.STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD, "topology-zk-secret"); - storedConf.put(BlowfishTupleSerializer.SECRET_KEY, "0123456789abcdef"); + storedConf.put(Nimbus.LEGACY_BLOWFISH_SECRET_KEY, "0123456789abcdef"); storedConf.put(Config.TOPOLOGY_WORKERS, 3); try (LocalCluster cluster = new LocalCluster.Builder() @@ -77,7 +76,7 @@ public void getTopologyConfMasksCredentialsAndKeepsOtherValues() throws Exceptio assertEquals(MASKED, served.get(Config.STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD), "the ZooKeeper auth payload should be masked"); - assertEquals(MASKED, served.get(BlowfishTupleSerializer.SECRET_KEY), + assertEquals(MASKED, served.get(Nimbus.LEGACY_BLOWFISH_SECRET_KEY), "the tuple serializer key should be masked"); assertEquals(TOPO_NAME, served.get(Config.TOPOLOGY_NAME)); @@ -85,7 +84,7 @@ public void getTopologyConfMasksCredentialsAndKeepsOtherValues() throws Exceptio assertEquals("topology-zk-secret", storedConf.get(Config.STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD), "the stored conf should keep its own values"); - assertEquals("0123456789abcdef", storedConf.get(BlowfishTupleSerializer.SECRET_KEY), + assertEquals("0123456789abcdef", storedConf.get(Nimbus.LEGACY_BLOWFISH_SECRET_KEY), "the stored conf should keep its own values"); } } diff --git a/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusGetTopologyPageInfoTest.java b/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusGetTopologyPageInfoTest.java index c9bf4d2c5e4..700bca0bda6 100644 --- a/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusGetTopologyPageInfoTest.java +++ b/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusGetTopologyPageInfoTest.java @@ -33,7 +33,6 @@ import org.apache.storm.generated.TopologyPageInfo; import org.apache.storm.generated.TopologyStatus; import org.apache.storm.scheduler.resource.TestUtilsForResourceAwareScheduler.TestSpout; -import org.apache.storm.security.serialization.BlowfishTupleSerializer; import org.apache.storm.topology.TopologyBuilder; import org.junit.jupiter.api.Test; import org.mockito.ArgumentMatchers; @@ -76,7 +75,7 @@ private static Map storedTopoConf() { topoConf.put(Config.TOPOLOGY_EVENTLOGGER_EXECUTORS, 0); topoConf.put(Config.TOPOLOGY_MESSAGE_TIMEOUT_SECS, 30); topoConf.put(Config.STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD, "topology-zk-secret"); - topoConf.put(BlowfishTupleSerializer.SECRET_KEY, "0123456789abcdef"); + topoConf.put(Nimbus.LEGACY_BLOWFISH_SECRET_KEY, "0123456789abcdef"); return topoConf; } @@ -137,7 +136,7 @@ public void getTopologyPageInfoMasksDaemonAndTopologyCredentials() throws Except // topology-side credentials, masked on getTopologyConf and equally reachable here assertEquals(MASKED, served.get(Config.STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD), "the topology ZooKeeper auth payload should be masked"); - assertEquals(MASKED, served.get(BlowfishTupleSerializer.SECRET_KEY), + assertEquals(MASKED, served.get(Nimbus.LEGACY_BLOWFISH_SECRET_KEY), "the tuple serializer key should be masked"); // values that carry no credential are served untouched