From dcbf4ef06b5f1d81b696398f6a8e4e95aa866a5e Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Fri, 31 Jul 2026 20:11:16 +0300 Subject: [PATCH 1/4] Fix LDAPConnection binding the client socket to the server address createSocket() has been binding the new client socket to the target server address instead of connecting to it since #279, so every plain or StartTLS connection made through org.opends.server.tools.LDAPConnection fails with "Address already in use" (server on the same host) or "Cannot assign requested address" (remote server). Affects the DSML gateway, stop-ds, manage-account and the other tools built on LDAPConnectionArgumentParser. --- .../main/java/org/opends/server/tools/LDAPConnection.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opendj-server-legacy/src/main/java/org/opends/server/tools/LDAPConnection.java b/opendj-server-legacy/src/main/java/org/opends/server/tools/LDAPConnection.java index f0eef293a3..9abe7e7754 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/tools/LDAPConnection.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/tools/LDAPConnection.java @@ -443,10 +443,10 @@ private Socket createSocket() throws LDAPConnectionException { try { - final Socket s=new Socket(); - s.setReuseAddress(true); - s.bind( new InetSocketAddress(inetAddress, portNumber)); - return s; + final Socket s = new Socket(); + s.setReuseAddress(true); + s.connect(new InetSocketAddress(inetAddress, portNumber)); + return s; } catch (ConnectException ce2) { From c9e4d088ac4b38c435cc91798f84dea597e8fe51 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Fri, 31 Jul 2026 20:11:16 +0300 Subject: [PATCH 2/4] [#809] fix DSML gateway NPE on abandonRequest and on missing Content-Type performLDAPRequest() returns null for an abandon request, but doPost() dereferenced the result unconditionally, so a batch containing ended in a NullPointerException; as the connection was closed after the loop instead of in a finally, one LDAP connection was leaked per request. messageFactory was only assigned when a SOAP 1.1 or SOAP 1.2 Content-Type header was present, and was then dereferenced both when parsing the request and when sending the response: a POST without Content-Type ended in a NullPointerException, and, when an error response had already been queued, in an empty HTTP 200 instead of that error. A missing or unsupported Content-Type is now answered with a malformedRequest batch response. Also log the failure instead of printing the stack trace when the response cannot be sent, and add regression tests for both defects. --- .../org/opends/dsml/protocol/DSMLServlet.java | 133 ++++--- .../dsml/protocol/DSMLServletTestCase.java | 376 ++++++++++++++++++ 2 files changed, 458 insertions(+), 51 deletions(-) create mode 100644 opendj-dsml-servlet/src/test/java/org/opends/dsml/protocol/DSMLServletTestCase.java diff --git a/opendj-dsml-servlet/src/main/java/org/opends/dsml/protocol/DSMLServlet.java b/opendj-dsml-servlet/src/main/java/org/opends/dsml/protocol/DSMLServlet.java index 46630f4354..2ca10282b5 100644 --- a/opendj-dsml-servlet/src/main/java/org/opends/dsml/protocol/DSMLServlet.java +++ b/opendj-dsml-servlet/src/main/java/org/opends/dsml/protocol/DSMLServlet.java @@ -400,9 +400,8 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) messageFactory = MessageFactory.newInstance(SOAPConstants.SOAP_1_2_PROTOCOL); messageContentType = SOAPConstants.SOAP_1_2_CONTENT_TYPE; } - else { - throw new ServletException("Content-Type does not match SOAP 1.1 or SOAP 1.2"); - } + // An unsupported Content-Type leaves the message factory unset: the + // request is rejected as malformed once all the headers are read. } catch (SOAPException e) { @@ -477,6 +476,30 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) } } + if ( messageFactory == null ) { + // The request carries no Content-Type header, or one which matches + // neither SOAP 1.1 nor SOAP 1.2: it cannot be parsed. Fall back to + // SOAP 1.1 for the response and reject the request as malformed, + // unless an error has already been reported. + try + { + messageFactory = MessageFactory.newInstance(SOAPConstants.SOAP_1_1_PROTOCOL); + messageContentType = SOAPConstants.SOAP_1_1_CONTENT_TYPE; + } + catch (SOAPException e) + { + throw new ServletException(e.getMessage()); + } + if ( batchResponses.isEmpty() ) { + ErrorResponse errorResponse = objFactory.createErrorResponse(); + errorResponse.setType(MALFORMED_REQUEST); + errorResponse.setMessage( + "Content-Type does not match SOAP 1.1 or SOAP 1.2"); + batchResponses.add( + objFactory.createBatchResponseErrorResponse(errorResponse)); + } + } + // if an error already occurred, the list is not empty if ( batchResponses.isEmpty() ) { try { @@ -541,60 +564,67 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) boolean connected = false; - if ( connection == null ) { - connection = new LDAPConnection(hostName, port, connOptions); - try { + try { + if ( connection == null ) { + connection = new LDAPConnection(hostName, port, connOptions); + try { - connection.connectToHost(bindDN, bindPassword); - if (authzInControl) - { - proxyAuthzControl = checkAuthzControl(connection, - batchRequest.authRequest.getPrincipal()); - } - if (authzInBind || authzInControl) - { - LDAPResult authResponse = objFactory.createLDAPResult(); - ResultCode code = ResultCodeFactory.create(objFactory, - LDAPResultCode.SUCCESS); - authResponse.setResultCode(code); - batchResponses.add( - objFactory.createBatchResponseAuthResponse(authResponse)); + connection.connectToHost(bindDN, bindPassword); + if (authzInControl) + { + proxyAuthzControl = checkAuthzControl(connection, + batchRequest.authRequest.getPrincipal()); + } + if (authzInBind || authzInControl) + { + LDAPResult authResponse = objFactory.createLDAPResult(); + ResultCode code = ResultCodeFactory.create(objFactory, + LDAPResultCode.SUCCESS); + authResponse.setResultCode(code); + batchResponses.add( + objFactory.createBatchResponseAuthResponse(authResponse)); + } + connected = true; + } catch (LDAPConnectionException e) { + // if connection failed, return appropriate error response + batchResponses.add(createErrorResponse(objFactory, e)); } - connected = true; - } catch (LDAPConnectionException e) { - // if connection failed, return appropriate error response - batchResponses.add(createErrorResponse(objFactory, e)); } - } - if ( connected ) { - List list = batchRequest.getBatchRequests(); - - for (DsmlMessage request : list) { - JAXBElement result = performLDAPRequest(connection, objFactory, proxyAuthzControl, request); - if ( result != null ) { - batchResponses.add(result); - } - // evaluate response to check if an error occurred - Object o = result.getValue(); - if ( o instanceof ErrorResponse ) { - if ( ON_ERROR_EXIT.equals(batchRequest.getOnError()) ) { - break; + if ( connected ) { + List list = batchRequest.getBatchRequests(); + + for (DsmlMessage request : list) { + JAXBElement result = performLDAPRequest(connection, objFactory, proxyAuthzControl, request); + if ( result == null ) { + // an abandon request does not produce any response element + continue; } - } else if ( o instanceof LDAPResult ) { - int code = ((LDAPResult)o).getResultCode().getCode(); - if ( code != LDAPResultCode.SUCCESS - && code != LDAPResultCode.REFERRAL - && code != LDAPResultCode.COMPARE_TRUE - && code != LDAPResultCode.COMPARE_FALSE && ON_ERROR_EXIT.equals(batchRequest.getOnError()) ) - { - break; + batchResponses.add(result); + // evaluate response to check if an error occurred + Object o = result.getValue(); + if ( o instanceof ErrorResponse ) { + if ( ON_ERROR_EXIT.equals(batchRequest.getOnError()) ) { + break; + } + } else if ( o instanceof LDAPResult ) { + int code = ((LDAPResult)o).getResultCode().getCode(); + if ( code != LDAPResultCode.SUCCESS + && code != LDAPResultCode.REFERRAL + && code != LDAPResultCode.COMPARE_TRUE + && code != LDAPResultCode.COMPARE_FALSE && ON_ERROR_EXIT.equals(batchRequest.getOnError()) ) + { + break; + } } } } - } - // close connection to LDAP server - if ( connection != null ) { - connection.close(nextMessageID); + } finally { + // close connection to LDAP server, whatever happened while + // processing the batch, and do not reuse it for the next one + if ( connection != null ) { + connection.close(nextMessageID); + connection = null; + } } } } @@ -604,7 +634,8 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) marshaller.marshal(objFactory.createBatchResponse(batchResponse), doc); sendResponse(doc, messageFactory, messageContentType, res); } catch (Exception e) { - e.printStackTrace(); + // the client gets an empty response: at least make the cause visible + Logger.getLogger(PKG_NAME).log(Level.SEVERE, "Unable to send the DSML response", e); } } diff --git a/opendj-dsml-servlet/src/test/java/org/opends/dsml/protocol/DSMLServletTestCase.java b/opendj-dsml-servlet/src/test/java/org/opends/dsml/protocol/DSMLServletTestCase.java new file mode 100644 index 0000000000..3a89f29e0b --- /dev/null +++ b/opendj-dsml-servlet/src/test/java/org/opends/dsml/protocol/DSMLServletTestCase.java @@ -0,0 +1,376 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.opends.dsml.protocol; + +import static org.opends.server.protocols.ldap.LDAPConstants.OP_TYPE_ABANDON_REQUEST; +import static org.opends.server.protocols.ldap.LDAPConstants.OP_TYPE_BIND_REQUEST; +import static org.opends.server.protocols.ldap.LDAPConstants.OP_TYPE_UNBIND_REQUEST; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.Closeable; +import java.io.IOException; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.net.InetAddress; +import java.net.ServerSocket; +import java.net.Socket; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Base64; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import jakarta.servlet.ReadListener; +import jakarta.servlet.ServletConfig; +import jakarta.servlet.ServletContext; +import jakarta.servlet.ServletInputStream; +import jakarta.servlet.ServletOutputStream; +import jakarta.servlet.WriteListener; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.forgerock.testng.ForgeRockTestCase; +import org.opends.server.protocols.ldap.BindResponseProtocolOp; +import org.opends.server.protocols.ldap.LDAPMessage; +import org.opends.server.protocols.ldap.LDAPResultCode; +import org.opends.server.tools.LDAPReader; +import org.opends.server.tools.LDAPWriter; +import org.testng.annotations.Test; + +/** + * Tests the error handling of {@link DSMLServlet#doPost}: an abandon request + * used to trigger a {@code NullPointerException} which leaked the LDAP + * connection, and a request without a usable Content-Type header used to + * trigger a {@code NullPointerException} as well. + */ +@SuppressWarnings("javadoc") +@Test(groups = { "precommit", "dsml" }) +public class DSMLServletTestCase extends ForgeRockTestCase +{ + /** SOAP 1.1 content type. */ + private static final String SOAP_1_1_CONTENT_TYPE = "text/xml"; + + private static final String ABANDON_BATCH = + "" + + "" + + "" + + "" + + "" + + "" + + "" + + ""; + + /** + * An abandon request produces no response element: the servlet must neither + * fail nor leave the connection to the directory server open. + */ + @Test + public void testAbandonRequestIsProcessedAndConnectionIsClosed() throws Exception + { + try (FakeLdapServer server = new FakeLdapServer()) + { + Map headers = new LinkedHashMap<>(); + headers.put("Content-Type", SOAP_1_1_CONTENT_TYPE); + + String response = doPost(server.getPort(), headers, ABANDON_BATCH); + + assertTrue(response.contains("batchResponse"), response); + assertFalse(response.contains("errorResponse"), response); + // no response element is defined for an abandon request + assertFalse(response.contains("abandonResponse"), response); + + server.awaitDisconnect(); + assertEquals(server.getReceivedOpTypes(), + list(OP_TYPE_BIND_REQUEST, OP_TYPE_ABANDON_REQUEST, OP_TYPE_UNBIND_REQUEST), + "the abandon request was not forwarded, or the connection was leaked"); + } + } + + /** A request without any Content-Type header must be rejected as malformed. */ + @Test + public void testMissingContentTypeIsRejectedAsMalformedRequest() throws Exception + { + try (FakeLdapServer server = new FakeLdapServer()) + { + String response = doPost(server.getPort(), new LinkedHashMap(), ABANDON_BATCH); + + assertTrue(response.contains("malformedRequest"), response); + assertTrue(server.getReceivedOpTypes().isEmpty(), + "no connection to the directory server should have been opened"); + } + } + + /** A Content-Type header matching neither SOAP 1.1 nor SOAP 1.2 is malformed too. */ + @Test + public void testUnsupportedContentTypeIsRejectedAsMalformedRequest() throws Exception + { + try (FakeLdapServer server = new FakeLdapServer()) + { + Map headers = new LinkedHashMap<>(); + headers.put("Content-Type", "application/json"); + + String response = doPost(server.getPort(), headers, ABANDON_BATCH); + + assertTrue(response.contains("malformedRequest"), response); + assertTrue(server.getReceivedOpTypes().isEmpty(), + "no connection to the directory server should have been opened"); + } + } + + /** + * An error detected before the request is parsed must still reach the client + * when the Content-Type header is missing. + */ + @Test + public void testMissingContentTypeStillReportsCredentialsError() throws Exception + { + try (FakeLdapServer server = new FakeLdapServer()) + { + Map headers = new LinkedHashMap<>(); + // credentials without the ':' separator: the password cannot be retrieved + headers.put("Authorization", "Basic " + Base64.getEncoder() + .encodeToString("cn=directory manager".getBytes(StandardCharsets.UTF_8))); + + String response = doPost(server.getPort(), headers, ABANDON_BATCH); + + assertTrue(response.contains("authenticationFailed"), response); + assertTrue(server.getReceivedOpTypes().isEmpty(), + "no connection to the directory server should have been opened"); + } + } + + /** Runs {@code doPost} against a servlet configured to use the given LDAP port. */ + private String doPost(int ldapPort, Map headers, String body) throws Exception + { + Map params = new LinkedHashMap<>(); + params.put("ldap.host", InetAddress.getLoopbackAddress().getHostAddress()); + params.put("ldap.port", String.valueOf(ldapPort)); + + DSMLServlet servlet = new DSMLServlet(); + servlet.init(servletConfig(params)); + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + servlet.doPost(httpRequest(headers, body.getBytes(StandardCharsets.UTF_8)), httpResponse(out)); + return new String(out.toByteArray(), StandardCharsets.UTF_8); + } + + private static List list(byte... opTypes) + { + List result = new ArrayList<>(opTypes.length); + for (byte opType : opTypes) + { + result.add(opType); + } + return result; + } + + /** + * A minimal LDAP endpoint which answers the bind request with a success + * result and records the type of every message it receives. + */ + private static final class FakeLdapServer implements Closeable + { + private final ServerSocket serverSocket; + private final List receivedOpTypes = new CopyOnWriteArrayList<>(); + private final CountDownLatch disconnected = new CountDownLatch(1); + private volatile boolean stopped; + + FakeLdapServer() throws IOException + { + serverSocket = new ServerSocket(0, 1, InetAddress.getLoopbackAddress()); + Thread thread = new Thread(this::serve, "fake-ldap-server"); + thread.setDaemon(true); + thread.start(); + } + + int getPort() + { + return serverSocket.getLocalPort(); + } + + List getReceivedOpTypes() + { + return new ArrayList<>(receivedOpTypes); + } + + void awaitDisconnect() throws InterruptedException + { + assertTrue(disconnected.await(30, TimeUnit.SECONDS), "the client did not disconnect"); + } + + private void serve() + { + try (Socket socket = serverSocket.accept()) + { + LDAPReader reader = new LDAPReader(socket); + LDAPWriter writer = new LDAPWriter(socket); + LDAPMessage message; + while ((message = reader.readMessage()) != null) + { + receivedOpTypes.add(message.getProtocolOpType()); + if (message.getProtocolOpType() == OP_TYPE_BIND_REQUEST) + { + writer.writeMessage(new LDAPMessage(message.getMessageID(), + new BindResponseProtocolOp(LDAPResultCode.SUCCESS))); + } + } + } + catch (Exception e) + { + if (!stopped) + { + e.printStackTrace(); + } + } + finally + { + disconnected.countDown(); + } + } + + @Override + public void close() throws IOException + { + stopped = true; + serverSocket.close(); + } + } + + private static ServletConfig servletConfig(final Map params) + { + final ServletContext context = stub(ServletContext.class, (proxy, method, args) -> { + switch (method.getName()) + { + case "getInitParameter": + return params.get(args[0]); + case "getInitParameterNames": + return Collections.enumeration(params.keySet()); + default: + return defaultValue(method); + } + }); + return stub(ServletConfig.class, (proxy, method, args) -> + "getServletContext".equals(method.getName()) ? context : defaultValue(method)); + } + + private static HttpServletRequest httpRequest(final Map headers, final byte[] body) + { + final ByteArrayInputStream content = new ByteArrayInputStream(body); + final ServletInputStream in = new ServletInputStream() + { + @Override + public int read() + { + return content.read(); + } + + @Override + public boolean isFinished() + { + return content.available() == 0; + } + + @Override + public boolean isReady() + { + return true; + } + + @Override + public void setReadListener(ReadListener readListener) + { + // not used + } + }; + return stub(HttpServletRequest.class, (proxy, method, args) -> { + switch (method.getName()) + { + case "getInputStream": + return in; + case "getHeaderNames": + return Collections.enumeration(headers.keySet()); + case "getHeader": + return headers.get(args[0]); + default: + return defaultValue(method); + } + }); + } + + private static HttpServletResponse httpResponse(final ByteArrayOutputStream out) + { + final ServletOutputStream os = new ServletOutputStream() + { + @Override + public void write(int b) + { + out.write(b); + } + + @Override + public boolean isReady() + { + return true; + } + + @Override + public void setWriteListener(WriteListener writeListener) + { + // not used + } + }; + return stub(HttpServletResponse.class, (proxy, method, args) -> + "getOutputStream".equals(method.getName()) ? os : defaultValue(method)); + } + + private static T stub(Class type, InvocationHandler handler) + { + return type.cast(Proxy.newProxyInstance( + DSMLServletTestCase.class.getClassLoader(), new Class[] { type }, handler)); + } + + private static Object defaultValue(Method method) + { + Class returnType = method.getReturnType(); + if (returnType == boolean.class) + { + return Boolean.FALSE; + } + else if (returnType == int.class) + { + return 0; + } + else if (returnType == long.class) + { + return 0L; + } + else if ("toString".equals(method.getName())) + { + return "stub"; + } + return null; + } +} From fcfd513ef659741afdbd0a593d189f54520a4170 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Mon, 3 Aug 2026 11:37:19 +0300 Subject: [PATCH 3/4] [#809] Address the review of the DSML gateway fix Report the failure to send the response to the container log: the java.util.logging record was dropped, as connectToHost() resets the LogManager and turns the root logger off on every non-verbose connection. This needs super.init(config), without which getServletContext() throws. Drop the authzid of the previous batch request before setting the new one: the connection options are shared by the whole SOAP body and addSASLProperty() appends to the values of a key, so a second authRequest made SASL PLAIN reject a multi-valued authzid. Now that the connection is never reused, make it a loop local and remove the dead null check that guarded the reuse. Build the malformed Content-Type response with createXMLParsingErrorResponse(), like the other two malformed paths, so that the requestID is recovered; and keep reading the headers after a malformed Authorization one, so that the reply keeps the SOAP version of the request. Cover the SOAP 1.2 path, the per-batch-request connection and the authzid, let the fake LDAP endpoint serve several connections and fail the test on a server-side error, and pin the createSocket() regression of #279 with a test in the module that owns it. --- .../org/opends/dsml/protocol/DSMLServlet.java | 88 +++--- .../dsml/protocol/DSMLServletTestCase.java | 277 ++++++++++++++++-- .../server/tools/LDAPConnectionTestCase.java | 96 ++++++ 3 files changed, 392 insertions(+), 69 deletions(-) create mode 100644 opendj-server-legacy/src/test/java/org/opends/server/tools/LDAPConnectionTestCase.java diff --git a/opendj-dsml-servlet/src/main/java/org/opends/dsml/protocol/DSMLServlet.java b/opendj-dsml-servlet/src/main/java/org/opends/dsml/protocol/DSMLServlet.java index 2ca10282b5..0407fb753f 100644 --- a/opendj-dsml-servlet/src/main/java/org/opends/dsml/protocol/DSMLServlet.java +++ b/opendj-dsml-servlet/src/main/java/org/opends/dsml/protocol/DSMLServlet.java @@ -165,6 +165,9 @@ public class DSMLServlet extends HttpServlet { */ @Override public void init(ServletConfig config) throws ServletException { + // Let GenericServlet keep the configuration: getServletContext() relies on + // it, and it is the only logging facility available at runtime. + super.init(config); try { hostName = stringValue(config, HOST); port = Integer.valueOf(stringValue(config, PORT)); @@ -333,7 +336,6 @@ public void doPost(HttpServletRequest req, HttpServletResponse res) connOptions.setUseSSL(useSSL); connOptions.setStartTLS(useStartTLS); - LDAPConnection connection = null; BatchRequest batchRequest = null; // Keep the Servlet input stream buffered in case the SOAP un-marshalling @@ -429,12 +431,14 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) bindPassword = unencoded.substring(colon + 1); } } catch (final LocalizedIllegalArgumentException ex) { - // user/DN:password parsing error + // user/DN:password parsing error. Keep reading the headers: the + // Content-Type may still be ahead, and it decides which SOAP + // version the error is reported with. batchResponses.add( createErrorResponse(objFactory, new LDAPException(LDAPResultCode.INVALID_CREDENTIALS, LocalizableMessage.raw(ex.getMessage())))); - break; + continue; } } StringTokenizer tk = new StringTokenizer(headerVal, ","); @@ -491,12 +495,13 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) throw new ServletException(e.getMessage()); } if ( batchResponses.isEmpty() ) { - ErrorResponse errorResponse = objFactory.createErrorResponse(); - errorResponse.setType(MALFORMED_REQUEST); - errorResponse.setMessage( - "Content-Type does not match SOAP 1.1 or SOAP 1.2"); + // Nothing has been read from the stream yet, so the SAX pass can still + // recover the requestID and let the client correlate the reply. batchResponses.add( - objFactory.createBatchResponseErrorResponse(errorResponse)); + createXMLParsingErrorResponse(is, + objFactory, + batchResponse, + "Content-Type does not match SOAP 1.1 or SOAP 1.2")); } } @@ -548,7 +553,12 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) */ if (batchRequest.authRequest != null) { if (authenticationIsID) { - // If we are using SASL, then use the bind authz. + // If we are using SASL, then use the bind authz. The options are + // shared by all the batch requests of this SOAP body, and + // addSASLProperty() appends to the values of a key: drop the + // authzid of the previous batch request, as SASL PLAIN rejects a + // multi-valued one. + connOptions.getSASLProperties().remove("authzid"); connOptions.addSASLProperty("authzid=" + batchRequest.authRequest.getPrincipal()); authzInBind = true; @@ -564,31 +574,31 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) boolean connected = false; + // Each batch request gets its own connection: the previous one has + // been closed by the finally block below. + LDAPConnection connection = + new LDAPConnection(hostName, port, connOptions); try { - if ( connection == null ) { - connection = new LDAPConnection(hostName, port, connOptions); - try { - - connection.connectToHost(bindDN, bindPassword); - if (authzInControl) - { - proxyAuthzControl = checkAuthzControl(connection, - batchRequest.authRequest.getPrincipal()); - } - if (authzInBind || authzInControl) - { - LDAPResult authResponse = objFactory.createLDAPResult(); - ResultCode code = ResultCodeFactory.create(objFactory, - LDAPResultCode.SUCCESS); - authResponse.setResultCode(code); - batchResponses.add( - objFactory.createBatchResponseAuthResponse(authResponse)); - } - connected = true; - } catch (LDAPConnectionException e) { - // if connection failed, return appropriate error response - batchResponses.add(createErrorResponse(objFactory, e)); + try { + connection.connectToHost(bindDN, bindPassword); + if (authzInControl) + { + proxyAuthzControl = checkAuthzControl(connection, + batchRequest.authRequest.getPrincipal()); + } + if (authzInBind || authzInControl) + { + LDAPResult authResponse = objFactory.createLDAPResult(); + ResultCode code = ResultCodeFactory.create(objFactory, + LDAPResultCode.SUCCESS); + authResponse.setResultCode(code); + batchResponses.add( + objFactory.createBatchResponseAuthResponse(authResponse)); } + connected = true; + } catch (LDAPConnectionException e) { + // if connection failed, return appropriate error response + batchResponses.add(createErrorResponse(objFactory, e)); } if ( connected ) { List list = batchRequest.getBatchRequests(); @@ -620,11 +630,8 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) } } finally { // close connection to LDAP server, whatever happened while - // processing the batch, and do not reuse it for the next one - if ( connection != null ) { - connection.close(nextMessageID); - connection = null; - } + // processing the batch + connection.close(nextMessageID); } } } @@ -634,8 +641,11 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) marshaller.marshal(objFactory.createBatchResponse(batchResponse), doc); sendResponse(doc, messageFactory, messageContentType, res); } catch (Exception e) { - // the client gets an empty response: at least make the cause visible - Logger.getLogger(PKG_NAME).log(Level.SEVERE, "Unable to send the DSML response", e); + // The client gets an empty response: at least make the cause visible. + // The container log is the only usable sink here, as connectToHost() + // turns java.util.logging off for the whole JVM and the war ships no + // SLF4J binding. + getServletContext().log("Unable to send the DSML response", e); } } diff --git a/opendj-dsml-servlet/src/test/java/org/opends/dsml/protocol/DSMLServletTestCase.java b/opendj-dsml-servlet/src/test/java/org/opends/dsml/protocol/DSMLServletTestCase.java index 3a89f29e0b..8d41ddb65e 100644 --- a/opendj-dsml-servlet/src/test/java/org/opends/dsml/protocol/DSMLServletTestCase.java +++ b/opendj-dsml-servlet/src/test/java/org/opends/dsml/protocol/DSMLServletTestCase.java @@ -20,6 +20,7 @@ import static org.opends.server.protocols.ldap.LDAPConstants.OP_TYPE_UNBIND_REQUEST; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; import java.io.ByteArrayInputStream; @@ -40,7 +41,6 @@ import java.util.List; import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import jakarta.servlet.ReadListener; @@ -63,8 +63,9 @@ /** * Tests the error handling of {@link DSMLServlet#doPost}: an abandon request * used to trigger a {@code NullPointerException} which leaked the LDAP - * connection, and a request without a usable Content-Type header used to - * trigger a {@code NullPointerException} as well. + * connection, a request without a usable Content-Type header used to trigger a + * {@code NullPointerException} as well, and the second batch request of a SOAP + * body used to be silently skipped. */ @SuppressWarnings("javadoc") @Test(groups = { "precommit", "dsml" }) @@ -72,16 +73,45 @@ public class DSMLServletTestCase extends ForgeRockTestCase { /** SOAP 1.1 content type. */ private static final String SOAP_1_1_CONTENT_TYPE = "text/xml"; + /** SOAP 1.2 content type. */ + private static final String SOAP_1_2_CONTENT_TYPE = "application/soap+xml"; + /** SOAP 1.2 envelope namespace, as it appears in the reply. */ + private static final String SOAP_1_2_NAMESPACE = "http://www.w3.org/2003/05/soap-envelope"; private static final String ABANDON_BATCH = - "" - + "" - + "" - + "" - + "" - + "" - + "" - + ""; + soap11(abandonBatch("1", null)); + + /** Two batch requests in a single SOAP body, each carrying an abandon request. */ + private static final String TWO_ABANDON_BATCHES = + soap11(abandonBatch("1", null) + abandonBatch("2", null)); + + /** Same, with an authRequest which turns into a SASL authzid on each bind. */ + private static final String TWO_AUTHZ_BATCHES = + soap11(abandonBatch("1", "dn:cn=first") + abandonBatch("2", "dn:cn=second")); + + private static String abandonBatch(String requestID, String authzPrincipal) + { + return "" + + (authzPrincipal != null ? "" : "") + + "" + + ""; + } + + private static String soap11(String body) + { + return "" + + "" + + "" + body + "" + + ""; + } + + private static String soap12(String body) + { + return "" + + "" + + "" + body + "" + + ""; + } /** * An abandon request produces no response element: the servlet must neither @@ -109,7 +139,10 @@ public void testAbandonRequestIsProcessedAndConnectionIsClosed() throws Exceptio } } - /** A request without any Content-Type header must be rejected as malformed. */ + /** + * A request without any Content-Type header must be rejected as malformed, + * keeping the requestID so that the client can correlate the reply. + */ @Test public void testMissingContentTypeIsRejectedAsMalformedRequest() throws Exception { @@ -118,6 +151,7 @@ public void testMissingContentTypeIsRejectedAsMalformedRequest() throws Exceptio String response = doPost(server.getPort(), new LinkedHashMap(), ABANDON_BATCH); assertTrue(response.contains("malformedRequest"), response); + assertTrue(response.contains("requestID=\"1\""), response); assertTrue(server.getReceivedOpTypes().isEmpty(), "no connection to the directory server should have been opened"); } @@ -162,12 +196,118 @@ public void testMissingContentTypeStillReportsCredentialsError() throws Exceptio } } + /** + * A malformed Authorization header must not stop the header scan: the + * Content-Type still decides which SOAP version the error is reported with. + */ + @Test + public void testMalformedAuthorizationKeepsTheRequestSoapVersion() throws Exception + { + try (FakeLdapServer server = new FakeLdapServer()) + { + Map headers = new LinkedHashMap<>(); + // credentials which are not valid Base64, read before the Content-Type + headers.put("Authorization", "Basic !!!"); + headers.put("Content-Type", SOAP_1_2_CONTENT_TYPE); + + String response = doPost(server.getPort(), headers, soap12(abandonBatch("1", null))); + + assertTrue(response.contains("authenticationFailed"), response); + assertTrue(response.contains(SOAP_1_2_NAMESPACE), response); + } + } + + /** The SOAP 1.2 request path must work as the SOAP 1.1 one does. */ + @Test + public void testSoap12RequestIsProcessed() throws Exception + { + try (FakeLdapServer server = new FakeLdapServer()) + { + Map headers = new LinkedHashMap<>(); + headers.put("Content-Type", SOAP_1_2_CONTENT_TYPE); + + String response = doPost(server.getPort(), headers, soap12(abandonBatch("1", null))); + + assertTrue(response.contains("batchResponse"), response); + assertFalse(response.contains("errorResponse"), response); + assertTrue(response.contains(SOAP_1_2_NAMESPACE), response); + + server.awaitDisconnect(); + assertEquals(server.getReceivedOpTypes(), + list(OP_TYPE_BIND_REQUEST, OP_TYPE_ABANDON_REQUEST, OP_TYPE_UNBIND_REQUEST), + "the abandon request was not forwarded, or the connection was leaked"); + } + } + + /** + * Every batch request of a SOAP body gets its own connection: the second one + * used to be silently skipped because the first connection was left assigned. + */ + @Test + public void testEachBatchRequestGetsItsOwnConnection() throws Exception + { + try (FakeLdapServer server = new FakeLdapServer()) + { + Map headers = new LinkedHashMap<>(); + headers.put("Content-Type", SOAP_1_1_CONTENT_TYPE); + + String response = doPost(server.getPort(), headers, TWO_ABANDON_BATCHES); + + assertFalse(response.contains("errorResponse"), response); + + server.awaitDisconnect(2); + assertEquals(server.getReceivedOpTypes(), + list(OP_TYPE_BIND_REQUEST, OP_TYPE_ABANDON_REQUEST, OP_TYPE_UNBIND_REQUEST, + OP_TYPE_BIND_REQUEST, OP_TYPE_ABANDON_REQUEST, OP_TYPE_UNBIND_REQUEST), + "the second batch request was not processed on its own connection"); + } + } + + /** + * The connection options are shared by all the batch requests of a SOAP body, + * and the SASL authzid they carry is single valued: the authzid of a batch + * request must not survive into the bind of the next one. + */ + @Test + public void testAuthzIdIsNotAccumulatedAcrossBatchRequests() throws Exception + { + try (FakeLdapServer server = new FakeLdapServer()) + { + Map params = new LinkedHashMap<>(); + // turn the HTTP credentials into a SASL PLAIN authid, so that the + // authRequest of each batch request becomes an authzid + params.put("ldap.authzidtypeisid", "true"); + + Map headers = new LinkedHashMap<>(); + headers.put("Content-Type", SOAP_1_1_CONTENT_TYPE); + headers.put("Authorization", "Basic " + Base64.getEncoder() + .encodeToString("user:password".getBytes(StandardCharsets.UTF_8))); + + String response = doPost(server.getPort(), params, headers, TWO_AUTHZ_BATCHES); + + assertFalse(response.contains("errorResponse"), response); + + server.awaitDisconnect(2); + assertEquals(server.getReceivedOpTypes(), + list(OP_TYPE_BIND_REQUEST, OP_TYPE_ABANDON_REQUEST, OP_TYPE_UNBIND_REQUEST, + OP_TYPE_BIND_REQUEST, OP_TYPE_ABANDON_REQUEST, OP_TYPE_UNBIND_REQUEST), + "the bind of the second batch request did not happen"); + } + } + /** Runs {@code doPost} against a servlet configured to use the given LDAP port. */ private String doPost(int ldapPort, Map headers, String body) throws Exception + { + return doPost(ldapPort, Collections. emptyMap(), headers, body); + } + + private String doPost(int ldapPort, Map extraParams, + Map headers, String body) throws Exception { Map params = new LinkedHashMap<>(); params.put("ldap.host", InetAddress.getLoopbackAddress().getHostAddress()); params.put("ldap.port", String.valueOf(ldapPort)); + params.putAll(extraParams); DSMLServlet servlet = new DSMLServlet(); servlet.init(servletConfig(params)); @@ -189,18 +329,22 @@ private static List list(byte... opTypes) /** * A minimal LDAP endpoint which answers the bind request with a success - * result and records the type of every message it receives. + * result and records the type of every message it receives. Connections are + * served one after the other, so that a SOAP body holding several batch + * requests can be exercised. */ private static final class FakeLdapServer implements Closeable { private final ServerSocket serverSocket; private final List receivedOpTypes = new CopyOnWriteArrayList<>(); - private final CountDownLatch disconnected = new CountDownLatch(1); + private final Object lock = new Object(); + private int closedConnections; + private volatile Exception failure; private volatile boolean stopped; FakeLdapServer() throws IOException { - serverSocket = new ServerSocket(0, 1, InetAddress.getLoopbackAddress()); + serverSocket = new ServerSocket(0, 16, InetAddress.getLoopbackAddress()); Thread thread = new Thread(this::serve, "fake-ldap-server"); thread.setDaemon(true); thread.start(); @@ -218,36 +362,84 @@ List getReceivedOpTypes() void awaitDisconnect() throws InterruptedException { - assertTrue(disconnected.await(30, TimeUnit.SECONDS), "the client did not disconnect"); + awaitDisconnect(1); + } + + /** Waits for the given number of connections to have been served. */ + void awaitDisconnect(int expectedConnections) throws InterruptedException + { + final long deadline = System.currentTimeMillis() + + TimeUnit.SECONDS.toMillis(30); + synchronized (lock) + { + while (closedConnections < expectedConnections) + { + final long remaining = deadline - System.currentTimeMillis(); + assertTrue(remaining > 0, "the client did not disconnect: " + + closedConnections + " connection(s) served out of " + expectedConnections); + lock.wait(remaining); + } + } + assertNull(failure, "the fake LDAP server failed: " + failure); } private void serve() { - try (Socket socket = serverSocket.accept()) + while (!stopped) { - LDAPReader reader = new LDAPReader(socket); - LDAPWriter writer = new LDAPWriter(socket); - LDAPMessage message; - while ((message = reader.readMessage()) != null) + final Socket socket; + try { - receivedOpTypes.add(message.getProtocolOpType()); - if (message.getProtocolOpType() == OP_TYPE_BIND_REQUEST) + socket = serverSocket.accept(); + } + catch (IOException e) + { + if (!stopped) + { + recordFailure(e); + } + return; + } + try (Socket connection = socket) + { + serveConnection(connection); + } + catch (Exception e) + { + recordFailure(e); + } + finally + { + synchronized (lock) { - writer.writeMessage(new LDAPMessage(message.getMessageID(), - new BindResponseProtocolOp(LDAPResultCode.SUCCESS))); + closedConnections++; + lock.notifyAll(); } } } - catch (Exception e) + } + + private void serveConnection(Socket socket) throws Exception + { + LDAPReader reader = new LDAPReader(socket); + LDAPWriter writer = new LDAPWriter(socket); + LDAPMessage message; + while ((message = reader.readMessage()) != null) { - if (!stopped) + receivedOpTypes.add(message.getProtocolOpType()); + if (message.getProtocolOpType() == OP_TYPE_BIND_REQUEST) { - e.printStackTrace(); + writer.writeMessage(new LDAPMessage(message.getMessageID(), + new BindResponseProtocolOp(LDAPResultCode.SUCCESS))); } } - finally + } + + private void recordFailure(Exception e) + { + if (!stopped && failure == null) { - disconnected.countDown(); + failure = e; } } @@ -352,6 +544,11 @@ private static T stub(Class type, InvocationHandler handler) DSMLServletTestCase.class.getClassLoader(), new Class[] { type }, handler)); } + /** + * A proxy must return a value assignable to the return type of the invoked + * method: {@code null} is only acceptable for a reference or {@code void} + * return type, so every primitive has to be covered here. + */ private static Object defaultValue(Method method) { Class returnType = method.getReturnType(); @@ -359,6 +556,18 @@ private static Object defaultValue(Method method) { return Boolean.FALSE; } + else if (returnType == char.class) + { + return (char) 0; + } + else if (returnType == byte.class) + { + return (byte) 0; + } + else if (returnType == short.class) + { + return (short) 0; + } else if (returnType == int.class) { return 0; @@ -367,6 +576,14 @@ else if (returnType == long.class) { return 0L; } + else if (returnType == float.class) + { + return 0f; + } + else if (returnType == double.class) + { + return 0d; + } else if ("toString".equals(method.getName())) { return "stub"; diff --git a/opendj-server-legacy/src/test/java/org/opends/server/tools/LDAPConnectionTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/tools/LDAPConnectionTestCase.java new file mode 100644 index 0000000000..b739fd4ccc --- /dev/null +++ b/opendj-server-legacy/src/test/java/org/opends/server/tools/LDAPConnectionTestCase.java @@ -0,0 +1,96 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.opends.server.tools; + +import static org.opends.server.protocols.ldap.LDAPResultCode.CLIENT_SIDE_CONNECT_ERROR; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.fail; + +import java.net.InetAddress; +import java.util.concurrent.atomic.AtomicInteger; + +import org.opends.server.DirectoryServerTestCase; +import org.opends.server.TestCaseUtils; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +/** + * Tests the plain (neither SSL nor StartTLS) connection path of + * {@link LDAPConnection}, which is the one used by the DSML gateway and by the + * tools built on {@code LDAPConnectionArgumentParser}. Those are the only + * callers reaching {@code createSocket()}: a caller which installs an + * {@code SSLConnectionFactory} goes through {@code createSSLSocket()} instead. + */ +@SuppressWarnings("javadoc") +@Test(groups = { "precommit", "tools" }, sequential = true) +public class LDAPConnectionTestCase extends DirectoryServerTestCase +{ + @BeforeClass + public void startServer() throws Exception + { + TestCaseUtils.startServer(); + } + + /** + * The socket must be connected to the directory server: binding it to the + * server address instead makes every plain connection fail with + * "Address already in use". + */ + @Test + public void testConnectToHostConnectsThePlainSocket() throws Exception + { + LDAPConnection connection = new LDAPConnection( + InetAddress.getLoopbackAddress().getHostAddress(), + TestCaseUtils.getServerLdapPort(), new LDAPConnectionOptions()); + try + { + connection.connectToHost("cn=Directory Manager", "password"); + assertNotNull(connection.getLDAPReader(), "the connection was not established"); + assertNotNull(connection.getLDAPWriter(), "the connection was not established"); + } + finally + { + connection.close(new AtomicInteger(1)); + } + } + + /** + * A port with nothing behind it must be reported as a connect error: it is + * the {@code ConnectException} of each candidate address which drives the + * failover of {@code createSocket()}. + */ + @Test + public void testConnectToClosedPortIsAConnectError() throws Exception + { + LDAPConnection connection = new LDAPConnection( + InetAddress.getLoopbackAddress().getHostAddress(), + TestCaseUtils.findFreePort(), new LDAPConnectionOptions()); + try + { + connection.connectToHost("cn=Directory Manager", "password"); + fail("connecting to a closed port should have failed"); + } + catch (LDAPConnectionException e) + { + assertEquals(e.getResultCode(), CLIENT_SIDE_CONNECT_ERROR, String.valueOf(e)); + } + finally + { + connection.close(new AtomicInteger(1)); + } + } +} From 9ddd7da1465b4567f63d324cad40d2eb86aa65d4 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Mon, 3 Aug 2026 14:56:44 +0300 Subject: [PATCH 4/4] [#809] Clear the SASL authzid for every batch request of a SOAP body The connection options are built once per doPost() and shared by all the batch requests of the SOAP body, but the authzid was dropped only when the next batch request carried an authRequest of its own. A body whose first batch request asks for an authorization identity and whose second does not left the first authzid in the options, so the operations of the second one ran under an identity the request never asked for. It is gated on ldap.authzidtypeisid=true, which the shipped web.xml leaves at false, and still subject to the proxied-auth privileges of the server. The clearing now happens at the top of every iteration, before the authRequest is looked at. DSMLServletTestCase records the authorization identity of every SASL bind at the fake endpoint: the existing test now asserts the identities themselves instead of the mere absence of an error, and a new one pins the mixed body, where the second batch request must bind with no authzid at all. The four remaining Logger.getLogger(PKG_NAME) calls are replaced by getServletContext().log(), so the class has a single logging sink: they were dead for the reason already documented for the response path, which moves to the class javadoc. --- .../org/opends/dsml/protocol/DSMLServlet.java | 33 +++---- .../dsml/protocol/DSMLServletTestCase.java | 90 ++++++++++++++++--- 2 files changed, 94 insertions(+), 29 deletions(-) diff --git a/opendj-dsml-servlet/src/main/java/org/opends/dsml/protocol/DSMLServlet.java b/opendj-dsml-servlet/src/main/java/org/opends/dsml/protocol/DSMLServlet.java index 0407fb753f..8163dba18c 100644 --- a/opendj-dsml-servlet/src/main/java/org/opends/dsml/protocol/DSMLServlet.java +++ b/opendj-dsml-servlet/src/main/java/org/opends/dsml/protocol/DSMLServlet.java @@ -42,8 +42,6 @@ import java.util.StringTokenizer; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; -import java.util.logging.Level; -import java.util.logging.Logger; import jakarta.servlet.ServletConfig; import jakarta.servlet.ServletException; @@ -106,6 +104,12 @@ * It parses the SOAP request, calls the appropriate class * which performs the LDAP operation, and returns the response * as a DSML response. + *

+ * Everything is logged through {@code getServletContext().log()}: it is the + * only sink which survives at runtime, as + * {@code LDAPConnection.connectToHost()} turns {@code java.util.logging} off + * for the whole JVM on every non-verbose connection, and the war ships + * {@code slf4j-api} without any provider. */ public class DSMLServlet extends HttpServlet { private static final String PKG_NAME = "org.opends.dsml.protocol"; @@ -548,17 +552,19 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) boolean authzInControl = false; batchRequest = batchRequestElement.getValue(); + // The connection options are shared by all the batch requests of this + // SOAP body, so the authzid of the previous one must not survive into + // the bind of this one: it would run under an authorization identity + // it never asked for, and addSASLProperty() appends to the values of + // a key, which SASL PLAIN rejects as a multi-valued authzid. + connOptions.getSASLProperties().remove("authzid"); + /* * Process optional authRequest (i.e. use authz) */ if (batchRequest.authRequest != null) { if (authenticationIsID) { - // If we are using SASL, then use the bind authz. The options are - // shared by all the batch requests of this SOAP body, and - // addSASLProperty() appends to the values of a key: drop the - // authzid of the previous batch request, as SASL PLAIN rejects a - // multi-valued one. - connOptions.getSASLProperties().remove("authzid"); + // If we are using SASL, then use the bind authz. connOptions.addSASLProperty("authzid=" + batchRequest.authRequest.getPrincipal()); authzInBind = true; @@ -642,9 +648,6 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) sendResponse(doc, messageFactory, messageContentType, res); } catch (Exception e) { // The client gets an empty response: at least make the cause visible. - // The container log is the only usable sink here, as connectToHost() - // turns java.util.logging off for the whole JVM and the war ships no - // SLF4J binding. getServletContext().log("Unable to send the DSML response", e); } @@ -669,14 +672,14 @@ private void safeSetFeature(XMLReader xmlReader, String feature, boolean flag) { if (logFeatureWarnings.compareAndSet(false, true)) { - Logger.getLogger(PKG_NAME).log(Level.SEVERE, "XMLReader unsupported feature " + feature); + getServletContext().log("XMLReader unsupported feature " + feature); } } catch (SAXNotRecognizedException e) { if (logFeatureWarnings.compareAndSet(false, true)) { - Logger.getLogger(PKG_NAME).log(Level.SEVERE, "XMLReader unrecognized feature " + feature); + getServletContext().log("XMLReader unrecognized feature " + feature); } } } @@ -934,7 +937,7 @@ private void safeSetFeature(DocumentBuilderFactory factory, String feature, bool catch (ParserConfigurationException e) { if (logFeatureWarnings.compareAndSet(false, true)) { - Logger.getLogger(PKG_NAME).log(Level.SEVERE, "DocumentBuilderFactory unsupported feature " + feature); + getServletContext().log("DocumentBuilderFactory unsupported feature " + feature); } } } @@ -957,7 +960,7 @@ private Document createSafeDocument() catch (ParserConfigurationException e) { if (logFeatureWarnings.compareAndSet(false, true)) { - Logger.getLogger(PKG_NAME).log(Level.SEVERE, "DocumentBuilderFactory cannot be configured securely"); + getServletContext().log("DocumentBuilderFactory cannot be configured securely"); } } dbf.setXIncludeAware(false); diff --git a/opendj-dsml-servlet/src/test/java/org/opends/dsml/protocol/DSMLServletTestCase.java b/opendj-dsml-servlet/src/test/java/org/opends/dsml/protocol/DSMLServletTestCase.java index 8d41ddb65e..d31a6e560e 100644 --- a/opendj-dsml-servlet/src/test/java/org/opends/dsml/protocol/DSMLServletTestCase.java +++ b/opendj-dsml-servlet/src/test/java/org/opends/dsml/protocol/DSMLServletTestCase.java @@ -15,6 +15,7 @@ */ package org.opends.dsml.protocol; +import static java.util.Arrays.asList; import static org.opends.server.protocols.ldap.LDAPConstants.OP_TYPE_ABANDON_REQUEST; import static org.opends.server.protocols.ldap.LDAPConstants.OP_TYPE_BIND_REQUEST; import static org.opends.server.protocols.ldap.LDAPConstants.OP_TYPE_UNBIND_REQUEST; @@ -52,7 +53,9 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.forgerock.opendj.ldap.ByteString; import org.forgerock.testng.ForgeRockTestCase; +import org.opends.server.protocols.ldap.BindRequestProtocolOp; import org.opends.server.protocols.ldap.BindResponseProtocolOp; import org.opends.server.protocols.ldap.LDAPMessage; import org.opends.server.protocols.ldap.LDAPResultCode; @@ -89,6 +92,10 @@ public class DSMLServletTestCase extends ForgeRockTestCase private static final String TWO_AUTHZ_BATCHES = soap11(abandonBatch("1", "dn:cn=first") + abandonBatch("2", "dn:cn=second")); + /** Same, but only the first batch request asks for an authorization identity. */ + private static final String MIXED_AUTHZ_BATCHES = + soap11(abandonBatch("1", "dn:cn=first") + abandonBatch("2", null)); + private static String abandonBatch(String requestID, String authzPrincipal) { return "" @@ -273,17 +280,7 @@ public void testAuthzIdIsNotAccumulatedAcrossBatchRequests() throws Exception { try (FakeLdapServer server = new FakeLdapServer()) { - Map params = new LinkedHashMap<>(); - // turn the HTTP credentials into a SASL PLAIN authid, so that the - // authRequest of each batch request becomes an authzid - params.put("ldap.authzidtypeisid", "true"); - - Map headers = new LinkedHashMap<>(); - headers.put("Content-Type", SOAP_1_1_CONTENT_TYPE); - headers.put("Authorization", "Basic " + Base64.getEncoder() - .encodeToString("user:password".getBytes(StandardCharsets.UTF_8))); - - String response = doPost(server.getPort(), params, headers, TWO_AUTHZ_BATCHES); + String response = doAuthzPost(server, TWO_AUTHZ_BATCHES); assertFalse(response.contains("errorResponse"), response); @@ -292,9 +289,48 @@ public void testAuthzIdIsNotAccumulatedAcrossBatchRequests() throws Exception list(OP_TYPE_BIND_REQUEST, OP_TYPE_ABANDON_REQUEST, OP_TYPE_UNBIND_REQUEST, OP_TYPE_BIND_REQUEST, OP_TYPE_ABANDON_REQUEST, OP_TYPE_UNBIND_REQUEST), "the bind of the second batch request did not happen"); + assertEquals(server.getReceivedAuthzIds(), asList("dn:cn=first", "dn:cn=second"), + "each batch request must bind under the authzid of its own authRequest"); } } + /** + * A batch request which carries no authRequest must not inherit the + * authorization identity of the previous one: the shared connection options + * have to be cleared whether or not this batch request sets an authzid. + */ + @Test + public void testAuthzIdDoesNotSurviveIntoBatchRequestWithoutAuthRequest() throws Exception + { + try (FakeLdapServer server = new FakeLdapServer()) + { + String response = doAuthzPost(server, MIXED_AUTHZ_BATCHES); + + assertFalse(response.contains("errorResponse"), response); + + server.awaitDisconnect(2); + assertEquals(server.getReceivedAuthzIds(), asList("dn:cn=first", ""), + "the second batch request ran under the authorization identity of the first one"); + } + } + + /** + * Posts the given SOAP body with HTTP credentials turned into a SASL PLAIN + * authid, so that the authRequest of a batch request becomes an authzid. + */ + private String doAuthzPost(FakeLdapServer server, String body) throws Exception + { + Map params = new LinkedHashMap<>(); + params.put("ldap.authzidtypeisid", "true"); + + Map headers = new LinkedHashMap<>(); + headers.put("Content-Type", SOAP_1_1_CONTENT_TYPE); + headers.put("Authorization", "Basic " + Base64.getEncoder() + .encodeToString("user:password".getBytes(StandardCharsets.UTF_8))); + + return doPost(server.getPort(), params, headers, body); + } + /** Runs {@code doPost} against a servlet configured to use the given LDAP port. */ private String doPost(int ldapPort, Map headers, String body) throws Exception { @@ -329,14 +365,16 @@ private static List list(byte... opTypes) /** * A minimal LDAP endpoint which answers the bind request with a success - * result and records the type of every message it receives. Connections are - * served one after the other, so that a SOAP body holding several batch - * requests can be exercised. + * result and records the type of every message it receives, as well as the + * authorization identity of every SASL bind. Connections are served one after + * the other, so that a SOAP body holding several batch requests can be + * exercised. */ private static final class FakeLdapServer implements Closeable { private final ServerSocket serverSocket; private final List receivedOpTypes = new CopyOnWriteArrayList<>(); + private final List receivedAuthzIds = new CopyOnWriteArrayList<>(); private final Object lock = new Object(); private int closedConnections; private volatile Exception failure; @@ -360,6 +398,12 @@ List getReceivedOpTypes() return new ArrayList<>(receivedOpTypes); } + /** The authorization identity of every SASL bind, in the order received. */ + List getReceivedAuthzIds() + { + return new ArrayList<>(receivedAuthzIds); + } + void awaitDisconnect() throws InterruptedException { awaitDisconnect(1); @@ -429,12 +473,30 @@ private void serveConnection(Socket socket) throws Exception receivedOpTypes.add(message.getProtocolOpType()); if (message.getProtocolOpType() == OP_TYPE_BIND_REQUEST) { + recordAuthzId(message.getBindRequestProtocolOp()); writer.writeMessage(new LDAPMessage(message.getMessageID(), new BindResponseProtocolOp(LDAPResultCode.SUCCESS))); } } } + /** + * Records the authorization identity of a SASL bind. The credentials of + * SASL PLAIN are "authzid NUL authid NUL password", with an empty authzid + * when the client asked for none. + */ + private void recordAuthzId(BindRequestProtocolOp bindRequest) + { + ByteString credentials = bindRequest.getSASLCredentials(); + if (credentials == null) + { + return; + } + String plain = credentials.toString(); + int separator = plain.indexOf('\0'); + receivedAuthzIds.add(separator >= 0 ? plain.substring(0, separator) : plain); + } + private void recordFailure(Exception e) { if (!stopped && failure == null)