EVerest / everest-demo

EVerest demo: Dockerized demo with software in the loop simulation
Apache License 2.0
14 stars 15 forks source link

MRE with working OCPP 2.0.1 security profile 1, 2 and 3 (all) #31

Closed sahabulh closed 6 months ago

sahabulh commented 6 months ago

Based on Bryan's fix for the first MRE. I have just added scripts and databases for all profiles.

Two issues:

  1. These errors below are still there:
    2024-03-16 17:43:29.840170 [ERRO] manager         void Everest::error::ErrorCommBridge::handle_request_clear_error(const json&) :: Error while clearing errors: No errors matched the request.
    2024-03-16 17:43:29.883335 [ERRO] manager         void Everest::error::ErrorCommBridge::handle_request_clear_error(const json&) :: Error while clearing errors: No errors matched the request.
    2024-03-16 17:43:29.883568 [INFO] evse_manager_2:  :: All errors cleared
    2024-03-16 17:43:29.884491 [INFO] evse_manager_1:  :: All errors cleared
    2024-03-16 17:43:29.928309 [ERRO] manager         void Everest::error::ErrorCommBridge::handle_request_clear_error(const json&) :: Error while clearing errors: No errors matched the request.
    2024-03-16 17:43:29.928491 [ERRO] manager         void Everest::error::ErrorCommBridge::handle_request_clear_error(const json&) :: Error while clearing errors: No errors matched the request.
    2024-03-16 17:43:29.928821 [INFO] evse_manager_1:  :: All errors cleared
    2024-03-16 17:43:29.971119 [INFO] evse_manager_2:  :: All errors cleared
    2024-03-16 17:43:30.059154 [ERRO] manager         void Everest::error::ErrorCommBridge::handle_request_clear_error(const json&) :: Error while clearing errors: No errors matched the request.
    2024-03-16 17:43:30.103009 [ERRO] manager         void Everest::error::ErrorCommBridge::handle_request_clear_error(const json&) :: Error while clearing errors: No errors matched the request.
  2. No OCPP logs although the OCPP module reports it is logging messages.
    2024-03-16 17:43:29.360335 [INFO] ocpp:OCPP201     :: Logging OCPP messages to log file: /tmp/everest_ocpp_logs/2024-03-16T17:43:29.360Z.log
    2024-03-16 17:43:29.360396 [INFO] ocpp:OCPP201     :: Logging OCPP messages to html file: /tmp/everest_ocpp_logs/2024-03-16T17:43:29.360Z.html
    2024-03-16 17:43:29.360427 [INFO] ocpp:OCPP201     :: Logging SecurityEvents to file
    $ /tmp/everest_ocpp_logs/2024-03-16T17:43:29.360Z.html
    bash: /tmp/everest_ocpp_logs/2024-03-16T17:43:29.360Z.html: No such file or directory
shankari commented 6 months ago

@activeshadow and @sahabulh you need to sign off on your commits. This is required for LFE projects to ensure clear copyright for the generated code https://developercertificate.org/, https://stackoverflow.com/a/1962112

If you want to go back and sign your commits, you can rebase with -s https://stackoverflow.com/a/15667644 or close this PR and submit a new one

No OCPP logs although the OCPP module reports it is logging messages

Where are you checking the logs? The fact that your command starts with $ seems to indicate that you are checking on the host and not the container. The code runs in the container so the log is in the container. You need to use docker cp to get it out.

shankari commented 6 months ago

@sahabulh can you please address the review feedback? Also here's the list of commits that are currently not signed off

Screenshot 2024-03-18 at 8 14 12 AM
sahabulh commented 6 months ago

@shankari This PR is ready to be looked at again

shankari commented 6 months ago

@sahabulh can you please comment on where the EonTI certificates come from? Did you generate them from the root, or are they generated by EonTI? I would like to respond and provide feedback to EonTI on the certificates that they have generated and the ability to use them in software settings https://github.com/EVerest/everest-demo/issues/25#issuecomment-2001084426 https://github.com/EVerest/everest-demo/issues/25#issuecomment-2002308322

activeshadow commented 6 months ago

@sahabulh can you please comment on where the EonTI certificates come from? Did you generate them from the root, or are they generated by EonTI? I would like to respond and provide feedback to EonTI on the certificates that they have generated and the ability to use them in software settings #25 (comment) #25 (comment)

@shankari sorry to jump in this conversation, especially since I'm not answering your EonTI question specifically, but I did want to point out that in general we (@sahabulh and I) did not include anything specific to EonTI in this PR. This PR is still using the same certs as the SP2 demo. If/When the EonTI stuff gets figured out, the plan is to have that in a separate commit/PR to keep things clean.

sahabulh commented 6 months ago

@sahabulh can you please comment on where the EonTI certificates come from? Did you generate them from the root, or are they generated by EonTI? I would like to respond and provide feedback to EonTI on the certificates that they have generated and the ability to use them in software settings #25 (comment) #25 (comment)

@shankari Craig is already talking with the EonTi team about this. I don't have the details. But I will let you know what happens.

activeshadow commented 6 months ago

@shankari also worth noting that if you want to test the SP3 demo script locally, you'll need to be sure to change line 34 in the demo script to clone the forked repo and branch this PR is based on instead of the main repo. Just FYSA, though I'm sure you're already aware.

shankari commented 6 months ago

@activeshadow understood. I hijacked this PR a bit because I would really like the response to the EonTI question and I wasn't seeing a response to it otherwise.

@sahabulh so are you saying that Craig provided the EonTI certificates to you? I would just like clarity on where the certificates came from because they do not appear to match the certificates in the Google Drive shared by EonTI. I can go out to EonTI saying that I don't know where the certificates came from, but in general, it is better to provide as much information as possible upfront.

CRR-SNL commented 6 months ago

@shankari @sahabulh @activeshadow Certificates we're using come from the CRP PKI prototype instance Sandia has access to under an MOU with SAE. They might differ from those EonTi is providing for NREL testing early next month. BTW I've invited SAE and EonTi leads to join the EVerest General weekly call tomorrow and requested a slot on the agenda for them to share the current situation re: availability of their planned test platform (APIs) and roadmap.

To move beyond uncertainty/confusion about cert provenance, intended use, etc. I've suggested EVerest modules use a local repository (for now, perhaps APIs later) that supports multiple well-documented cert bundles.

activeshadow commented 6 months ago

@sahabulh you didn't sign off on your most recent commit that reverted the changes to config.json per the requested changes. I would suggest just squashing your latest commit with the reverted changes into the first commit in this PR where the changes were originally made and then force pushing to your branch for this PR. I can walk you though the steps if you like. It can be done via an interactive rebase like we did earlier.

sahabulh commented 6 months ago

@activeshadow Sorry, fixed it in a different way. Is it okay?

activeshadow commented 6 months ago

@activeshadow Sorry, fixed it in a different way. Is it okay?

Yes sir, all good!

sahabulh commented 6 months ago

@activeshadow Respecting your suggestion, I squashed the last commit into the first one using the interactive rebase and then force pushed it.

sahabulh commented 6 months ago

@sahabulh Once you revert that change, I am happy to merge this. But when you submit the EonTI PR, I would like to see a description of how the certs are generated. This is an MRE after all so we need to understand all the steps properly!

I also assume you have tested, and am merging optimistically! We can always fix if it turns out that this doesn't work.

Please review again. I reverted the changes to the config file.

activeshadow commented 6 months ago

@activeshadow Respecting your suggestion, I squashed the last commit into the first one using the interactive rebase and then force pushed it.

Noice! :+1:

shankari commented 6 months ago

@CRR-SNL thank you for that clarification! That explains why I could not find any of these certs in the files provided by EonTI for the PKI testing event.

wrt

To move beyond uncertainty/confusion about cert provenance, intended use, etc. I've suggested EVerest modules use a local repository (for now, perhaps APIs later) that supports multiple well-documented cert bundles.

The EVerest libevsecurity module supports cert bundles - if you check the script included here, you will see that we upload a cert bundle to the EVSE container and copy the certs to the correct locations. Any arbitrary cert bundle can be supported in the same way.

However, I am not sure that EVerest will want to have multiple cert bundles installed in the EVSE at the same time; in production, an EVSE will have the cert bundle for the CSMS that they are connecting to.

Having said that, from an internoperability perspective, maybe it would be better to have multiple V2G roots in a root bundle to make it easier to switch between networks. To go back to my favorite example of browser + website interop, most browsers will store multiple CA roots, which they keep updated through periodic updates.

Did you mean multiple roots or multiple bundles?

shankari commented 6 months ago

@sahabulh the checks are failing since you renamed device_model_storage_maeve_preconfigured.db to manager/device_model_storage_maeve_sp1.db

Please either restore, or change the manager Dockerfile

sahabulh commented 6 months ago

@sahabulh the checks are failing since you renamed device_model_storage_maeve_preconfigured.db to manager/device_model_storage_maeve_sp1.db

Please either restore, or change the manager Dockerfile

Done!

shankari commented 6 months ago

Checks now pass. We should fix the static analysis across all the scripts so that all the checks pass 😄 but that doesn't need to hold up this release.