From 63fe226eb08b1ab975d8e8678210e5086c48ac6e Mon Sep 17 00:00:00 2001 From: Robert Schlabbach Date: Fri, 28 Jun 2024 17:24:32 +0200 Subject: [PATCH 1/2] Revert commits breaking API and binary OCPP 1.6 passwords These commits were based on a misunderstanding of the OCPP-J 1.6 specification, which clearly states that the password is a byte sequence and not a string: Revert "More password fixes." This reverts commit fb5d0fb4c9e5cf8e1f14ab97ea5e15422fb441f4. Revert "Fix password decoding." This reverts commit cf20205e849a258ee5f4893a1d916ef8091f12d1. Revert "Recommended by 1.6 spec is a 20 byte (40 chars) key." This reverts commit f7b92a3d6ae51cce4e6bbdcc25148cb951e986d1. This commit breaks the API, because the behaviour of the method is changed to return the last configuration instead of the default configuration: Revert "A single instace, otherwise a static get() method makes no sense." This reverts commit 953f50b3e7c4b975cae5ebcb74c2bd6e381d5e23. --- .../java/eu/chargetime/ocpp/JSONConfiguration.java | 4 +--- .../java/eu/chargetime/ocpp/WebSocketListener.java | 10 +++++----- .../main/java/eu/chargetime/ocpp/ListenerEvents.java | 2 +- .../src/main/java/eu/chargetime/ocpp/Server.java | 2 +- .../main/java/eu/chargetime/ocpp/ServerEvents.java | 2 +- .../java/eu/chargetime/ocpp/test/DummyHandlers.java | 2 +- .../ocpp/MultiProtocolWebSocketListener.java | 12 ++++++------ .../eu/chargetime/ocpp/test/FakeCentralSystem.java | 2 +- 8 files changed, 17 insertions(+), 19 deletions(-) diff --git a/OCPP-J/src/main/java/eu/chargetime/ocpp/JSONConfiguration.java b/OCPP-J/src/main/java/eu/chargetime/ocpp/JSONConfiguration.java index acee0395c..782550433 100644 --- a/OCPP-J/src/main/java/eu/chargetime/ocpp/JSONConfiguration.java +++ b/OCPP-J/src/main/java/eu/chargetime/ocpp/JSONConfiguration.java @@ -49,10 +49,8 @@ public class JSONConfiguration { private JSONConfiguration() {} - private static final JSONConfiguration instance = new JSONConfiguration(); - public static JSONConfiguration get() { - return instance; + return new JSONConfiguration(); } public JSONConfiguration setParameter(String name, T value) { diff --git a/OCPP-J/src/main/java/eu/chargetime/ocpp/WebSocketListener.java b/OCPP-J/src/main/java/eu/chargetime/ocpp/WebSocketListener.java index 9f026cbbd..5eb06e667 100644 --- a/OCPP-J/src/main/java/eu/chargetime/ocpp/WebSocketListener.java +++ b/OCPP-J/src/main/java/eu/chargetime/ocpp/WebSocketListener.java @@ -51,7 +51,7 @@ public class WebSocketListener implements Listener { private static final int TIMEOUT_IN_MILLIS = 10000; private static final int OCPPJ_CP_MIN_PASSWORD_LENGTH = 16; - private static final int OCPPJ_CP_MAX_PASSWORD_LENGTH = 40; + private static final int OCPPJ_CP_MAX_PASSWORD_LENGTH = 20; private static final String HTTP_HEADER_PROXIED_ADDRESS = "X-Forwarded-For"; @@ -146,7 +146,7 @@ public ServerHandshakeBuilder onWebsocketHandshakeReceivedAsServer( .build(); String username = null; - String password = null; + byte[] password = null; if (clientHandshake.hasFieldValue("Authorization")) { String authorization = clientHandshake.getFieldValue("Authorization"); if (authorization != null && authorization.toLowerCase().startsWith("basic")) { @@ -159,15 +159,15 @@ public ServerHandshakeBuilder onWebsocketHandshakeReceivedAsServer( username = new String(Arrays.copyOfRange(credDecoded, 0, i), StandardCharsets.UTF_8); if (i + 1 < credDecoded.length) { - password = new String(Arrays.copyOfRange(credDecoded, i + 1, credDecoded.length)); + password = Arrays.copyOfRange(credDecoded, i + 1, credDecoded.length); } break; } } } if (password == null - || password.length() < configuration.getParameter(JSONConfiguration.OCPPJ_CP_MIN_PASSWORD_LENGTH, OCPPJ_CP_MIN_PASSWORD_LENGTH) - || password.length() > configuration.getParameter(JSONConfiguration.OCPPJ_CP_MAX_PASSWORD_LENGTH, OCPPJ_CP_MAX_PASSWORD_LENGTH)) + || password.length < configuration.getParameter(JSONConfiguration.OCPPJ_CP_MIN_PASSWORD_LENGTH, OCPPJ_CP_MIN_PASSWORD_LENGTH) + || password.length > configuration.getParameter(JSONConfiguration.OCPPJ_CP_MAX_PASSWORD_LENGTH, OCPPJ_CP_MAX_PASSWORD_LENGTH)) throw new InvalidDataException(401, "Invalid password length"); } diff --git a/ocpp-common/src/main/java/eu/chargetime/ocpp/ListenerEvents.java b/ocpp-common/src/main/java/eu/chargetime/ocpp/ListenerEvents.java index a40da3fab..3e8be78d0 100644 --- a/ocpp-common/src/main/java/eu/chargetime/ocpp/ListenerEvents.java +++ b/ocpp-common/src/main/java/eu/chargetime/ocpp/ListenerEvents.java @@ -28,7 +28,7 @@ of this software and associated documentation files (the "Software"), to deal import eu.chargetime.ocpp.model.SessionInformation; public interface ListenerEvents { - void authenticateSession(SessionInformation information, String username, String password) + void authenticateSession(SessionInformation information, String username, byte[] password) throws AuthenticationException; void newSession(ISession session, SessionInformation information); diff --git a/ocpp-common/src/main/java/eu/chargetime/ocpp/Server.java b/ocpp-common/src/main/java/eu/chargetime/ocpp/Server.java index d80171161..d62abe934 100644 --- a/ocpp-common/src/main/java/eu/chargetime/ocpp/Server.java +++ b/ocpp-common/src/main/java/eu/chargetime/ocpp/Server.java @@ -81,7 +81,7 @@ public void open(String hostname, int port, ServerEvents serverEvents) { @Override public void authenticateSession( - SessionInformation information, String username, String password) + SessionInformation information, String username, byte[] password) throws AuthenticationException { serverEvents.authenticateSession(information, username, password); } diff --git a/ocpp-common/src/main/java/eu/chargetime/ocpp/ServerEvents.java b/ocpp-common/src/main/java/eu/chargetime/ocpp/ServerEvents.java index 593778d7d..2e66f0f30 100644 --- a/ocpp-common/src/main/java/eu/chargetime/ocpp/ServerEvents.java +++ b/ocpp-common/src/main/java/eu/chargetime/ocpp/ServerEvents.java @@ -29,7 +29,7 @@ of this software and associated documentation files (the "Software"), to deal import java.util.UUID; public interface ServerEvents { - void authenticateSession(SessionInformation information, String username, String password) throws AuthenticationException; + void authenticateSession(SessionInformation information, String username, byte[] password) throws AuthenticationException; void newSession(UUID sessionIndex, SessionInformation information); diff --git a/ocpp-v1_6-test/src/main/java/eu/chargetime/ocpp/test/DummyHandlers.java b/ocpp-v1_6-test/src/main/java/eu/chargetime/ocpp/test/DummyHandlers.java index 7db86d059..0ec7e8509 100644 --- a/ocpp-v1_6-test/src/main/java/eu/chargetime/ocpp/test/DummyHandlers.java +++ b/ocpp-v1_6-test/src/main/java/eu/chargetime/ocpp/test/DummyHandlers.java @@ -203,7 +203,7 @@ public ServerEvents generateServerEventsHandler() { return new ServerEvents() { @Override public void authenticateSession( - SessionInformation information, String username, String password) throws AuthenticationException {} + SessionInformation information, String username, byte[] password) throws AuthenticationException {} @Override public void newSession(UUID sessionIndex, SessionInformation information) { diff --git a/ocpp-v2/src/main/java/eu/chargetime/ocpp/MultiProtocolWebSocketListener.java b/ocpp-v2/src/main/java/eu/chargetime/ocpp/MultiProtocolWebSocketListener.java index 0b0762f1e..47c37fe45 100644 --- a/ocpp-v2/src/main/java/eu/chargetime/ocpp/MultiProtocolWebSocketListener.java +++ b/ocpp-v2/src/main/java/eu/chargetime/ocpp/MultiProtocolWebSocketListener.java @@ -165,7 +165,7 @@ public ServerHandshakeBuilder onWebsocketHandshakeReceivedAsServer( .build(); String username = null; - String password = null; + byte[] password = null; if (clientHandshake.hasFieldValue("Authorization")) { String authorization = clientHandshake.getFieldValue("Authorization"); if (authorization != null && authorization.toLowerCase().startsWith("basic")) { @@ -178,7 +178,7 @@ public ServerHandshakeBuilder onWebsocketHandshakeReceivedAsServer( username = new String(Arrays.copyOfRange(credDecoded, 0, i), StandardCharsets.UTF_8); if (i + 1 < credDecoded.length) { - password = new String(Arrays.copyOfRange(credDecoded, i + 1, credDecoded.length)); + password = Arrays.copyOfRange(credDecoded, i + 1, credDecoded.length); } break; } @@ -186,13 +186,13 @@ public ServerHandshakeBuilder onWebsocketHandshakeReceivedAsServer( } if (protocolVersion == null || protocolVersion == ProtocolVersion.OCPP1_6) { if (password == null - || password.length() < configuration.getParameter(JSONConfiguration.OCPPJ_CP_MIN_PASSWORD_LENGTH, OCPPJ_CP_MIN_PASSWORD_LENGTH) - || password.length() > configuration.getParameter(JSONConfiguration.OCPPJ_CP_MAX_PASSWORD_LENGTH, OCPPJ_CP_MAX_PASSWORD_LENGTH)) + || password.length < configuration.getParameter(JSONConfiguration.OCPPJ_CP_MIN_PASSWORD_LENGTH, OCPPJ_CP_MIN_PASSWORD_LENGTH) + || password.length > configuration.getParameter(JSONConfiguration.OCPPJ_CP_MAX_PASSWORD_LENGTH, OCPPJ_CP_MAX_PASSWORD_LENGTH)) throw new InvalidDataException(401, "Invalid password length"); } else { if (password == null - || password.length() < configuration.getParameter(JSONConfiguration.OCPP2J_CP_MIN_PASSWORD_LENGTH, OCPP2J_CP_MIN_PASSWORD_LENGTH) - || password.length() > configuration.getParameter(JSONConfiguration.OCPP2J_CP_MAX_PASSWORD_LENGTH, OCPP2J_CP_MAX_PASSWORD_LENGTH)) + || password.length < configuration.getParameter(JSONConfiguration.OCPP2J_CP_MIN_PASSWORD_LENGTH, OCPP2J_CP_MIN_PASSWORD_LENGTH) + || password.length > configuration.getParameter(JSONConfiguration.OCPP2J_CP_MAX_PASSWORD_LENGTH, OCPP2J_CP_MAX_PASSWORD_LENGTH)) throw new InvalidDataException(401, "Invalid password length"); } } diff --git a/ocpp-v2_0-test/src/main/java/eu/chargetime/ocpp/test/FakeCentralSystem.java b/ocpp-v2_0-test/src/main/java/eu/chargetime/ocpp/test/FakeCentralSystem.java index fdcac5805..25cae3629 100644 --- a/ocpp-v2_0-test/src/main/java/eu/chargetime/ocpp/test/FakeCentralSystem.java +++ b/ocpp-v2_0-test/src/main/java/eu/chargetime/ocpp/test/FakeCentralSystem.java @@ -74,7 +74,7 @@ public void started() throws Exception { new ServerEvents() { @Override public void authenticateSession( - SessionInformation information, String username, String password) throws AuthenticationException {} + SessionInformation information, String username, byte[] password) throws AuthenticationException {} @Override public void newSession(UUID sessionIndex, SessionInformation information) { From cac89cb3e21a4bf3b4408fb544f449d785e03683 Mon Sep 17 00:00:00 2001 From: Robert Schlabbach Date: Sat, 29 Jun 2024 01:14:06 +0200 Subject: [PATCH 2/2] Implement CALLERROR: SecurityError response OCPP 2.0.1 requires a CALLERROR: SecurityError response from the CSMS if the Charging Station sends a request before having been accepted. Implement a mechanism to send this response, by throwing the newly added SecurityErrorException in the message handler of a disallowed request. This fixes issue #350. --- .../ocpp/SecurityErrorException.java | 35 +++++++++++++++++++ .../main/java/eu/chargetime/ocpp/Session.java | 3 ++ 2 files changed, 38 insertions(+) create mode 100644 ocpp-common/src/main/java/eu/chargetime/ocpp/SecurityErrorException.java diff --git a/ocpp-common/src/main/java/eu/chargetime/ocpp/SecurityErrorException.java b/ocpp-common/src/main/java/eu/chargetime/ocpp/SecurityErrorException.java new file mode 100644 index 000000000..e7450b0a2 --- /dev/null +++ b/ocpp-common/src/main/java/eu/chargetime/ocpp/SecurityErrorException.java @@ -0,0 +1,35 @@ +package eu.chargetime.ocpp; + +/* +ChargeTime.eu - Java-OCA-OCPP +Copyright (C) 2024 Robert Schlabbach + +MIT License + +Copyright (C) 2024 Robert Schlabbach + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. +*/ + +/** A security issue occurred */ +public class SecurityErrorException extends IllegalStateException { + public SecurityErrorException(String s) { + super(s); + } +} diff --git a/ocpp-common/src/main/java/eu/chargetime/ocpp/Session.java b/ocpp-common/src/main/java/eu/chargetime/ocpp/Session.java index 1c3294b94..41a1a6472 100644 --- a/ocpp-common/src/main/java/eu/chargetime/ocpp/Session.java +++ b/ocpp-common/src/main/java/eu/chargetime/ocpp/Session.java @@ -228,6 +228,9 @@ public synchronized void onCall(String id, String action, Object payload) { } catch (PropertyConstraintException ex) { logger.warn(ex.getMessage(), ex); communicator.sendCallError(id, action, "TypeConstraintViolation", ex.getMessage()); + } catch (SecurityErrorException ex) { + logger.warn(ex.getMessage(), ex); + communicator.sendCallError(id, action, "SecurityError", ex.getMessage()); } catch (Exception ex) { logger.warn(UNABLE_TO_PROCESS, ex); communicator.sendCallError(id, action, isLegacyRPC() ? "FormationViolation" :