Implement OCPP 1.6 Security #1854

juherr · 2025-10-19T09:04:59Z

@florinmandache I built on your work and made the minimal required changes.
Some tests would be very welcome. I also noticed that CSMS security requests aren’t implemented yet.

@goekay It seems to be working (at least it starts correctly), but I haven’t fully tested it yet and it could use some polishing.
Feel free to rework or improve it as you see fit.

Fix #100
Fix #1353

goekay · 2025-10-20T08:04:10Z

@juherr i will take a look at this ASAP when i am finished with some other duties.

on a more general note: i had a conversation with @florinmandache after his initial big PR. he said that he has many things going on and is short on time, and he wanted to contribute on good will. since he is a busy person, he was not interested in follow-up, review rounds and eventual clean-ups (i.e. the usual process of PR and reviewing). due to the substantial nature of the contribution, i accepted it as-is and took it upon me to do whatever is necessary to bring these features to main branch.

in this context, i thank you for doing one part of it.

juherr · 2025-10-20T08:23:56Z

@goekay Thanks for the clarification. I think Gemini did an interesting initial job exploring the different topics, but it definitely needs more work based on what I’ve seen so far (especially regarding OCPP 1.6 Security and OICP). Since OCPI and OCPP 2.x are an even tougher challenge, and my time is just as limited, I’ll let you take the first shot on those ones 😉

# Conflicts: # src/main/java/de/rwth/idsg/steve/service/CentralSystemService16_Service.java # src/test/java/de/rwth/idsg/steve/utils/__DatabasePreparer__.java

goekay · 2025-10-31T09:45:45Z

FYI @juherr: steve-community/ocpp-jaxb#18

naming of the json schema files differ a bit from the original (see links from comment). do you know why?

juherr · 2025-10-31T11:34:46Z

Ah, you mean the request suffix in some filenames?
I personally find it cleaner and more robust to change it, but your approach seems to work fine too 👍

* clean up resources folder and pom file * and do necessary changes for project to compile

goekay · 2025-11-02T11:40:37Z

i did some "low-hanging fruit" refactoring to reduce the size of the PR (i.e. moving the data models and their generation to ocpp-jaxb library). will do some more. i think this is very important in order to able to properly review the PR and separate the good parts from inaccurate AI slop. while being on this...

i did a litmus test and tried to understand how "http basic auth" (security profile 1) is supposed to work in the impl. ChargePointRepository has 2 new methods isRegistered(String chargeBoxId) and validatePassword(String chargeBoxId, String password), but no one is using them.

moreover, there is no change in SecurityConfiguration or OcppWebSocketHandshakeHandler (more emphasis on this), such that the handler retrieves the basic auth header from request and validates the password against the DB.

* and also make getRegistrationStatus return the password additionally, in order prevent another DB lookup later

goekay · 2025-11-04T14:00:48Z

WIP - Implementation status of new operations:

Operation	Initiator	Request Handling	Response Handling
CertificateSigned	Central System	✅ (no Web UI)	✅
DeleteCertificate	Central System	✅ (UI via data table)	✅
ExtendedTriggerMessage	Central System	✅	✅
GetInstalledCertificateIds	Central System	✅	✅
GetLog	Central System	✅	✅
InstallCertificate	Central System	✅	✅
LogStatusNotification	Charge Point	✅	✅
SecurityEventNotification	Charge Point	✅	✅
SignCertificate	Charge Point	✅	✅
SignedFirmwareStatusNotification	Charge Point	✅	✅
SignedUpdateFirmware	Central System	✅	✅

Other ToDos:

goekay · 2025-11-05T13:07:19Z

ignoring and removing the following method completely, since it has no correlation with reality IMO:

    private static String determineSeverity(String eventType) {
        if (eventType == null) {
            return "INFO";
        }

        var upperType = eventType.toUpperCase();

        if (upperType.contains("ATTACK") || upperType.contains("BREACH") || upperType.contains("TAMPER")) {
            return "CRITICAL";
        }

        if (upperType.contains("FAIL") || upperType.contains("ERROR") || upperType.contains("INVALID")
            || upperType.contains("UNAUTHORIZED") || upperType.contains("REJECT")) {
            return "HIGH";
        }

        if (upperType.contains("WARNING") || upperType.contains("EXPIR")) {
            return "MEDIUM";
        }

        return "INFO";
    }

the eventType references type field in SecurityEventNotification message with the following value possibilities:

the table actually continues in the next page with more types, but there is no type with INFO, ATTACK (heh?), BREACH (what?) etc. so this whole thing is made up.

moreover, the PR invents new types (that are not in the list), such as SignCertificateError with a HIGH severity, for ex in

            securityRepository.insertSecurityEvent(
                chargeBoxIdentity,
                "SignCertificateError",
                DateTime.now(),
                "Error signing CSR: " + e.getMessage(),
                "HIGH"
            );

these usages feel also wrong. actually, the motivation is nice: if there is an error during the signing of certificate, create a security event and store it in DB. but, these security events are NOT coming from station, and intermixing these with regular events is problematic IMO.

@juherr do you happen to know whether these are retrofits coming from ocpp 2.x ?

juherr · 2025-11-05T14:27:43Z

@juherr do you happen to know whether these are retrofits coming from ocpp 2.x ?

I haven’t looked deeply into OCPP 2.x or even the current PR once I understood where it originated from.
My assumption is that Gemini applied a pattern from another context by mistake.

According to the whitepaper, only critical events are supposed to be sent by the station — and I believe all of those should indeed be stored.
Happy to help if you can elaborate a bit more on your question.

goekay · 2025-11-05T15:08:27Z

so, i can assume that these additions were a mistake and i can remove them

additional severity column derived from security event type
additional security event inserts (triggered by backend/steve) when local certificate-related operation fails

?

* various simplifications and improvements * certificate is still as is. this is a future TODO

* remove unnecessary and invalid parts

juherr · 2025-11-05T16:38:33Z

In my opinion, security events triggered by the backend are a separate concern, and a warning log should be enough for now.
At the very least, an internal event could be raised so that any fork can decide how it wants to handle it.

juherr

Some remarks after a first review of the changes.
I should be able to make them by myself as it comes from my branch, but I would like your review first.

src/main/java/de/rwth/idsg/steve/ocpp/ChargePointServiceInvokerImpl.java

src/main/java/de/rwth/idsg/steve/service/CentralSystemService16_Service.java

juherr · 2025-11-05T17:54:22Z

src/main/java/de/rwth/idsg/steve/service/CentralSystemService16_Service.java

+        try {
+            if (parameters.getRequestId() == null) {
+                log.warn("No requestId in {}", parameters);
+            } else {
+                securityRepository.insertLogUploadStatus(
+                    chargeBoxIdentity,
+                    parameters.getRequestId(),
+                    parameters.getStatus().value(),
+                    DateTime.now()
+                );
+            }


Suggested change

try {

if (parameters.getRequestId() == null) {

log.warn("No requestId in {}", parameters);

} else {

securityRepository.insertLogUploadStatus(

chargeBoxIdentity,

parameters.getRequestId(),

parameters.getStatus().value(),

DateTime.now()

);

}

if (parameters.getRequestId() == null) {

throw new InvalidArgumentException("RequestId is null");

}

try {

securityRepository.insertLogUploadStatus(

chargeBoxIdentity,

parameters.getRequestId(),

parameters.getStatus().value(),

DateTime.now()

);

}

juherr · 2025-11-05T17:55:58Z

src/main/java/de/rwth/idsg/steve/service/CentralSystemService16_Service.java

+        try {
+            if (parameters.getRequestId() == null) {
+                log.warn("No requestId in {}", parameters);
+            } else {
+                securityRepository.insertFirmwareUpdateStatus(
+                    chargeBoxIdentity,
+                    parameters.getRequestId(),
+                    parameters.getStatus().value(),
+                    DateTime.now()
+                );
+            }


Suggested change

try {

if (parameters.getRequestId() == null) {

log.warn("No requestId in {}", parameters);

} else {

securityRepository.insertFirmwareUpdateStatus(

chargeBoxIdentity,

parameters.getRequestId(),

parameters.getStatus().value(),

DateTime.now()

);

}

if (parameters.getRequestId() == null) {

throw new InvalidArgumentException("RequestId is null");

}

try {

securityRepository.insertFirmwareUpdateStatus(

chargeBoxIdentity,

parameters.getRequestId(),

parameters.getStatus().value(),

DateTime.now()

);

}

juherr · 2025-11-05T17:56:41Z

src/main/java/de/rwth/idsg/steve/service/CertificateSigningService.java

+        String keystorePath = securityConfig.getKeystorePath();
+        String keystorePassword = securityConfig.getKeystorePassword();
+        String keystoreType = securityConfig.getKeystoreType();
+
+        KeyStore keystore = KeyStore.getInstance(keystoreType);
+        try (FileInputStream fis = new FileInputStream(keystorePath)) {
+            keystore.load(fis, keystorePassword.toCharArray());
+        }
+
+        String alias = keystore.aliases().nextElement();


Suggested change

String keystorePath = securityConfig.getKeystorePath();

String keystorePassword = securityConfig.getKeystorePassword();

String keystoreType = securityConfig.getKeystoreType();

KeyStore keystore = KeyStore.getInstance(keystoreType);

try (FileInputStream fis = new FileInputStream(keystorePath)) {

keystore.load(fis, keystorePassword.toCharArray());

}

String alias = keystore.aliases().nextElement();

var keystorePath = securityConfig.getKeystorePath();

var keystorePassword = securityConfig.getKeystorePassword();

var keystoreType = securityConfig.getKeystoreType();

var keystore = KeyStore.getInstance(keystoreType);

try (var fis = new FileInputStream(keystorePath)) {

keystore.load(fis, keystorePassword.toCharArray());

}

var alias = keystore.aliases().nextElement();

src/main/java/de/rwth/idsg/steve/web/dto/ocpp/CertificateSignedParams.java

src/main/java/de/rwth/idsg/steve/web/dto/ocpp/InstallCertificateParams.java

src/main/java/de/rwth/idsg/steve/web/dto/ocpp/SignedUpdateFirmwareParams.java

src/main/webapp/WEB-INF/views/security/events.jsp

goekay · 2025-11-05T21:21:51Z

@juherr pls do not review yet. i did not go over all changes yet. some of your remarks will be obsolete when i finally arrive at these areas to improve. pls see table in #1854 (comment)

…server.Ssl and remove configuration page to render security config details.

design decision: this function will not be triggered over Operations page. there will be a "installed certificates" page under "security management". each row (and therefore cert) will have a delete button.

…ntRepository

…uth password

comment in JSP should not be in final html

... connecting new operations with "events and certificates" pages

goekay · 2025-11-12T08:09:19Z

FYI: the implementation work is finished, with the exception of security profile 3 (mTLS). i need to investigate how to do it.

juherr · 2025-11-12T10:27:32Z

@goekay Congratulations, it looks great! If you're fine with it, I can review the changes now.

Regarding profile 3, please share your thoughts. I’ve had similar reflections on that topic.

goekay · 2025-11-12T11:25:24Z

Congratulations, it looks great! If you're fine with it, I can review the changes now.

yes, please do. when reviewing this, could you focus mainly on the logic, correctness, and functional aspects? i’d like to prevent drifting into stylistic and preference discussions ;)

Regarding profile 3, please share your thoughts. I’ve had similar reflections on that topic.

there is many. as starters: it should actually work with current state of this implementation as long as the user (the user of steve) configures and activates SSL and maintains the certs in truststore in a file. then, spring boot will configure SSL for jetty including the SSLContext using the truststore. then, the underlying standard TLS implementation will handle all mTLS stuff.

but there are many issues with this:

managing truststore (and adding/removing root/CA certs) becomes a process outside of steve. it is not integrated. we need to refer users to terminal tools or other software.
it requires restart of steve whenever something changes.
after cert changes in station (after CertificateSigned etc.), steve cannot seamlessly import the root cert etc. into truststore. it will be an additional external step to update truststore.
mTLS should be mandatory only in profile 3. stations using other profiles should be able to connect without station cert.
what if steve is behind a reverse proxy that handles and terminates TLS? such that steve runs only on HTTP?
truststore as a file on filesystem feels clunky in modern cloud environments

i am thinking about a solution approach that involves following ideas:

truststore certs to be stored in relational DB
the user can see and manage them
certificate-related OCPP operations can insert/remove certs in DB
SSLContext can be recreated/refreshed and attached to jetty during runtime after truststore changes
handling cert validation/cheks in application code conditionally (addresses the points 4 and 5 from above). big issue here: we cannot rely on standard TLS implementation anymore, and need to re-impl some aspects of it which makes me worried due to potential mistakes/errors. i really would like to NOT hand-roll TLS/security stuff.

* remove security impl status doc, since not useful * remove security profiled doc, since moved to wiki: https://github.com/steve-community/steve/wiki/OCPP-1.6J-Security-Configuration

juherr

I didn't review the JSP files.

Aside from a few style inconsistencies (like mixed use of var and occasional silent errors), everything looks good to me.

src/main/java/de/rwth/idsg/steve/ocpp/task/CertificateSignedTask.java

src/main/java/de/rwth/idsg/steve/ocpp/ws/OcppWebSocketHandshakeHandler.java

src/main/java/de/rwth/idsg/steve/web/controller/Ocpp12Controller.java

src/main/java/de/rwth/idsg/steve/web/controller/Ocpp15Controller.java

juherr · 2025-11-12T16:05:06Z

src/main/java/de/rwth/idsg/steve/web/dto/ocpp/ConfigurationKeyEnum.java

+    // -------------------------------------------------------------------------
+
+    AdditionalRootCertificateCheck("boolean", R, newHashSet(V_16)),
+    AuthorizationKey("string", W, newHashSet(V_16)),


Like WebSocketPingInterval , this one is defined in Specification OCPPJ

Like WebSocketPingInterval , this one is defined in Specification OCPPJ

i dont know what to do with this comment.

You may move it before the comment because it was not defined first in security extension.
I need to check it again because it could be an issue in my file.

it was not defined first in security extension.

WebSocketPingInterval was there in ocpp 1.6 spec (no extension). the enum was added years ago at the right place.

AuthorizationKey was added with the ocpp 1.6 security extension. it is there in edition 3 (final). this enum was added in this work at the right place.

AuthorizationKey is referenced in
ocpp-j-1.6-specification - section 6.2.2. Charge point authentication

fair enough. it comes up in text, but not in the list of

"9. Standard Configuration Key Names & Values" section of ocpp-1.6 edition 2.pdf, or

"7. Configuration" section of ocpp-j-1.6-specification.pdf

which i took as basis for the initial ocpp 1.6 impl.

now, it is part of "7. Configuration Keys" of OCPP 1.6 security whitepaper edition 3.pdf which i took as the basis for the security extension impl.

moreover, since we did not initially support basic auth (and AuthorizationKey) and this is such a bikeshedding topic, i have no problem having it at the current place of the code.

Since SteVe is one of the reference OCPP implementations, it's worth adding a comment here with a bit of context.

thanks for your careful review and the detailed thoughts you’ve shared. i appreciate your attention to detail.

however, this is such a small and not-so-important thing we are discussing, i.e. the place of the enum in the source code. we should not dwell on it. i propose we move on. as i said earlier: i’d like to prevent drifting into stylistic and preference discussions. would you be okay with this? thank you!

lets put our precious time and energy into things that matter. lets talk about the more important (core) stuff where i am not sure how to continue. for example this. WDYT? (we should talk about it in a separate thread)

reason: we don't want to silently ignore (or skip) non-Json chargers. ChargePointServiceJsonInvoker.runPipeline(..) has the necessary error-handling in case of exceptions. so, we can do the check there.

reason: preparation for security profile 3 (mTLS) impl. we need to validate CpoName of the certificate, which means the info has to exist somewhere within steve beforehand.

juherr · 2025-11-15T11:05:29Z

@goekay

lets talk about the more important (core) stuff where i am not sure how to continue. for example #1854 (comment). WDYT? (we should talk about it in a separate thread)

The TLS/mTLS topic is much broader than choosing a coding convention. It is a strategic architectural decision that affects how the entire project is deployed and operated.

Since you asked for my view: TLS/mTLS termination provided by the CSMS itself is fundamentally an infrastructure concern, not an application concern. Because of that, I don’t think steve should try to solve it internally or make it a core responsibility. This is exactly what edge components (reverse proxies, ingress controllers, API gateways, service meshes) are designed for.

Steve’s responsibility is on the business side of the certificate logic (e.g. mapping certificate → chargeBoxId → security profile), not on performing low-level TLS operations.

If, after further analysis, we still conclude that steve must handle TLS/mTLS directly, then the clean solution would require a multi-layer, event-driven architecture with dedicated gateway nodes and a separate backend for the business logic. Christian from Monta has written several useful posts about this approach:
https://www.linkedin.com/posts/cweinberger_engineering-ocpp-evbetter-activity-7198954253028081664-sn0y

But I don’t see how an all-in-one product could reliably manage TLS, mTLS, certificate lifecycle, OCPP logic, scaling, and cloud-native deployment without breaking how Spring Boot is supposed to work or without rebuilding infrastructure features inside the application.

feat: implement OCPP 1.6 Security

81ba853

juherr force-pushed the feature/ocpp16-security branch from e9b8666 to 81ba853 Compare October 19, 2025 11:28

juherr mentioned this pull request Oct 28, 2025

Feature/ocpp16 security #1842

Open

goekay added 2 commits October 31, 2025 10:17

Merge branch steve-community/steve 'master' into feature/ocpp16-security

b753424

# Conflicts: # src/main/java/de/rwth/idsg/steve/service/CentralSystemService16_Service.java # src/test/java/de/rwth/idsg/steve/utils/__DatabasePreparer__.java

fix: build error because of db migration with same version exists

b021b4f

goekay mentioned this pull request Oct 31, 2025

Add OCPP 1.6 security 3rd edition schema steve-community/ocpp-jaxb#18

Closed

goekay added 2 commits November 2, 2025 01:53

use data models from ocpp-jaxb dependency

ca48906

* clean up resources folder and pom file * and do necessary changes for project to compile

fix builds

ad62b97

goekay added 2 commits November 2, 2025 14:28

refactor

eb13078

refactor: move password validation to ChargePointService

3878731

* and also make getRegistrationStatus return the password additionally, in order prevent another DB lookup later

goekay force-pushed the feature/ocpp16-security branch from 036e217 to 3878731 Compare November 3, 2025 09:30

refactor DB changes

59c1c8d

* various simplifications and improvements * certificate is still as is. this is a future TODO

goekay force-pushed the feature/ocpp16-security branch from db70668 to 59c1c8d Compare November 5, 2025 15:12

goekay added 3 commits November 5, 2025 16:17

make project compile

a47ed12

* remove unnecessary and invalid parts

implement LogStatusNotification

9ad638a

implement SignedFirmwareStatusNotification

cb36fe7

implement ExtendedTriggerMessage

d84e47c

goekay force-pushed the feature/ocpp16-security branch from d54af17 to d84e47c Compare November 5, 2025 17:43

juherr commented Nov 5, 2025

View reviewed changes

goekay added 18 commits November 7, 2025 13:10

merge SecurityProfileConfiguration into org.springframework.boot.web.…

9c92479

…server.Ssl and remove configuration page to render security config details.

refactor and nits

14c0cf9

implement GetInstalledCertificateIds

256445e

implement DeleteCertificate

96f4d14

design decision: this function will not be triggered over Operations page. there will be a "installed certificates" page under "security management". each row (and therefore cert) will have a delete button.

implement and refactor certificates pages

abb7ee9

implement page for "signed certificates"

45aab67

refactor: split SecurityRepository into CertificateRepository and Eve…

30270ba

…ntRepository

build fix, refactor and cleanup

bb6c4a2

add CRUD functionality for new station fields: security profile and a…

2f5722c

…uth password

refactor

f766840

add temporal filtering to certificate and event pages

d3cfba4

refactor: move columns for consistency

0261550

nits [skip ci]

9f3a7fe

comment in JSP should not be in final html

add authPassword validation for profiles 1 and 2 in ChargePointForm

c8032fd

implement basic auth validation during websocket handshake

976d208

nits

2a5f8b0

add info boxes

a4e730f

... connecting new operations with "events and certificates" pages

nits

2f2f5a9

goekay added 3 commits November 12, 2025 12:28

nits for mTLS

d8e0147

refactor: we can access Ssl in ServerProperties (which is a bean)

2bdb892

work on markdown docs

7ea8a11

* remove security impl status doc, since not useful * remove security profiled doc, since moved to wiki: https://github.com/steve-community/steve/wiki/OCPP-1.6J-Security-Configuration

juherr commented Nov 12, 2025

View reviewed changes

goekay added 4 commits November 12, 2025 18:17

refactor: PR feedback

31cb162

push isJson() check down to ChargePointServiceJsonInvoker

d8c089e

reason: we don't want to silently ignore (or skip) non-Json chargers. ChargePointServiceJsonInvoker.runPipeline(..) has the necessary error-handling in case of exceptions. so, we can do the check there.

nits

5940aea

get CpoName and update in DB eagerly after each ws connection

9d81d32

reason: preparation for security profile 3 (mTLS) impl. we need to validate CpoName of the certificate, which means the info has to exist somewhere within steve beforehand.

Uh oh!

Implement OCPP 1.6 Security #1854

Are you sure you want to change the base?

Implement OCPP 1.6 Security #1854

Conversation

juherr commented Oct 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

goekay commented Oct 20, 2025

Uh oh!

juherr commented Oct 20, 2025

Uh oh!

goekay commented Oct 31, 2025

Uh oh!

juherr commented Oct 31, 2025

Uh oh!

goekay commented Nov 2, 2025

Uh oh!

goekay commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

goekay commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

juherr commented Nov 5, 2025

Uh oh!

goekay commented Nov 5, 2025

Uh oh!

juherr commented Nov 5, 2025

Uh oh!

juherr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

goekay commented Nov 5, 2025

Uh oh!

goekay commented Nov 12, 2025

Uh oh!

juherr commented Nov 12, 2025

Uh oh!

goekay commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

juherr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

goekay Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juherr commented Oct 19, 2025 •

edited

Loading

goekay commented Nov 4, 2025 •

edited

Loading

goekay commented Nov 5, 2025 •

edited

Loading

goekay commented Nov 12, 2025 •

edited

Loading

goekay Nov 13, 2025 •

edited

Loading