torusresearch / torus-utils-swift

Swift package for fetching shares from torus-nodes
BSD 3-Clause "New" or "Revised" License
4 stars 11 forks source link

`retrieveShare` sometimes does not retrieve the correct share #64

Closed matthiasgeihs closed 8 months ago

matthiasgeihs commented 9 months ago

Context

In some cases, the retrieveShare function does not retrieve the correct key share. https://github.com/torusresearch/torus-utils-swift/blob/61b36686c2def5f2f40d8cb0d5bc3af34559c1d5/Sources/TorusUtils/Extensions/TorusUtils%2Bextension.swift#L270 The issue is not perfectly understood, but there is currently a workaround that seems to resolve the issue for all tested instances. Roughly, the workaround tries two different padding schemes for decryption and takes the one that works. https://github.com/torusresearch/torus-utils-swift/blob/61b36686c2def5f2f40d8cb0d5bc3af34559c1d5/Sources/TorusUtils/Extensions/TorusUtils%2Bextension.swift#L456

Observation 1: Ciphertext padding

The following piece of code applies padding to the ciphertext. Padding a ciphertext should usually not be done as it can lead to decryption failures. https://github.com/torusresearch/torus-utils-swift/blob/61b36686c2def5f2f40d8cb0d5bc3af34559c1d5/Sources/TorusUtils/Extensions/TorusUtils%2Bextension.swift#L453 Question: Why is this padding is applied?

Observation 2: Double encoding

The ciphertext seems to be double-encoded: Base64(Hex(ciphertext)). https://github.com/torusresearch/torus-utils-swift/blob/61b36686c2def5f2f40d8cb0d5bc3af34559c1d5/Sources/TorusUtils/Extensions/TorusUtils%2Bextension.swift#L452 Question: Why do we use this double encoding? Why not just transmit as hex string?

Observation 3: Custom implementation of ECDH

The secp256k1.swift library already provides an implementation of ECDH. However, we ship our own implementation of ECDH, unnecessarily increasing the size of our code base and introducing additional surface for implementation error. https://github.com/torusresearch/torus-utils-swift/blob/61b36686c2def5f2f40d8cb0d5bc3af34559c1d5/Sources/TorusUtils/Extensions/TorusUtils%2Bextension.swift#L1562 Question: Why do we use a custom implementation of ECDH? Why not just use the provided one?

Observation 4: AES padding implementation does not seem to be root cause of issue

I have implemented a minimal example consisting of a Go program and a Swift program that tests the ICIES encryption functionality in an isolated setting. Encryption is done in Go using TRON-US/go-eccrypto library, and decryption is done in Swift using a custom implementation of the ICIES decryption algorithm that relies on the Swift libraries secp256k1.swift for ECDH and CryptoSwift for AES. The two implementations seem to be fully compatible. There were no decryption failures in 1000 examples.

Interim conclusion (to be further investigated)

The most probable cause for the decryption failure as I currently see it is a padding / encoding issue.