SecurityEventNotification.req

[olga2323]

https://aftersales.alfen.com/servicedesk/customershim/secure/attachment/1024186/1024186_OCPP+1.6+security+whitepaper+edition+3.pdf?fromIssue=390054

Seems that ocpp1.6 does not handle SecurityEventNotification. Is it planned to integrate this? Issue here is with Alfen stations that keep sending SecurityEventNotification and stack does not respond.

[xBlaz3kx]

We had the same issue with ABB, so we modified the library to support OCPP 1.6j security extension, but it does not include unit tests. We tested it in production and so far, it worked fine (https://github.com/GLCharge/ocpp-go)

[lorenzodonini]

@xBlaz3kx would you like creating a PR for the security extension? It's been on my ToDo list but didn't get to it yet. Happy to include it and then write some tests for it.

[xBlaz3kx]

@lorenzodonini Sure, I can do that.

The only thing I'm a bit unsure of is the way I implemented/added additional handlers for incoming messages, specified in the extension.

The extension's specification doesn't really mention if the functionalities should be split into different profiles (like in the original specification), nor does it specify any additional SupportedFeatures variable (as far as I understood the specification), that would indicate support for the security extension.

So I did this:

// OCPP 1.6j Security extension
	chargePoint.SetCertificateHandler(handler)
	chargePoint.SetLogHandler(handler)
	chargePoint.SetSecureFirmwareHandler(handler)
	chargePoint.SetExtendedTriggerMessageHandler(handler)
	chargePoint.SetSecurityHandler(handler)

I'd like to hear your thoughts regarding the implementation approach.

[lorenzodonini]

@xBlaz3kx thanks a lot for opening the PR. I'll have a detailed look in the next few days.

Regarding the structure, since the security extension is backported from v2, it's technically up to us where we want to put it.
If we go by the book, the features should be structured like:

• security
  • SecurityEventNotification
  • SignCertificate
  • CertificateSigned
• iso15811
  • GetInstalledCertificateIds
  • DeleteCertificate
  • InstallCertificate
• diagnostics
  • GetLog
  • LogStatusNotification
• firmware
  • SignedUpdateFirmware
  • SignedFirmwareStatusNotification
• remotecontrol
  • ExtendedTriggerMessage (this is a special kid, since it's technically just the TriggerMessage feature from v2, but since v1.6 already has it, they had to name this one differently)

Just FYI the structuring is derived from the functional blocks as presented in the 2.0.1 specs, which are identified by letters A - P.

If we follow this split I would recommend leveraging at least the existing firmware package in v1.6. Alternatively we can just group all of these features in a dedicated security-extension package. I guess as a developer I would prefer the latter, but happy to take what exists already.

Your point about the SupportedFeatures not being mentioned is valid and I agree with your current approach to stay on the safe side. We could dare adding the Security profile to the list, as it should be straightforward to understand.

[xBlaz3kx]

@lorenzodonini Of course, no problem. I'll have to fix a couple of details (I've renamed the module because we're using it in our product) before it can be merged.

Regarding the structure - it is essentially what I've done, with minor naming differences, so I think we are mostly aligned. Take a look when you can, and let me know if I should fix anything 😄

[atschabu]

We had the same issue with ABB, so we modified the library to support OCPP 1.6j security extension, but it does not include unit tests. We tested it in production and so far, it worked fine (https://github.com/GLCharge/ocpp-go)

Couldn't comment directly on your fork, but I noticed that the types.CertificateUse enum for CentralSystemRootCertificate does not match specifications

const (
	CSMSRootCertificate         CertificateUse = "CSMSRootCertificate"
	ManufacturerRootCertificate CertificateUse = "ManufacturerRootCertificate"
)

[xBlaz3kx]

We had the same issue with ABB, so we modified the library to support OCPP 1.6j security extension, but it does not include unit tests. We tested it in production and so far, it worked fine (https://github.com/GLCharge/ocpp-go)

Couldn't comment directly on your fork, but I noticed that the types.CertificateUse enum for CentralSystemRootCertificate does not match specifications

const (
	CSMSRootCertificate         CertificateUse = "CSMSRootCertificate"
	ManufacturerRootCertificate CertificateUse = "ManufacturerRootCertificate"
)

Thanks for catching this! What I've done is copy most of the code from the 2.0.1 implementation/folder, as the functionality and naming conventions very closely match the ones defined in the security extension, so I guess I need to double check.

[atschabu]

@xBlaz3kx I created PR against your fork, with a few fixes. Given how #266 is using types from 2.0.1 exclusively it will need some updating. Do you have plans on updating the PR?

[xBlaz3kx]

@xBlaz3kx I created PR against your fork, with a few fixes. Given how #266 is using types from 2.0.1 exclusively it will need some updating. Do you have plans on updating the PR?

Hey @atschabu, thanks! Unfortunately I don't work for the company anymore, so I can't do much about the PR. Instead, you can make a PR against my repository, and I'll merge the fixes.

[AhmedAbouelkher]

@xBlaz3kx I created PR against your fork, with a few fixes. Given how #266 is using types from 2.0.1 exclusively it will need some updating. Do you have plans on updating the PR?

Hey @atschabu, thanks! Unfortunately I don't work for the company anymore, so I can't do much about the PR. Instead, you can make a PR against my repository, and I'll merge the fixes.

Hey @xBlaz3kx, tried to create a PR to include @atschabu changes into your fork to speed up things, but I noticed that your fork is not quite a mirror of (GLCharge/ocpp-go)

I can't find the charge center

• SetSecurityHandler
• SetSecureFirmwareHandler
• SetLogHandler

And I can't find the charge point

• SetCertificateHandler
• SetLogHandler
• SetSecureFirmwareHandler
• SetExtendedTriggerMessageHandler
• SetSecurityHandler

[xBlaz3kx]

@xBlaz3kx I created PR against your fork, with a few fixes. Given how #266 is using types from 2.0.1 exclusively it will need some updating. Do you have plans on updating the PR?

Hey @atschabu, thanks! Unfortunately I don't work for the company anymore, so I can't do much about the PR. Instead, you can make a PR against my repository, and I'll merge the fixes.

Hey @xBlaz3kx, tried to create a PR to include @atschabu changes into your fork to speed up things, but I noticed that your fork is not quite a mirror of (GLCharge/ocpp-go)

I can't find the charge center

• SetSecurityHandler
• SetSecureFirmwareHandler
• SetLogHandler

And I can't find the charge point

• SetCertificateHandler
• SetLogHandler
• SetSecureFirmwareHandler
• SetExtendedTriggerMessageHandler
• SetSecurityHandler

Hey @AhmedAbouelkher,

correct, its not a direct mirror of the repository. I've implemented the changes in a separate branch, not in the main.

[AhmedAbouelkher]

@xBlaz3kx I created PR against your fork, with a few fixes. Given how #266 is using types from 2.0.1 exclusively it will need some updating. Do you have plans on updating the PR?

Hey @atschabu, thanks! Unfortunately I don't work for the company anymore, so I can't do much about the PR. Instead, you can make a PR against my repository, and I'll merge the fixes.

Hey @xBlaz3kx, tried to create a PR to include @atschabu changes into your fork to speed up things, but I noticed that your fork is not quite a mirror of (GLCharge/ocpp-go)
I can't find the charge center

• SetSecurityHandler
• SetSecureFirmwareHandler
• SetLogHandler

And I can't find the charge point

• SetCertificateHandler
• SetLogHandler
• SetSecureFirmwareHandler
• SetExtendedTriggerMessageHandler
• SetSecurityHandler

Hey @AhmedAbouelkher,

correct, its not a direct mirror of the repository. I've implemented the changes in a separate branch, not in the main.

That's great. I can confirm that this is indeed the branch that I cloned and did not find these missing functions.