Skip to content
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

SignatureException is thrown at EVSE #19

Closed
JanBoldt-uml opened this issue Jan 10, 2018 · 7 comments
Closed

SignatureException is thrown at EVSE #19

JanBoldt-uml opened this issue Jan 10, 2018 · 7 comments

Comments

@JanBoldt-uml
Copy link

Hello,

When setting DC_combo_core as requested energy transfer mode, the Charging Loop Counter is not considered (see here). The charging loop is instead stopped by a SignatureException thrown in the SECC-Code.
Apparently the signature of an MeteringReciptReq sent by the EVCC is invalid.

2018-01-10 11:42:26.447 [ConnectionThread fe80:0:0:0:6ce8:6864:8bd7:9744%2] ERROR SecurityUtils - SignatureException occurred while trying to verify signature value
java.security.SignatureException: Could not verify signature
	at sun.security.ec.ECDSASignature.engineVerify(ECDSASignature.java:325) ~[sunec.jar:1.8.0_151]
	at java.security.Signature$Delegate.engineVerify(Signature.java:1219) ~[?:1.8.0_144]
	at java.security.Signature.verify(Signature.java:652) ~[?:1.8.0_144]
	at com.v2gclarity.risev2g.shared.utils.SecurityUtils.verifySignature(SecurityUtils.java:1920) [classes/:?]
	at com.v2gclarity.risev2g.shared.utils.SecurityUtils.verifySignature(SecurityUtils.java:1826) [classes/:?]
	at com.v2gclarity.risev2g.secc.states.WaitForMeteringReceiptReq.isResponseCodeOK(WaitForMeteringReceiptReq.java:111) [classes/:?]
	at com.v2gclarity.risev2g.secc.states.WaitForMeteringReceiptReq.processIncomingMessage(WaitForMeteringReceiptReq.java:63) [classes/:?]
	at com.v2gclarity.risev2g.secc.session.V2GCommunicationSessionSECC.processIncomingMessage(V2GCommunicationSessionSECC.java:184) [classes/:?]
	at com.v2gclarity.risev2g.secc.session.V2GCommunicationSessionSECC.update(V2GCommunicationSessionSECC.java:162) [classes/:?]
	at java.util.Observable.notifyObservers(Observable.java:159) [?:1.8.0_144]
	at com.v2gclarity.risev2g.secc.transportLayer.ConnectionHandler.run(ConnectionHandler.java:135) [classes/:?]
	at java.lang.Thread.run(Thread.java:748) [?:1.8.0_144]
Caused by: java.security.SignatureException: Invalid encoding for signature
	at sun.security.ec.ECDSASignature.decodeSignature(ECDSASignature.java:400) ~[sunec.jar:1.8.0_151]
	at sun.security.ec.ECDSASignature.engineVerify(ECDSASignature.java:322) ~[sunec.jar:1.8.0_151]
	... 11 more
Caused by: java.io.IOException: Invalid encoding: redundant leading 0s
	at sun.security.util.DerInputBuffer.getBigInteger(DerInputBuffer.java:162) ~[?:1.8.0_144]
	at sun.security.util.DerValue.getPositiveBigInteger(DerValue.java:552) ~[?:1.8.0_144]
	at sun.security.ec.ECDSASignature.decodeSignature(ECDSASignature.java:385) ~[sunec.jar:1.8.0_151]
	at sun.security.ec.ECDSASignature.engineVerify(ECDSASignature.java:322) ~[sunec.jar:1.8.0_151]
	... 11 more

This is propably the late result of an exception thrown by the EVCC-Code:

2018-01-10 11:42:26.440 [Thread-1] WARN  SecurityUtils - ArrayIndexOutOfBoundsException occurred while trying to get raw signature from DER encoded signature
java.lang.ArrayIndexOutOfBoundsException: null
	at java.lang.System.arraycopy(Native Method) ~[?:1.8.0_144]
	at com.v2gclarity.risev2g.shared.utils.SecurityUtils.getRawSignatureFromDEREncoding(SecurityUtils.java:2009) [classes/:?]
	at com.v2gclarity.risev2g.shared.utils.SecurityUtils.signSignedInfoElement(SecurityUtils.java:1791) [classes/:?]
	at com.v2gclarity.risev2g.shared.messageHandling.MessageHandler.getHeader(MessageHandler.java:208) [classes/:?]
	at com.v2gclarity.risev2g.shared.messageHandling.MessageHandler.getV2GMessage(MessageHandler.java:188) [classes/:?]
	at com.v2gclarity.risev2g.shared.messageHandling.MessageHandler.getV2GMessage(MessageHandler.java:174) [classes/:?]
	at com.v2gclarity.risev2g.shared.misc.State.getSendMessage(State.java:97) [classes/:?]
	at com.v2gclarity.risev2g.shared.misc.State.getSendMessage(State.java:59) [classes/:?]
	at com.v2gclarity.risev2g.evcc.states.WaitForCurrentDemandRes.processIncomingMessage(WaitForCurrentDemandRes.java:81) [classes/:?]
	at com.v2gclarity.risev2g.evcc.session.V2GCommunicationSessionEVCC.update(V2GCommunicationSessionEVCC.java:178) [classes/:?]
	at java.util.Observable.notifyObservers(Observable.java:159) [?:1.8.0_144]
	at com.v2gclarity.risev2g.evcc.transportLayer.StatefulTransportLayerClient.processIncomingMessage(StatefulTransportLayerClient.java:119) [classes/:?]
	at com.v2gclarity.risev2g.evcc.transportLayer.TLSClient.run(TLSClient.java:169) [classes/:?]
	at java.lang.Thread.run(Thread.java:748) [?:1.8.0_144]

It seems the length of R and S are not always 32 or 33 but sometimes 31 as well.
How should this case be handled?

Logfiles for this run are attached. If you need more details i will provide them ASAP.

Thank You for the Support!
EVCC.log

Best regards,
Jan

@JanBoldt-uml
Copy link
Author

Hi,
here is the second logfile:
SECC-Copy.log

Best regards,
Jan

@MarcMueltin
Copy link
Contributor

MarcMueltin commented Jan 11, 2018

Hi Jan,
thanks for pointing this out. I tried to reconstruct the problem you illustrated and can confirm that indeed the signature that results from Java's ecdsa.sign() function sometimes returns a byte array of 69 bytes instead of the expected 70 bytes.
I thought a DER encoded signature will always consist of 70 bytes, but apparently not.
The 69 bytes signature leads to the S value being 31 bytes instead of 32 bytes. Which is very odd, because both the R and the S value should be 32 bytes long (in a system where 256 bit ECDSA keys are used).
My assumption would be that Java does not apply padding to the maximal size of the key (32 bytes) automatically and that I need to do this manually.

So I need to dig deeper and will let you know once I found out.

@MarcMueltin
Copy link
Contributor

MarcMueltin commented Jan 11, 2018

OK, I think I now found the solution to the problem and understood better how to handle DER encoded ECDSA signatures.

DER-encoded integers are defined so that they can encode both positive and negative values (aka signed values). This means that the leftmost bit (first bit of the first byte in BigEndian) indicates whether the value is positive (0) or negative (1).

For ECDSA, however, the r and s values are positive integers. So the leftmost bit must be a 0. If it's not, a 0x00 padding byte must be added.

So the byte that encodes the length of r or s will either be 0x1F (31 bytes), 0x20 (32 bytes) or 0x21 (33 bytes), depending on the leftmost bit of the value of r or s.

Case 31 bytes: Java seems to have cut off the leftmost 0x00 byte (doesn't change the value or r or s), and the leftmost bit of the remaining byte array is 0 (-> a positive value).

Case 32 bytes: What we would normally expect, as the x- and y-coordinates are positive values of 32 bytes length. The leftmost bit is set to 0.

Case 33 bytes: Java added another 0x00 fill byte as the most significant (leftmost) byte (doesn't change the value of r or s) because the original value had the leftmost bit set to 1, which would result in a negative value.

I will go on and test the code once I corrected it and provide an updated release (probably today).

@JanBoldt-uml
Copy link
Author

JanBoldt-uml commented Jan 11, 2018

Hi Marc,

Thank you for digging into this.

I already modified the getDEREncodedSignature and getRawSignatureFromDEREncoding methods to encode and decode the signatures dynamically according to their length. Yet, i did not try the implementation and i am not sure whether my implementation is correct on the "ISO15118 side".

BR,
Jan

@JanBoldt-uml
Copy link
Author

Hi Marc,

My modifications to the aforementioned methods are working fine. Still i do not know if the "raw" signature is encoded according to ISO15118.

As i cannot commit to this repository, i attached a patch file.

Best Regards,

0001-Fix-for-the-issue-regarding-the-length-of-R-and-S-in.zip

Jan

@MarcMueltin
Copy link
Contributor

Hi Jan,

I just finished fixing the signature issue and will commit later that day.

MarcMueltin pushed a commit that referenced this issue Jan 12, 2018
…eteringReceiptRes) and #19 (error in DER encoding of ECDSA signature)
@MarcMueltin
Copy link
Contributor

Hi Jan,

I looked at your code and I like your suggestion. There is only a slight modification needed in the getRAWSignatureFromDEREncoding() function. You'll see the changes to your code in the commit 0e4b838.
I tested the new code and it runs stable, no more encoding/decoding errors. Thanks again for pointing this out! Highly appreciated.

I'm gonna close this ticket now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants