-
Notifications
You must be signed in to change notification settings - Fork 99
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: plug and charge authorization, basic happy path #83
feat: plug and charge authorization, basic happy path #83
Conversation
a813a3a
to
303ce9d
Compare
# TODO Check if EMAID has correct syntax | ||
verify_certs( | ||
leaf_cert, sub_ca_certs, self._mobility_operator_root_cert_path() | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will add a ticket noting that the verification needs to be more intelligent than this, to figure out which root cert to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HugoJP1 Have you done this, or shall I pick it back up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this falls under #94 .
Namespace.ISO_V2_MSG_DEF, | ||
) | ||
else: | ||
# TODO: handle ONGOING case, and REJECTED more fully |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will create a ticket for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isnt this part quite straight forward? in case of Ongoing or Accepted we go to the Authorization state.
in case of rejected then we can answer with an Error code.
we also need to save the state of the authorization in the session here, I believe, because if the auth is successful in the PaymentDetailsReq, then we wont need to request it in the Authorization state and we can move forward. Unless, the ocpp client holds the authorization response and answers immediately if we do another request for the same eMAID.
if the answer "accepted" was not yet received, then we will keep requesting until the timeout (60s)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Attached this to the existing #54
Namespace.ISO_V2_MSG_DEF, | ||
) | ||
else: | ||
# TODO: handle ONGOING case, and REJECTED more fully |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isnt this part quite straight forward? in case of Ongoing or Accepted we go to the Authorization state.
in case of rejected then we can answer with an Error code.
we also need to save the state of the authorization in the session here, I believe, because if the auth is successful in the PaymentDetailsReq, then we wont need to request it in the Authorization state and we can move forward. Unless, the ocpp client holds the authorization response and answers immediately if we do another request for the same eMAID.
if the answer "accepted" was not yet received, then we will keep requesting until the timeout (60s)
) | ||
authorization_result = await self.comm_session.evse_controller.is_authorized( | ||
id_token=id_token, | ||
id_token_type=self.comm_session.selected_auth_option, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just again a note about the auth option may not be what we want
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned, have resolved this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added some suggestions and food for thought
iso15118/shared/security.py
Outdated
"issuer_key_hash": public_key_hasher.finalize(), | ||
"serial_number": serial_number, | ||
# TODO: Populate with a real-world verification server | ||
"responder_url": "https://www.example.com/", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did some research and this is what we need to do to extract the responder_url
from cryptography.x509.oid import ExtensionOID, AuthorityInformationAccessOID
def get_ocsp_url(cert):
auth_inf_access = cert.extensions.get_extension_for_oid(ExtensionOID.AUTHORITY_INFORMATION_ACCESS).value
ocsps = [access_descriptor for access_descriptor in auth_inf_access if access_descriptor.access_method == AuthorityInformationAccessOID.OCSP]
if not ocsps:
raise Exception(f'no ocsp server entry in Authority Information Access extension field')
return ocsps[0].access_location.value
If the Authority Information Access is not found, the method raises a ExtensionNotFound
error
The information here: https://www.notion.so/switchev/Certificates-944733c8b468493a9e0eeace3b772d6a#3a1c89b988f2432c81c603ab370f879e
can be used to get the ocsp_url using openssl and validate the python code
iso15118/shared/security.py
Outdated
"issuer_key_hash": public_key_hasher.finalize(), | ||
"serial_number": serial_number, | ||
# TODO: Populate with a real-world verification server | ||
"responder_url": "https://www.example.com/", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, to get the hashed info for the OCSP request we can do this:
from cryptography.x509.ocsp import OCSPRequestBuilder
def get_oscp_request(cert: bytes, issuer_cert: bytes):
"""
cert: The certificate in DER format
issuer_cert: The certificate used to sign the `cert` certificate, also called the issuer certificate
"""
cert = load_der_x509_certificate(cert)
issuer_cert = load_der_x509_certificate(issuer_cert)
builder = OCSPRequestBuilder().add_certificate(cert, issuer_cert, cert.signature_hash_algorithm)
return builder.build()
ocsp_request = get_oscp_request(certificate, issuer_cert)
The data can then be accessed through the returned object, for example
ocsp_request.issuer_key_hash
.
Not sure if this is better as we need to load the issuer certificate and we may not have the root of the MO, but makes it easier then to extract the needed data. at least it can be used to compare some results...
Reference:
https://cryptography.io/en/latest/x509/ocsp/#cryptography.x509.ocsp.OCSPRequest
For some reason I can't respond directly to some of your comments. The OCSP URL stuff looks good to me -- thanks for digging this up. On handling authorization, I'd prefer to do this as a separate follow-up PR -- may do some minor refactoring relating to is_authorized as well. Agreed that we may not want to use the OCSPRequest object if it requires us to have the root certificate. |
599778e
to
0ea5a7e
Compare
iso15118/shared/security.py
Outdated
except (ExtensionNotFound, OCSPServerNotFoundError): | ||
# TODO: This may just result in failure down the road. | ||
# Should we let this fail on these exceptions, or is there | ||
# another way to try to get a responder_url? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tropxy What do you think -- is it an unrecoverable failure if the certificate does not provide a URL for an OCSP server, or is there something else we could try in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, if we cant obtain the OCSP url, then we cant do the check, then we should answer back that the check failed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iso15118/shared/security.py
Outdated
except (ExtensionNotFound, OCSPServerNotFoundError): | ||
# TODO: This may just result in failure down the road. | ||
# Should we let this fail on these exceptions, or is there | ||
# another way to try to get a responder_url? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, if we cant obtain the OCSP url, then we cant do the check, then we should answer back that the check failed
root_cert_path = self._mobility_operator_root_cert_path() | ||
verify_certs(leaf_cert, sub_ca_certs, load_cert(root_cert_path)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we dont have a guarantee that the MO root will be available in the station. So, we may actually call a evse_controller to get a True/False result for the question "verify contract certificate chain?"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deab046
to
1b8ed1a
Compare
1b8ed1a
to
003a108
Compare
evse_timestamp=time.time(), | ||
) | ||
|
||
# TODO: Should the next message really be Authorization? If so why? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the Auth status is Accepted or Ongoing, yes, the next step shall be the Authorization class, as that is the message the EV will send after reception of the PaymentDetailsRes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This implements the basic flow for Plug and Charge authorization.
Still to be done:
Note that this prepares the certificate chain and hash data in the form in which OCPP will require them.
I've left some outstanding questions in TODOs for the time being as well -- let's either resolve or make issues for these.