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

crypto: clean-up impossible errors from blake2b and PublicKeyWithHash #63

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- Add `FromBase58CheckError::IncorrectBase58Prefix` variant.
- Add `NomReader`, `BinWriter` support for `Ed25519Signature`.
- Add `signature::Signature` enum representing possible types of signature used in Tezos.
- Add `From<PublicKeyEd25519>` impl for `ContractTz1Hash`.
- Add `From<PublicKeySecp256k1>` impl for `ContractTz2Hash`.
- Add `From<PublicKeyP256>` impl for `ContractTz3Hash`.
- Add `From<PublicKeyBls>` impl for `ContractTz4Hash`.

### Changed

Expand All @@ -19,6 +23,13 @@ parameterized by the lifetime of the input byte slice.
- Renamed `hash::Signature` to `hash::UnknownSignature`.
- All hash structs no longer publicly expose the underlying `Hash`. Instead, use `Into<Vec<u8>>` and `AsRef<[u8]>`.
- `ToBase58Check` no longer returns a `Result` type.
- `blake2b::digest_128`, `blake2b::digest_160`, `blake2b::digest_256` return
plain `Vec<u8>` instead of `Result<Vec<u8>, Blake2bError>`, the error was
never possible.
- `blake2b::merkle_tree` returns plain `Vec<u8>` instead of `Result<Vec<u8>,
Blake2bError>`, the error was never possible.
- `tezos_crypto_rs`: `PublicKeyWithHash::pk_hash` now returns `Self::Hash`
instead of `Result`.

### Deprecated

Expand All @@ -28,6 +39,11 @@ parameterized by the lifetime of the input byte slice.

- Removed legacy `SecretKeyEd25519` encoding.
- Removed `ToBase58CheckError`.
- Removed unused `Blake2bError::Other`.
- Removed impossible `TryFromPKError`.
- `tezos_data_encoding`: Removed unused `DecodeErrorKind::Hash` and
`DecodeError::hash_error`
- `tezos_crypto_rs`: Removed unused `Error` type from `PublicKeyWithHash`

### Fixed

Expand Down
36 changes: 13 additions & 23 deletions crypto/src/blake2b.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,21 @@ use thiserror::Error;
pub enum Blake2bError {
#[error("Output digest length must be between 16 and 64 bytes.")]
InvalidLength,
#[error("Blake2b failed")]
Other,
}

impl From<()> for Blake2bError {
fn from(_: ()) -> Self {
Self::Other
}
}

/// Generate digest of length 256 bits (32bytes) from arbitrary binary data
pub fn digest_256(data: &[u8]) -> Result<Vec<u8>, Blake2bError> {
digest(data, 32)
pub fn digest_256(data: &[u8]) -> Vec<u8> {
digest(data, 32).unwrap()
}

// Generate digest of length 160 bits (20bytes) from arbitrary binary data
pub fn digest_160(data: &[u8]) -> Result<Vec<u8>, Blake2bError> {
digest(data, 20)
pub fn digest_160(data: &[u8]) -> Vec<u8> {
digest(data, 20).unwrap()
}

/// Generate digest of length 128 bits (16bytes) from arbitrary binary data
pub fn digest_128(data: &[u8]) -> Result<Vec<u8>, Blake2bError> {
digest(data, 16)
pub fn digest_128(data: &[u8]) -> Vec<u8> {
digest(data, 16).unwrap()
}

/// Arbitrary Blake2b digest generation from generic data.
Expand Down Expand Up @@ -99,7 +91,7 @@ where
//
// TODO: optimize it,
// this implementation will calculate the same hash [5, 5] two times.
pub fn merkle_tree<Leaf>(list: &[Leaf]) -> Result<Vec<u8>, Blake2bError>
pub fn merkle_tree<Leaf>(list: &[Leaf]) -> Vec<u8>
where
Leaf: AsRef<[u8]>,
{
Expand Down Expand Up @@ -151,10 +143,7 @@ where
}
}

fn merkle_tree_inner<Leaf>(
list: &RepeatingSlice<Leaf>,
degree: u32,
) -> Result<Vec<u8>, Blake2bError>
fn merkle_tree_inner<Leaf>(list: &RepeatingSlice<Leaf>, degree: u32) -> Vec<u8>
where
Leaf: AsRef<[u8]>,
{
Expand All @@ -164,11 +153,12 @@ where
let middle = 1 << (d - 1);
digest_all(
&[
merkle_tree_inner(&RepeatingSlice(&list[..middle]), d - 1)?,
merkle_tree_inner(&RepeatingSlice(&list[middle..]), d - 1)?,
merkle_tree_inner(&RepeatingSlice(&list[..middle]), d - 1),
merkle_tree_inner(&RepeatingSlice(&list[middle..]), d - 1),
],
32,
)
.unwrap() // we know length is within bounds, and that's the only error possible
}
}
}
Expand All @@ -186,7 +176,7 @@ mod tests {

#[test]
fn blake2b_256() {
let hash = digest_256(b"hello world").unwrap();
let hash = digest_256(b"hello world");
let expected =
hex::decode("256c83b297114d201b30179f3f0ef0cace9783622da5974326b436178aeef610")
.unwrap();
Expand All @@ -195,7 +185,7 @@ mod tests {

#[test]
fn blake2b_128() {
let hash = digest_128(b"hello world").unwrap();
let hash = digest_128(b"hello world");
let expected = hex::decode("e9a804b2e527fd3601d2ffc0bb023cd6").unwrap();
assert_eq!(expected, hash);
}
Expand Down
6 changes: 2 additions & 4 deletions crypto/src/bls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,7 @@ pub fn keypair_with_hash_from_ikm(
) -> Result<(SecretKeyBls, PublicKeyBls, ContractTz4Hash), CryptoError> {
let (sk, pk) = keypair_from_ikm(ikm)?;

let hash = pk
.pk_hash()
.map_err(|e| CryptoError::AlgorithmError(format!("BLS_PK_HASH: {:?}", e)))?;
let hash = pk.pk_hash();

Ok((sk, pk, hash))
}
Expand Down Expand Up @@ -447,7 +445,7 @@ mod tests {
.expect("pk_bytes should be 48 bytes long");
let pk = PublicKeyBls(pk_bytes.to_vec());

let tz4 = pk.pk_hash().expect("PublicKey hashable");
let tz4 = pk.pk_hash();

let expected_tz4 = ContractTz4Hash::from_b58check(tz4_b58).expect("Valid tz4 b58");

Expand Down
92 changes: 23 additions & 69 deletions crypto/src/hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -516,31 +516,28 @@ impl HashType {

/// Implementation of chain_id.ml -> of_block_hash
#[inline]
pub fn chain_id_from_block_hash(block_hash: &BlockHash) -> Result<ChainId, Blake2bError> {
let result = crate::blake2b::digest_256(&block_hash.0)?;
Ok(ChainId::from_bytes(&result[0..HashType::ChainId.size()])
.unwrap_or_else(|_| unreachable!("ChainId is created from slice of correct size")))
}

#[cfg_attr(feature = "fuzzing", derive(fuzzcheck::DefaultMutator))]
#[derive(Debug, Error)]
pub enum TryFromPKError {
#[error("Error calculating digest")]
Digest(#[from] Blake2bError),
#[error("Invalid hash size")]
Size(#[from] FromBytesError),
pub fn chain_id_from_block_hash(block_hash: &BlockHash) -> ChainId {
let result = crate::blake2b::digest_256(&block_hash.0);
ChainId::from_bytes(&result[0..HashType::ChainId.size()])
.unwrap_or_else(|_| unreachable!("ChainId is created from slice of correct size"))
}

macro_rules! pk_with_hash {
($pk:ident, $pkh:ident) => {
impl PublicKeyWithHash for $pk {
type Hash = $pkh;
type Error = TryFromPKError;

fn pk_hash(&self) -> Result<Self::Hash, Self::Error> {
let hash = blake2b::digest_160(&self.0)?;
let typed_hash = Self::Hash::from_bytes(&hash)?;
Ok(typed_hash)
fn pk_hash(&self) -> Self::Hash {
let hash = blake2b::digest_160(&self.0);
// hash size is 20 bytes (160 bits), exactly how many
// ContractTz*Hash expect, safe to unwrap
Self::Hash::from_bytes(&hash).unwrap()
}
}

impl From<$pk> for $pkh {
fn from(source: $pk) -> Self {
source.pk_hash()
}
}
};
Expand All @@ -551,46 +548,6 @@ pk_with_hash!(PublicKeySecp256k1, ContractTz2Hash);
pk_with_hash!(PublicKeyP256, ContractTz3Hash);
pk_with_hash!(PublicKeyBls, ContractTz4Hash);

impl TryFrom<PublicKeyEd25519> for ContractTz1Hash {
type Error = TryFromPKError;

fn try_from(source: PublicKeyEd25519) -> Result<Self, Self::Error> {
let hash = blake2b::digest_160(&source.0)?;
let typed_hash = Self::from_bytes(&hash)?;
Ok(typed_hash)
}
}

impl TryFrom<PublicKeySecp256k1> for ContractTz2Hash {
type Error = TryFromPKError;

fn try_from(source: PublicKeySecp256k1) -> Result<Self, Self::Error> {
let hash = blake2b::digest_160(&source.0)?;
let typed_hash = Self::from_bytes(&hash)?;
Ok(typed_hash)
}
}

impl TryFrom<PublicKeyP256> for ContractTz3Hash {
type Error = TryFromPKError;

fn try_from(source: PublicKeyP256) -> Result<Self, Self::Error> {
let hash = blake2b::digest_160(&source.0)?;
let typed_hash = Self::from_bytes(&hash)?;
Ok(typed_hash)
}
}

impl TryFrom<PublicKeyBls> for ContractTz4Hash {
type Error = TryFromPKError;

fn try_from(source: PublicKeyBls) -> Result<Self, Self::Error> {
let hash = blake2b::digest_160(&source.0)?;
let typed_hash = Self::from_bytes(&hash)?;
Ok(typed_hash)
}
}

impl TryFrom<&PublicKeyEd25519> for ed25519_dalek::VerifyingKey {
type Error = FromBytesError;

Expand Down Expand Up @@ -638,7 +595,7 @@ pub enum PublicKeyError {
impl PublicKeyEd25519 {
/// Generates public key hash for public key ed25519
pub fn public_key_hash(&self) -> Result<CryptoboxPublicKeyHash, PublicKeyError> {
CryptoboxPublicKeyHash::try_from(crate::blake2b::digest_128(self.0.as_ref())?)
CryptoboxPublicKeyHash::try_from(crate::blake2b::digest_128(self.0.as_ref()))
.map_err(Into::into)
}
}
Expand All @@ -661,8 +618,7 @@ impl SecretKeyEd25519 {
actual: self.0.len(),
})?;

let payload = crate::blake2b::digest_256(data.as_ref())
.map_err(|e| CryptoError::AlgorithmError(e.to_string()))?;
let payload = crate::blake2b::digest_256(data.as_ref());
let signature = sk.sign(&payload);
Ok(Signature::Ed25519(Ed25519Signature(
signature.to_bytes().to_vec(),
Expand All @@ -685,8 +641,7 @@ impl PublicKeySignatureVerifier for PublicKeyEd25519 {
let pk = ed25519_dalek::VerifyingKey::try_from(self)
.map_err(|_| CryptoError::InvalidPublicKey)?;

let payload = crate::blake2b::digest_256(bytes)
.map_err(|e| CryptoError::AlgorithmError(e.to_string()))?;
let payload = crate::blake2b::digest_256(bytes);

pk.verify_strict(&payload, &signature)
.map_err(CryptoError::Ed25519)?;
Expand Down Expand Up @@ -783,8 +738,8 @@ impl PublicKeySignatureVerifier for PublicKeyP256 {
}

impl OperationListHash {
pub fn calculate(list: &[OperationHash]) -> Result<Self, Blake2bError> {
blake2b::merkle_tree(list).map(OperationListHash)
pub fn calculate(list: &[OperationHash]) -> Self {
OperationListHash(blake2b::merkle_tree(list))
}
}

Expand Down Expand Up @@ -839,14 +794,14 @@ mod tests {
fn test_chain_id_from_block_hash() -> Result<(), anyhow::Error> {
let decoded_chain_id: ChainId = chain_id_from_block_hash(&BlockHash::from_base58_check(
"BLockGenesisGenesisGenesisGenesisGenesisb83baZgbyZe",
)?)?;
)?);
let decoded_chain_id: &str = &decoded_chain_id.to_base58_check();
let expected_chain_id = "NetXgtSLGNJvNye";
assert_eq!(expected_chain_id, decoded_chain_id);

let decoded_chain_id: ChainId = chain_id_from_block_hash(&BlockHash::from_base58_check(
"BLockGenesisGenesisGenesisGenesisGenesisd6f5afWyME7",
)?)?;
)?);
let decoded_chain_id: &str = &decoded_chain_id.to_base58_check();
let expected_chain_id = "NetXjD3HPJJjmcd";
assert_eq!(expected_chain_id, decoded_chain_id);
Expand Down Expand Up @@ -1359,8 +1314,7 @@ mod tests {
OperationHash::from_base58_check(operation_0).unwrap(),
OperationHash::from_base58_check(operation_1).unwrap(),
OperationHash::from_base58_check(operation_2).unwrap(),
])
.unwrap();
]);

let predecessor = "BLc1ntjBDjsszZkGrbyHMoQ6gByJzkEjMs94T7UaM1ogGXa1PzF";
let payload_hash = BlockPayloadHash::calculate(
Expand Down
3 changes: 1 addition & 2 deletions crypto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,8 @@ pub enum CryptoError {
/// Public key that support hashing.
pub trait PublicKeyWithHash {
type Hash;
type Error;

fn pk_hash(&self) -> Result<Self::Hash, Self::Error>;
fn pk_hash(&self) -> Self::Hash;
}

/// Public key that supports signature verification
Expand Down
15 changes: 1 addition & 14 deletions tezos-encoding/src/nom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use self::error::{BoundedEncodingKind, DecodeError, DecodeErrorKind};
pub mod error {
use std::{fmt::Write, str::Utf8Error};

use crypto::blake2b::Blake2bError;
use nom::{
error::{ErrorKind, FromExternalError},
Offset,
Expand Down Expand Up @@ -65,8 +64,6 @@ pub mod error {
UnknownTag(String),
/// Invalid tag
InvalidTag(String),
/// Other errors can be generated by custom parsers.
Hash(Blake2bError),
}

/// Specific bounded encoding kind.
Expand Down Expand Up @@ -126,14 +123,6 @@ pub mod error {
_ => None,
}
}

pub fn hash_error(input: NomInput<'a>, error: Blake2bError) -> Self {
Self {
input,
kind: DecodeErrorKind::Hash(error),
other: None,
}
}
}

impl<I> nom::error::ParseError<I> for DecodeError<I> {
Expand Down Expand Up @@ -206,7 +195,6 @@ pub mod error {
DecodeErrorKind::Bits(e) => write!(res, " while performing bits operation: {}", e),
DecodeErrorKind::UnknownTag(tag) => write!(res, " caused by unsupported tag `{}`", tag),
DecodeErrorKind::InvalidTag(tag) => write!(res, " caused by invalid tag `{}`", tag),
DecodeErrorKind::Hash(e) => write!(res, " because of error calculating hash: {}", e),
};

if let Some(other) = error.other {
Expand Down Expand Up @@ -584,8 +572,7 @@ where
{
move |input| {
let (rest, result) = parser(input)?;
let hash = crypto::blake2b::digest_256(&input[..input.len() - rest.len()])
.map_err(|e| nom::Err::Failure(NomError::hash_error(input, e)))?;
emturner marked this conversation as resolved.
Show resolved Hide resolved
let hash = crypto::blake2b::digest_256(&input[..input.len() - rest.len()]);
Ok((rest, (result, hash)))
}
}
Expand Down