GSMA-CPAS / BWRP-chaincode

Apache License 2.0
1 stars 0 forks source link

Forged certificates can be used for double signing #40

Closed informartin closed 3 years ago

informartin commented 3 years ago

Currently, double signing is preventing by merely comparing the certificate strings: https://github.com/GSMA-CPAS/BWRP-chaincode/blob/afff34c4b1787d034c35dc094a8905291f7e4b38/hybrid/contract/roaming.go#L593

I expected any change to the string would render the certificate's signature in the chain invalid. However, this doesn't seem to be the case necessarily. For instance, double signing is possible using the follwing certificates:

-----BEGIN CERTIFICATE-----
MIICkTCCAjegAwIBAgIUP10LqzohH8cuePIdHAnU0Rp8rbEwCgYIKoZIzj0EAwIw
gZkxEDAOBgNVBAYTB0dlcm1hbnkxDTALBgNVBAgTBEJvbm4xDTALBgNVBAcTBEJv
bm4xIjAgBgNVBAkTGUZyaWVkcmljaC1FYmVydC1BbGxlZSAxNDAxDjAMBgNVBBET
BTUzMTEzMRcwFQYDVQQKEw5kdGFnLm5vbWFkLmNvbTEaMBgGA1UEAxMRY2EuZHRh
Zy5ub21hZC5jb20wHhcNMjEwNDE0MTM0NTAwWhcNMjIwNDE0MTM1MDAwWjAfMQ8w
DQYDVQQLEwZjbGllbnQxDDAKBgNVBAMTA2FhYTBZMBMGByqGSM49AgEGCCqGSM49
AwEHA0IABHkSg66brPU66HJ3zvoKILvBUy2bHdSKcPNRhzDyspS8I4MPoZ2vLZtw
yBy4TtCO83z4nKDLKUBqixOxCYaj5QejgdUwgdIwDgYDVR0PAQH/BAQDAgeAMAwG
A1UdEwEB/wQCMAAwHQYDVR0OBBYEFCQIZgiyw+mTUk9RQa7mnOLJP4PDMCsGA1Ud
IwQkMCKAII4NzV1uHNbUIHhJV7pshUiWURqWj/zWt7lNT+ELWCH/MGYGCCoDBAUG
BwgBBFp7ImF0dHJzIjp7IkNhblNpZ24iOiJ5ZXMiLCJoZi5BZmZpbGlhdGlvbiI6
IiIsImhmLkVucm9sbG1lbnRJRCI6ImFhYSIsImhmLlR5cGUiOiJjbGllbnQifX0w
CgYIKoZIzj0EAwIDSAAwRQIhAPfNmbNUKHqMeUDrSxhlTnP2AHd//O/9s3oyIM3S
Swz1AiAr4uZ7L57DclBYCw23Ar2LaAZwC/nInM49ZhMBzxQGIW==
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
MIICkTCCAjegAwIBAgIUP10LqzohH8cuePIdHAnU0Rp8rbEwCgYIKoZIzj0EAwIw
gZkxEDAOBgNVBAYTB0dlcm1hbnkxDTALBgNVBAgTBEJvbm4xDTALBgNVBAcTBEJq
bm4xIjAgBgNVBAkTGUZyaWVkcmljaC1FYmVydC1BbGxlZSAxNDAxDjAMBgNVBBET
BTUzMTEzMRcwFQYDVQQKEw5kdGFnLm5vbWFkLmNvbTEaMBgGA1UEAxMRY2EuZHRh
Zy5ub21hZC5jb20wHhcNMjEwNDE0MTM0NTAwWhcNMjIwNDE0MTM1MDAwWjAfMQ8w
DQYDVQQLEwZjbGllbnQxDDAKBgNVBAMTA2FhYTBZMBMGByqGSM49AgEGCCqGSM49
AwEHA0IABHkSg66brPU66HJ3zvoKILvBUy2bHdSKcPNRhzDyspS8I4MPoZ2vLZtw
yBy4TtCO83z4nKDLKUBqixOxCYaj5QejgdUwgdIwDgYDVR0PAQH/BAQDAgeAMAwG
A1UdEwEB/wQCMAAwHQYDVR0OBBYEFCQIZgiyw+mTUk9RQa7mnOLJP4PDMCsGA1Ud
IwQkMCKAII4NzV1uHNbUIHhJV7pshUiWURqWj/zWt7lNT+ELWCH/MGYGCCoDBAUG
BwgBBFp7ImF0dHJzIjp7IkNhblNpZ24iOiJ5ZXMiLCJoZi5BZmZpbGlhdGlvbiI6
IiIsImhmLkVucm9sbG1lbnRJRCI6ImFhYSIsImhmLlR5cGUiOiJjbGllbnQifX0w
CgYIKoZIzj0EAwIDSAAwRQIhAPfNmbNUKHqMeUDrSxhlTnP2AHd//O/9s3oyIM3S
Swz1AiAr4uZ7L57DclBYCw23Ar2LaAZwC/nInM49ZhMBzxQGIW==
-----END CERTIFICATE-----

The only difference is the letter v that turned to q in the end of the second line. Both certificates are deemed valid though. I assume the encoding captures some part of the certificate that is not signed by the respective CA.

Therefore, a more elaborate mechanism is required, e.g. using the certificate's serial number.

EDIT: The root cause is probably that there is no actual validation of the certificate in the storage process, as described in issue #41. Nevertheless, a more robust implementation as proposed may still be beneficial, as new line encodings etc. could imply distinct strings while after decoding the certificate may be deemed valid.