EVerest / libevse-security

Apache License 2.0
7 stars 5 forks source link

Feature/get ocsp cache data #64

Closed AssemblyJohn closed 2 months ago

AssemblyJohn commented 2 months ago

Describe your changes

Issue ticket number and link

https://github.com/EVerest/libocpp/pull/596

Checklist before requesting a review

AssemblyJohn commented 2 months ago

lgtm except for the OCSP update and retrieval.

My idea would be: In update_ocsp_cache we can simply choose a random file name for the OCSP response and store it in some directory that is dedicated for OCSP responses.

For retrieve_ocsp_cache we should not search for the ocsp response by filename, but rather iterate over the files in the OCSP directory, parse the OCSP data and check if the certificate hash data matches.

What do you think?

I would require some hint in extracting the certificate hash data from the OCSP response. How is that possible? Also it will incur a certain performance cost, since it will imply parsing of all the data all the time, however it seems more stable than the current version.

Pietfried commented 2 months ago

lgtm except for the OCSP update and retrieval. My idea would be: In update_ocsp_cache we can simply choose a random file name for the OCSP response and store it in some directory that is dedicated for OCSP responses. For retrieve_ocsp_cache we should not search for the ocsp response by filename, but rather iterate over the files in the OCSP directory, parse the OCSP data and check if the certificate hash data matches. What do you think?

I would require some hint in extracting the certificate hash data from the OCSP response. How is that possible? Also it will incur a certain performance cost, since it will imply parsing of all the data all the time, however it seems more stable than the current version.

I was hoping openssl provides functionality to decode and parse the certificate hash data from the DER encoded ocsp response, but I dont know if that is the case.

It would be interesting to know how the OCSP response is loaded during the TLS handshake, because this is our targeted use case. Maybe @james-ctc already has some insights about this?

AssemblyJohn commented 2 months ago

Implemented header/interface refactor, request feedback before proceeding.

AssemblyJohn commented 2 months ago

There's also a requirement to integrate this with the garbage collection:

AssemblyJohn commented 2 months ago

Comments have been implemented, OCSP relevant test has been implemented.

One more test update is required, for garbage collection related to deleted certificates.

AssemblyJohn commented 2 months ago

Relevant issues and comments have been solved.