Skip to content

refactor(sha): use sha2 crate for hashes instead of sodiumoxide #449

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

Merged
merged 1 commit into from
Apr 29, 2021
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
2 changes: 1 addition & 1 deletion .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
os: [ubuntu-latest, macos-latest, windows-latest]
rust:
- stable
- 1.44.0
- 1.48.0
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ issue / pull request should be filled on the reference repository.
[CONTRIBUTING.md](/CONTRIBUTING.md).

## Building
Fairly simple. First, install [Rust] >= 1.44.0 and a C compiler ([Build Tools
Fairly simple. First, install [Rust] >= 1.48.0 and a C compiler ([Build Tools
for Visual Studio][VSBuild] on Windows, GCC or Clang on other platforms).

Then you can build the debug version with
Expand Down
15 changes: 2 additions & 13 deletions tox_binary_io/src/sodium.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use nom::named;

use nom::{map_opt, take};
use nom::{map_opt, named, take};

use sodiumoxide::crypto::box_::{
PublicKey,
Expand All @@ -11,7 +9,6 @@ use sodiumoxide::crypto::box_::{
NONCEBYTES
};

use sodiumoxide::crypto::hash::{sha256, sha512};
use sodiumoxide::crypto::secretbox;

use super::FromBytes;
Expand Down Expand Up @@ -46,18 +43,10 @@ impl FromBytes for secretbox::Nonce {
named!(from_bytes<secretbox::Nonce>, map_opt!(take!(secretbox::NONCEBYTES), secretbox::Nonce::from_slice));
}

impl FromBytes for sha256::Digest {
named!(from_bytes<sha256::Digest>, map_opt!(take!(sha256::DIGESTBYTES), sha256::Digest::from_slice));
}

impl FromBytes for sha512::Digest {
named!(from_bytes<sha512::Digest>, map_opt!(take!(sha512::DIGESTBYTES), sha512::Digest::from_slice));
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn public_key_parse_bytes_test() {
let bytes = [42; PUBLICKEYBYTES];
Expand Down
1 change: 1 addition & 0 deletions tox_core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ lru = "0.6"
bitflags = "1.0"
itertools = "0.10"
rand = "0.8"
sha2 = "0.9"

[dependencies.tokio]
version = "1.0"
Expand Down
3 changes: 1 addition & 2 deletions tox_core/src/dht/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2414,12 +2414,11 @@ mod tests {
let response = unpack!(packet, Packet::OnionResponse3);
let response = unpack!(response.payload, InnerOnionResponse::OnionAnnounceResponse);
let payload = response.get_payload(&precomp).unwrap();
let ping_id = sha256::Digest(payload.ping_id_or_pk);

// announce node

let payload = OnionAnnounceRequestPayload {
ping_id,
ping_id: payload.ping_id_or_pk,
search_pk: gen_keypair().0,
data_pk: gen_keypair().0,
sendback_data: 42
Expand Down
18 changes: 9 additions & 9 deletions tox_core/src/onion/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use tox_packet::ip_port::*;
use crate::onion::client::errors::*;
use crate::onion::client::onion_path::*;
use crate::onion::client::paths_pool::*;
use crate::onion::onion_announce::INITIAL_PING_ID;
use crate::onion::onion_announce::{PingId, INITIAL_PING_ID};
use tox_packet::onion::*;
use tox_packet::packed_node::*;
use crate::relay::client::Connections as TcpConnections;
Expand Down Expand Up @@ -169,7 +169,7 @@ struct OnionNode {
/// Path used to send packets to this node.
path_id: OnionPathId,
/// Ping id that should be used to announce to this node.
ping_id: Option<sha256::Digest>,
ping_id: Option<PingId>,
/// Data `PublicKey` that should be used to send data packets to our friend
/// through this node.
data_pk: Option<PublicKey>,
Expand Down Expand Up @@ -268,7 +268,7 @@ struct AnnouncePacketData<'a> {
impl<'a> AnnouncePacketData<'a> {
/// Create `InnerOnionAnnounceRequest`. The request is a search request if
/// pind_id is 0 and an announce request otherwise.
fn request(&self, node_pk: &PublicKey, ping_id: Option<sha256::Digest>, request_id: u64) -> InnerOnionAnnounceRequest {
fn request(&self, node_pk: &PublicKey, ping_id: Option<PingId>, request_id: u64) -> InnerOnionAnnounceRequest {
let payload = OnionAnnounceRequestPayload {
ping_id: ping_id.unwrap_or(INITIAL_PING_ID),
search_pk: self.search_pk,
Expand All @@ -286,7 +286,7 @@ impl<'a> AnnouncePacketData<'a> {
self.request(node_pk, None, request_id)
}
/// Create `InnerOnionAnnounceRequest` for an announce request.
pub fn announce_request(&self, node_pk: &PublicKey, ping_id: sha256::Digest, request_id: u64) -> InnerOnionAnnounceRequest {
pub fn announce_request(&self, node_pk: &PublicKey, ping_id: PingId, request_id: u64) -> InnerOnionAnnounceRequest {
self.request(node_pk, Some(ping_id), request_id)
}
}
Expand Down Expand Up @@ -469,7 +469,7 @@ impl OnionClient {
let (ping_id, data_pk) = if payload.announce_status == AnnounceStatus::Found {
(None, Some(PublicKey(payload.ping_id_or_pk)))
} else {
(Some(sha256::Digest(payload.ping_id_or_pk)), None)
(Some(payload.ping_id_or_pk), None)
};

let now = clock_now();
Expand Down Expand Up @@ -1021,7 +1021,7 @@ mod tests {
keys: [gen_keypair().0, gen_keypair().0, gen_keypair().0],
path_type: OnionPathType::Udp,
};
let ping_id = sha256::hash(&[1, 2, 3]);
let ping_id = [42; 32];
let data_pk = gen_keypair().0;
let new_now = now + Duration::from_secs(1);
let other_onion_node = OnionNode {
Expand Down Expand Up @@ -1228,7 +1228,7 @@ mod tests {
// The sender should be added to close nodes
let onion_node = state.announce_list.get_node(&real_pk, &sender_pk).unwrap();
assert_eq!(onion_node.path_id, path.id());
assert_eq!(onion_node.ping_id, Some(sha256::Digest(ping_id)));
assert_eq!(onion_node.ping_id, Some(ping_id));
assert_eq!(onion_node.data_pk, None);
assert_eq!(onion_node.announce_status, AnnounceStatus::Announced);

Expand Down Expand Up @@ -1920,7 +1920,7 @@ mod tests {
state.paths_pool.path_nodes.put(node);
}

let ping_id = sha256::hash(&[1, 2, 3]);
let ping_id = [42; 32];
let now = Instant::now();

let mut nodes_key_by_addr = HashMap::new();
Expand Down Expand Up @@ -2064,7 +2064,7 @@ mod tests {
saddr,
path_id: path.id(),
// regardless of this ping_id search requests should contain 0
ping_id: Some(sha256::hash(&[1, 2, 3])),
ping_id: Some([42; 32]),
data_pk: None,
unsuccessful_pings: 0,
added_time: now,
Expand Down
19 changes: 12 additions & 7 deletions tox_core/src/onion/onion_announce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,18 @@
use std::io::{ErrorKind, Error};
use std::net::{IpAddr, SocketAddr};
use std::time::{Duration, Instant, SystemTime};
use sha2::{Digest, Sha256};
use sha2::digest::generic_array::typenum::marker_traits::Unsigned;

use tox_binary_io::*;
use tox_crypto::*;
use crate::time::*;
use tox_packet::onion::*;
use crate::dht::kbucket::Distance;

/// The type of onion ping ID which is SHA256 hash.
pub type PingId = [u8; <Sha256 as Digest>::OutputSize::USIZE];

/// Number of secret random bytes to make onion ping id unique for each node.
pub const SECRET_BYTES_SIZE: usize = 32;

Expand All @@ -28,7 +33,7 @@ pub const PING_ID_TIMEOUT: Duration = Duration::from_secs(300);
pub const ONION_ANNOUNCE_TIMEOUT: Duration = Duration::from_secs(300);

/// Create onion ping id filled with zeros.
pub const INITIAL_PING_ID: sha256::Digest = sha256::Digest([0; sha256::DIGESTBYTES]);
pub const INITIAL_PING_ID: PingId = [0; <Sha256 as Digest>::OutputSize::USIZE];

/** Entry that corresponds to announced onion node.

Expand Down Expand Up @@ -137,11 +142,11 @@ impl OnionPingData {
so this hash remains unchanged for `PING_ID_TIMEOUT`.

*/
pub fn ping_id(&self) -> sha256::Digest {
pub fn ping_id(&self) -> PingId {
let mut buf = [0; ONION_PING_DATA_SIZE];
// can not fail since buf has enough length
self.to_bytes((&mut buf, 0)).unwrap();
sha256::hash(&buf)
Sha256::digest(&buf).into()
}
}

Expand Down Expand Up @@ -176,7 +181,7 @@ impl OnionAnnounce {
so this hash remains unchanged for `PING_ID_TIMEOUT`.

*/
fn ping_id(&self, time: SystemTime, pk: PublicKey, ip_addr: IpAddr, port: u16) -> sha256::Digest {
fn ping_id(&self, time: SystemTime, pk: PublicKey, ip_addr: IpAddr, port: u16) -> PingId {
let data = OnionPingData {
secret_bytes: self.secret_bytes,
time,
Expand Down Expand Up @@ -286,18 +291,18 @@ impl OnionAnnounce {
if entry.data_pk != payload.data_pk {
// failed to find ourselves with same long term pk but different data pk
// weird case, should we remove it?
(AnnounceStatus::Failed, ping_id_2.0)
(AnnounceStatus::Failed, ping_id_2)
} else {
// successfully announced ourselves
(AnnounceStatus::Announced, ping_id_2.0)
(AnnounceStatus::Announced, ping_id_2)
}
} else {
// requested node is found by its long term pk
(AnnounceStatus::Found, entry.data_pk.0)
}
} else {
// requested node not found or failed to announce
(AnnounceStatus::Failed, ping_id_2.0)
(AnnounceStatus::Failed, ping_id_2)
}
}

Expand Down
1 change: 0 additions & 1 deletion tox_crypto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

pub use sodiumoxide::randombytes::randombytes_into;
pub use sodiumoxide::crypto::box_::*;
pub use sodiumoxide::crypto::hash::{sha256, sha512};
pub use sodiumoxide::crypto::secretbox;

pub use sodiumoxide::crypto::pwhash;
Expand Down
1 change: 1 addition & 0 deletions tox_encryptsave/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ edition = "2018"
[dependencies]
tox_crypto = { version = "0.1.0", path = "../tox_crypto" }
failure = "0.1"
sha2 = "0.9"
5 changes: 3 additions & 2 deletions tox_encryptsave/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ assert_eq!(plaintext,
*/

use failure::Fail;
use sha2::{Digest, Sha256};

use tox_crypto::pwhash::{
MEMLIMIT_INTERACTIVE, OPSLIMIT_INTERACTIVE,
Expand All @@ -33,7 +34,7 @@ use tox_crypto::{
NONCEBYTES, MACBYTES,
Nonce, PrecomputedKey,
gen_nonce,
secretbox, sha256
secretbox
};

/// Length in bytes of the salt used to encrypt/decrypt data.
Expand Down Expand Up @@ -117,7 +118,7 @@ impl PassKey {
pub fn with_salt(passphrase: &[u8], salt: Salt) -> Result<PassKey, KeyDerivationError> {
if passphrase.is_empty() { return Err(KeyDerivationError::Null) };

let sha256::Digest(passhash) = sha256::hash(passphrase);
let passhash = Sha256::digest(passphrase);
let OpsLimit(ops) = OPSLIMIT_INTERACTIVE;
let mut key = secretbox::Key([0; secretbox::KEYBYTES]);

Expand Down
1 change: 1 addition & 0 deletions tox_packet/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ nom = "5.1"
cookie-factory = "0.3"
bitflags = "1.0"
failure = "0.1"
sha2 = "0.9"
9 changes: 6 additions & 3 deletions tox_packet/src/dht/cookie.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@

use super::*;
use nom::number::complete::be_u64;
use sha2::{Digest, Sha512};
use sha2::digest::generic_array::typenum::marker_traits::Unsigned;

use std::time::SystemTime;
use std::{convert::TryInto, time::SystemTime};

use tox_binary_io::*;
use tox_crypto::*;
Expand Down Expand Up @@ -160,10 +162,11 @@ impl EncryptedCookie {
}
}
/// Calculate SHA512 hash of encrypted cookie together with nonce
pub fn hash(&self) -> sha512::Digest {
pub fn hash(&self) -> [u8; <Sha512 as Digest>::OutputSize::USIZE] {
let mut buf = [0; 112];
let (_, size) = self.to_bytes((&mut buf, 0)).unwrap();
sha512::hash(&buf[..size])
// TODO: use `Into` directly when GenericArray supports it
Sha512::digest(&buf[..size]).as_slice().try_into().unwrap()
}
}

Expand Down
14 changes: 9 additions & 5 deletions tox_packet/src/dht/crypto_handshake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@

use super::*;

use std::convert::TryInto;
use nom::map_opt;
use sha2::{Digest, Sha512};
use sha2::digest::generic_array::typenum::marker_traits::Unsigned;
use tox_binary_io::*;
use tox_crypto::*;
use crate::dht::cookie::EncryptedCookie;
Expand Down Expand Up @@ -126,7 +130,7 @@ pub struct CryptoHandshakePayload {
/// used to make sure that possible attacker can't combine payload from old
/// `CryptoHandshake` with new `Cookie` and try to do mess sending such
/// packets.
pub cookie_hash: sha512::Digest,
pub cookie_hash: [u8; <Sha512 as Digest>::OutputSize::USIZE],
/// Encrypted cookie of sender of `CryptoHandshake` packet. When node
/// receives `CryptoHandshake` it can take this cookie instead of sending
/// `CookieRequest` to obtain one.
Expand All @@ -137,7 +141,7 @@ impl FromBytes for CryptoHandshakePayload {
named!(from_bytes<CryptoHandshakePayload>, do_parse!(
base_nonce: call!(Nonce::from_bytes) >>
session_pk: call!(PublicKey::from_bytes) >>
cookie_hash: call!(sha512::Digest::from_bytes) >>
cookie_hash: map_opt!(take!(<Sha512 as Digest>::OutputSize::USIZE), |bytes: &[u8]| bytes.try_into().ok()) >>
cookie: call!(EncryptedCookie::from_bytes) >>
eof!() >>
(CryptoHandshakePayload {
Expand Down Expand Up @@ -184,7 +188,7 @@ mod tests {
CryptoHandshakePayload {
base_nonce: gen_nonce(),
session_pk: gen_keypair().0,
cookie_hash: sha512::hash(&[1, 2, 3]),
cookie_hash: [42; 64],
cookie: EncryptedCookie {
nonce: secretbox::gen_nonce(),
payload: vec![42; 88],
Expand All @@ -205,7 +209,7 @@ mod tests {
let payload = CryptoHandshakePayload {
base_nonce: gen_nonce(),
session_pk: gen_keypair().0,
cookie_hash: sha512::hash(&[1, 2, 3]),
cookie_hash: [42; 64],
cookie: EncryptedCookie {
nonce: secretbox::gen_nonce(),
payload: vec![42; 88],
Expand Down Expand Up @@ -234,7 +238,7 @@ mod tests {
let payload = CryptoHandshakePayload {
base_nonce: gen_nonce(),
session_pk: gen_keypair().0,
cookie_hash: sha512::hash(&[1, 2, 3]),
cookie_hash: [42; 64],
cookie: EncryptedCookie {
nonce: secretbox::gen_nonce(),
payload: vec![42; 88],
Expand Down
Loading