diff --git a/activemq-client/src/main/java/org/apache/activemq/ActiveMQConnection.java b/activemq-client/src/main/java/org/apache/activemq/ActiveMQConnection.java index 900938ec04c..71b5192fa09 100644 --- a/activemq-client/src/main/java/org/apache/activemq/ActiveMQConnection.java +++ b/activemq-client/src/main/java/org/apache/activemq/ActiveMQConnection.java @@ -147,6 +147,8 @@ public class ActiveMQConnection implements Connection, TopicConnection, QueueCon * This strictly rejects non-standard property types such as Character, Map, and List. */ private boolean strictCompliance = false; + // set when the client identifier came from the connection factory configuration + private boolean adminConfiguredClientID = false; private boolean disableTimeStampsByDefault; private boolean optimizedMessageDispatch = true; @@ -463,6 +465,12 @@ public void setClientID(String newClientID) throws JMSException { throw new IllegalStateException("The clientID has already been set"); } + // The specification forbids overriding an administratively configured client + // identifier; ActiveMQ has always allowed it, so enforce under strict compliance. + if (this.adminConfiguredClientID && this.strictCompliance) { + throw new IllegalStateException("The clientID was administratively configured and cannot be changed"); + } + if (this.isConnectionInfoSentToBroker) { throw new IllegalStateException("Setting clientID on a used Connection is not allowed"); } @@ -479,6 +487,7 @@ public void setClientID(String newClientID) throws JMSException { public void setDefaultClientID(String clientID) throws JMSException { this.info.setClientId(clientID); this.userSpecifiedClientID = true; + this.adminConfiguredClientID = true; } /** @@ -1566,6 +1575,21 @@ protected synchronized void checkClosed() throws JMSException { } } + /** + * Validates this connection's credentials with the broker using a throwaway + * ConnectionInfo that is removed again immediately. The connection's own + * ConnectionInfo is left unsent, so setClientID() remains possible afterwards + * as the specification requires. Used by the factory under strictCompliance + * so createConnection fails fast with JMSSecurityException on bad credentials. + */ + protected void authenticate() throws JMSException { + var probe = info.copy(); + probe.setConnectionId(new ConnectionId(info.getConnectionId().getValue() + ":auth")); + probe.setClientId(clientIdGenerator.generateId()); + syncSendPacket(probe, getConnectResponseTimeout()); + asyncSendPacket(probe.createRemoveCommand()); + } + /** * Send the ConnectionInfo to the Broker * diff --git a/activemq-client/src/main/java/org/apache/activemq/ActiveMQConnectionFactory.java b/activemq-client/src/main/java/org/apache/activemq/ActiveMQConnectionFactory.java index 93cfb20d309..2b1f9ac4c09 100644 --- a/activemq-client/src/main/java/org/apache/activemq/ActiveMQConnectionFactory.java +++ b/activemq-client/src/main/java/org/apache/activemq/ActiveMQConnectionFactory.java @@ -308,6 +308,7 @@ public JMSContext createContext(String userName, String password) { */ @Override public JMSContext createContext(String userName, String password, int sessionMode) { + ActiveMQSession.validateSessionMode(sessionMode); try { return new ActiveMQContext(createActiveMQConnection(userName, password), sessionMode); } catch (JMSException e) { @@ -320,6 +321,7 @@ public JMSContext createContext(String userName, String password, int sessionMod */ @Override public JMSContext createContext(int sessionMode) { + ActiveMQSession.validateSessionMode(sessionMode); try { return new ActiveMQContext(createActiveMQConnection(getUserName(), getPassword()), sessionMode); } catch (JMSException e) { @@ -398,6 +400,15 @@ protected ActiveMQConnection createActiveMQConnection(String userName, String pa connection.setDefaultClientID(clientID); } + // Jakarta Messaging expects createConnection/createContext to authenticate + // the caller immediately (JMSSecurityException on bad credentials). ActiveMQ + // historically defers the ConnectionInfo exchange until first use, so the + // eager check is only performed under strictCompliance, and it uses a probe + // so the connection's own identity stays unset for a later setClientID(). + if (isStrictCompliance()) { + connection.authenticate(); + } + return connection; } catch (JMSException e) { // Clean up! diff --git a/activemq-client/src/main/java/org/apache/activemq/ActiveMQContext.java b/activemq-client/src/main/java/org/apache/activemq/ActiveMQContext.java index 72ef853313b..0f9bf543a31 100644 --- a/activemq-client/src/main/java/org/apache/activemq/ActiveMQContext.java +++ b/activemq-client/src/main/java/org/apache/activemq/ActiveMQContext.java @@ -97,6 +97,7 @@ public JMSContext createContext(int sessionMode) { if(connectionCounter.get() == 0l) { throw new JMSRuntimeException("Context already closed"); } + ActiveMQSession.validateSessionMode(sessionMode); connectionCounter.incrementAndGet(); return new ActiveMQContext(activemqConnection, sessionMode, connectionCounter); diff --git a/activemq-client/src/main/java/org/apache/activemq/ActiveMQSession.java b/activemq-client/src/main/java/org/apache/activemq/ActiveMQSession.java index dc206850415..ae5369821bd 100644 --- a/activemq-client/src/main/java/org/apache/activemq/ActiveMQSession.java +++ b/activemq-client/src/main/java/org/apache/activemq/ActiveMQSession.java @@ -36,6 +36,7 @@ import jakarta.jms.InvalidDestinationException; import jakarta.jms.InvalidSelectorException; import jakarta.jms.JMSException; +import jakarta.jms.JMSRuntimeException; import jakarta.jms.MapMessage; import jakarta.jms.Message; import jakarta.jms.MessageConsumer; @@ -197,6 +198,17 @@ public class ActiveMQSession implements Session, QueueSession, TopicSession, Sta public static final int INDIVIDUAL_ACKNOWLEDGE = 4; public static final int MAX_ACK_CONSTANT = INDIVIDUAL_ACKNOWLEDGE; + /** + * Rejects a JMSContext session mode outside the four Jakarta Messaging modes + * and ActiveMQ's INDIVIDUAL_ACKNOWLEDGE extension, as the specification + * requires a JMSRuntimeException for an invalid mode. + */ + static void validateSessionMode(int sessionMode) { + if (sessionMode < Session.SESSION_TRANSACTED || sessionMode > MAX_ACK_CONSTANT) { + throw new JMSRuntimeException("Invalid session mode: " + sessionMode); + } + } + public static interface DeliveryListener { void beforeDelivery(ActiveMQSession session, Message msg); diff --git a/activemq-client/src/main/java/org/apache/activemq/command/ActiveMQBytesMessage.java b/activemq-client/src/main/java/org/apache/activemq/command/ActiveMQBytesMessage.java index 822d7643652..2e177af95b6 100644 --- a/activemq-client/src/main/java/org/apache/activemq/command/ActiveMQBytesMessage.java +++ b/activemq-client/src/main/java/org/apache/activemq/command/ActiveMQBytesMessage.java @@ -965,7 +965,10 @@ protected void doCompress() throws IOException { @Override @SuppressWarnings("unchecked") public boolean isBodyAssignableTo(Class c) { - return getContent() == null || c.isAssignableFrom(byte[].class); + // Bytes written in write-only mode sit in bytesOut until stored, so a null + // content alone does not mean the message has no body. + var hasBody = getContent() != null || (bytesOut != null && bytesOut.size() > 0); + return !hasBody || c.isAssignableFrom(byte[].class); } @SuppressWarnings("unchecked") diff --git a/activemq-client/src/main/java/org/apache/activemq/command/ActiveMQMapMessage.java b/activemq-client/src/main/java/org/apache/activemq/command/ActiveMQMapMessage.java index 6f5473f659b..576a4a65fc1 100644 --- a/activemq-client/src/main/java/org/apache/activemq/command/ActiveMQMapMessage.java +++ b/activemq-client/src/main/java/org/apache/activemq/command/ActiveMQMapMessage.java @@ -793,8 +793,9 @@ public void setBytes(String name, byte[] value, int offset, int length) throws J public void setObject(String name, Object value) throws JMSException { initializeWriting(); if (value != null) { - // byte[] not allowed on properties - if (!(value instanceof byte[])) { + // byte[] and Character are valid MapMessage body types (MapMessage#setBytes, + // #setChar) but not valid property types, so they bypass the property check + if (!(value instanceof byte[]) && !(value instanceof Character)) { checkValidObject(value); } put(name, value); diff --git a/activemq-tooling/activemq-jakarta-messaging-tck/pom.xml b/activemq-tooling/activemq-jakarta-messaging-tck/pom.xml index 1a6ebae848f..36577f48953 100644 --- a/activemq-tooling/activemq-jakarta-messaging-tck/pom.xml +++ b/activemq-tooling/activemq-jakarta-messaging-tck/pom.xml @@ -43,6 +43,11 @@ org.apache.activemq activemq-kahadb-store + + + org.apache.activemq + activemq-jaas + org.apache.logging.log4j log4j-slf4j2-impl diff --git a/activemq-tooling/activemq-jakarta-messaging-tck/src/main/java/org/apache/activemq/tck/JNDIInitialContextFactory.java b/activemq-tooling/activemq-jakarta-messaging-tck/src/main/java/org/apache/activemq/tck/JNDIInitialContextFactory.java index a95a86eeb23..66dccd6edaf 100644 --- a/activemq-tooling/activemq-jakarta-messaging-tck/src/main/java/org/apache/activemq/tck/JNDIInitialContextFactory.java +++ b/activemq-tooling/activemq-jakarta-messaging-tck/src/main/java/org/apache/activemq/tck/JNDIInitialContextFactory.java @@ -28,6 +28,9 @@ import javax.naming.NamingException; import javax.naming.spi.InitialContextFactory; +import org.apache.activemq.broker.BrokerPlugin; +import org.apache.activemq.security.AuthenticationUser; +import org.apache.activemq.security.SimpleAuthenticationPlugin; import org.apache.activemq.ActiveMQConnectionFactory; import org.apache.activemq.broker.BrokerService; import org.apache.activemq.command.ActiveMQQueue; @@ -97,6 +100,11 @@ public Context getInitialContext(final Hashtable environment) throws Namin private static ActiveMQConnectionFactory createConnectionFactory(final String clientId) { final ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory(BROKER_URL); factory.setNestedMapAndListEnabled(false); + // The TCK asserts strict Jakarta Messaging semantics that ActiveMQ relaxes by + // default for backwards compatibility: eager authentication on + // createConnection/createContext, the administratively configured client + // identifier, and the strict message property rules. + factory.setStrictCompliance(true); if (clientId != null) { factory.setClientID(clientId); } @@ -117,6 +125,14 @@ private static void ensureBrokerStarted() { bs.setPersistent(false); bs.setUseJmx(false); bs.setAdvisorySupport(false); + // The TCK expects invalid credentials to be rejected (JMSSecurityException + // from createConnection, JMSSecurityRuntimeException from createContext). + // Authenticate the ts.jte user and keep anonymous access for the many + // tests that connect without credentials. + var authentication = new SimpleAuthenticationPlugin( + java.util.List.of(new AuthenticationUser("guest", "guest", "users"))); + authentication.setAnonymousAccessAllowed(true); + bs.setPlugins(new BrokerPlugin[] {authentication}); bs.start(); bs.waitUntilStarted(); broker = bs; diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/ActiveMQMessagePropertyTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/ActiveMQMessagePropertyTest.java index 1180c825a9a..ce8a041be5e 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/ActiveMQMessagePropertyTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/ActiveMQMessagePropertyTest.java @@ -22,6 +22,7 @@ import org.junit.Test; import jakarta.jms.Connection; +import jakarta.jms.MapMessage; import jakarta.jms.Message; import jakarta.jms.MessageFormatException; import jakarta.jms.Session; @@ -108,4 +109,49 @@ public void testLegacyModeStillAllowsCharacter() throws Exception { assertEquals('A', message.getObjectProperty("charProp")); } } + + @Test + public void testStrictComplianceAllowsCharacterInMapMessageBody() throws Exception { + var factory = new ActiveMQConnectionFactory(connectionUri); + factory.setStrictCompliance(true); + + try (var connection = factory.createConnection(); + var session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE)) { + + // MapMessage bodies support char per the spec; only message properties + // exclude Character under strict compliance + var message = session.createMapMessage(); + message.setChar("charEntry", 'A'); + message.setObject("charObject", 'B'); + assertEquals('A', message.getChar("charEntry")); + assertEquals('B', message.getObject("charObject")); + + try { + message.setObjectProperty("charProp", 'C'); + fail("Character properties must still be rejected under strict compliance"); + } catch (MessageFormatException expected) { + } + } + } + + @Test + public void testGetBodyOnWriteOnlyBytesMessageChecksType() throws Exception { + var factory = new ActiveMQConnectionFactory(connectionUri); + + try (var connection = factory.createConnection(); + var session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE)) { + + var message = session.createBytesMessage(); + message.writeBytes(new byte[] {1, 2, 3}); + + // bytes written but not yet stored are still a body: a non-byte[] type + // must be rejected even while the message is in write-only mode + try { + message.getBody(StringBuffer.class); + fail("Expected MessageFormatException for a non byte[] body type"); + } catch (MessageFormatException expected) { + } + assertEquals(3, message.getBody(byte[].class).length); + } + } } diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/StrictComplianceAuthenticationTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/StrictComplianceAuthenticationTest.java new file mode 100644 index 00000000000..6f91e5c696b --- /dev/null +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/StrictComplianceAuthenticationTest.java @@ -0,0 +1,137 @@ +/** + * 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.activemq; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; + +import java.util.ArrayList; + +import jakarta.jms.JMSException; +import jakarta.jms.JMSSecurityException; +import jakarta.jms.JMSSecurityRuntimeException; + +import org.apache.activemq.broker.BrokerPlugin; +import org.apache.activemq.broker.BrokerService; +import org.apache.activemq.security.AuthenticationUser; +import org.apache.activemq.security.SimpleAuthenticationPlugin; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +/** + * Jakarta Messaging expects createConnection and createContext to authenticate + * the caller immediately. ActiveMQ defers the ConnectionInfo exchange until + * first use, so the eager handshake is enabled by strictCompliance. + */ +public class StrictComplianceAuthenticationTest { + + private BrokerService broker; + private String connectionUri; + + @Before + public void setUp() throws Exception { + broker = new BrokerService(); + broker.setPersistent(false); + broker.setUseJmx(false); + broker.setAdvisorySupport(false); + var users = new ArrayList(); + users.add(new AuthenticationUser("system", "manager", "users,admins")); + broker.setPlugins(new BrokerPlugin[] {new SimpleAuthenticationPlugin(users)}); + broker.addConnector("tcp://localhost:0"); + broker.start(); + broker.waitUntilStarted(); + connectionUri = broker.getTransportConnectors().get(0).getPublishableConnectString(); + } + + @After + public void tearDown() throws Exception { + if (broker != null) { + broker.stop(); + broker.waitUntilStopped(); + } + } + + @Test(timeout = 60000) + public void testStrictCreateConnectionRejectsBadCredentialsImmediately() throws Exception { + var factory = new ActiveMQConnectionFactory(connectionUri); + factory.setStrictCompliance(true); + + try { + factory.createConnection("invalid", "credentials"); + fail("Expected JMSSecurityException from createConnection"); + } catch (JMSSecurityException expected) { + } + + try (var connection = factory.createConnection("system", "manager")) { + connection.start(); + assertNotNull(connection.getClientID()); + } + } + + @Test(timeout = 60000) + public void testStrictCreateContextRejectsBadCredentialsImmediately() throws Exception { + var factory = new ActiveMQConnectionFactory(connectionUri); + factory.setStrictCompliance(true); + + try { + factory.createContext("invalid", "credentials"); + fail("Expected JMSSecurityRuntimeException from createContext"); + } catch (JMSSecurityRuntimeException expected) { + } + + try (var context = factory.createContext("system", "manager")) { + context.start(); + assertNotNull(context.getClientID()); + } + } + + @Test(timeout = 60000) + public void testStrictCreateConnectionStillAllowsSetClientID() throws Exception { + var factory = new ActiveMQConnectionFactory(connectionUri); + factory.setStrictCompliance(true); + + // the spec-mandated sequence: set the client identifier immediately after + // creation, before any other action; eager authentication must not + // consume the connection's identity + try (var connection = factory.createConnection("system", "manager")) { + connection.setClientID("strict-client"); + connection.start(); + assertEquals("strict-client", connection.getClientID()); + } + } + + @Test(timeout = 60000) + public void testLegacyCreateConnectionDefersAuthenticationToStart() throws Exception { + var factory = new ActiveMQConnectionFactory(connectionUri); + + // Default strictCompliance = false: creation succeeds, the failure surfaces + // when the connection first talks to the broker + var connection = factory.createConnection("invalid", "credentials"); + try { + connection.start(); + fail("Expected JMSSecurityException on start"); + } catch (JMSSecurityException expected) { + } catch (JMSException disposed) { + // the failed connection may be torn down asynchronously before the + // security exception reaches start(); either form indicates rejection + } finally { + try { connection.close(); } catch (Exception ignored) {} + } + } +} diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/StrictComplianceClientIDTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/StrictComplianceClientIDTest.java new file mode 100644 index 00000000000..a78b9981dda --- /dev/null +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/StrictComplianceClientIDTest.java @@ -0,0 +1,94 @@ +/** + * 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.activemq; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import jakarta.jms.IllegalStateRuntimeException; + +import org.apache.activemq.broker.BrokerService; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +/** + * A client identifier configured on the connection factory is administratively + * configured; the specification forbids the application from overriding it. + * ActiveMQ has always allowed the override, so enforcement is tied to + * strictCompliance. + */ +public class StrictComplianceClientIDTest { + + private BrokerService broker; + + @Before + public void setUp() throws Exception { + broker = new BrokerService(); + broker.setPersistent(false); + broker.setUseJmx(false); + broker.setAdvisorySupport(false); + broker.addConnector("vm://localhost"); + broker.start(); + broker.waitUntilStarted(); + } + + @After + public void tearDown() throws Exception { + if (broker != null) { + broker.stop(); + broker.waitUntilStopped(); + } + } + + @Test(timeout = 60000) + public void testStrictRejectsOverridingAdminConfiguredClientID() throws Exception { + var factory = new ActiveMQConnectionFactory("vm://localhost"); + factory.setStrictCompliance(true); + factory.setClientID("admin-configured"); + + try (var connection = factory.createConnection()) { + try { + connection.setClientID("application-override"); + fail("Expected IllegalStateException overriding an administratively configured clientID"); + } catch (jakarta.jms.IllegalStateException expected) { + } + assertEquals("admin-configured", connection.getClientID()); + } + + try (var context = factory.createContext()) { + try { + context.setClientID("application-override"); + fail("Expected IllegalStateRuntimeException overriding an administratively configured clientID"); + } catch (IllegalStateRuntimeException expected) { + } + assertEquals("admin-configured", context.getClientID()); + } + } + + @Test(timeout = 60000) + public void testLegacyAllowsOverridingAdminConfiguredClientID() throws Exception { + var factory = new ActiveMQConnectionFactory("vm://localhost"); + factory.setClientID("admin-configured"); + + // Default strictCompliance = false keeps the historical override behavior + try (var connection = factory.createConnection()) { + connection.setClientID("application-override"); + assertEquals("application-override", connection.getClientID()); + } + } +} diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/jms2/ActiveMQJMS2ContextTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/jms2/ActiveMQJMS2ContextTest.java index 5334546aa56..5a792289a9f 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/jms2/ActiveMQJMS2ContextTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/jms2/ActiveMQJMS2ContextTest.java @@ -60,6 +60,32 @@ public void testConnectionFactoryCreateContext() { } } + @Test + public void testCreateContextRejectsInvalidSessionMode() { + for (int invalidMode : new int[] {-1, 5, 99}) { + try { + activemqConnectionFactory.createContext(invalidMode).close(); + fail("Expected JMSRuntimeException for session mode " + invalidMode); + } catch (JMSRuntimeException expected) { + } + try { + activemqConnectionFactory.createContext(DEFAULT_JMS_USER, DEFAULT_JMS_PASS, invalidMode).close(); + fail("Expected JMSRuntimeException for session mode " + invalidMode); + } catch (JMSRuntimeException expected) { + } + } + + // the ActiveMQ INDIVIDUAL_ACKNOWLEDGE extension remains a valid mode + try (var jmsContext = activemqConnectionFactory.createContext(org.apache.activemq.ActiveMQSession.INDIVIDUAL_ACKNOWLEDGE)) { + assertEquals(org.apache.activemq.ActiveMQSession.INDIVIDUAL_ACKNOWLEDGE, jmsContext.getSessionMode()); + try { + jmsContext.createContext(42).close(); + fail("Expected JMSRuntimeException for child context session mode 42"); + } catch (JMSRuntimeException expected) { + } + } + } + @Test public void testConnectionFactoryCreateContextSession() { try(JMSContext jmsContext = activemqConnectionFactory.createContext(Session.AUTO_ACKNOWLEDGE)) {