Skip to content

Commit

Permalink
Disable deprecated stream ciphers by default
Browse files Browse the repository at this point in the history
- Enable stream ciphers explicitly with stream-cipher feature
- Upgraded shadowsocks-crypto to v0.1.2

fixes #373
  • Loading branch information
zonyitoo committed Jan 4, 2021
1 parent 47169d6 commit 9000b31
Show file tree
Hide file tree
Showing 14 changed files with 67 additions and 27 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
- name: Build & Test (--no-default-features)
run: cargo test --verbose --no-default-features --no-fail-fast
- name: Build with All Features Enabled
run: cargo build --verbose --features "local-redir local-dns dns-over-tls"
run: cargo build --verbose --features "local-redir local-dns dns-over-tls dns-over-https stream-cipher"

build-windows:
runs-on: windows-latest
Expand All @@ -56,7 +56,7 @@ jobs:
- name: Build & Test (--no-default-features)
run: cargo test --verbose --no-default-features --no-fail-fast
- name: Build with All Features Enabled
run: cargo build --verbose --features "local-dns dns-over-tls"
run: cargo build --verbose --features "local-dns dns-over-tls dns-over-https stream-cipher"

build-macos:
runs-on: macos-latest
Expand Down Expand Up @@ -84,4 +84,4 @@ jobs:
- name: Build & Test (--no-default-features)
run: cargo test --verbose --no-default-features --no-fail-fast
- name: Build with All Features Enabled
run: cargo build --verbose --features "local-redir local-dns dns-over-tls"
run: cargo build --verbose --features "local-redir local-dns dns-over-tls dns-over-https stream-cipher"
32 changes: 16 additions & 16 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,12 @@ tcmalloc-vendored = ["tcmalloc/bundled"]
# Enable tokio's multi-threaded runtime
multi-threaded = ["tokio/rt-multi-thread"]

# Enable Stream Cipher Protocol
# WARN: Stream Cipher Protocol is proved to be insecured
# https://github.com/shadowsocks/shadowsocks-rust/issues/373
# Users should always avoid using these ciphers in practice
stream-cipher = ["shadowsocks-service/stream-cipher"]

[dependencies]
log = "0.4"
log4rs = { version = "1.0", optional = true }
Expand Down
6 changes: 6 additions & 0 deletions crates/shadowsocks-service/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ local-tunnel = ["local"]
# Enable socks4 protocol for sslocal
local-socks4 = ["local"]

# Enable Stream Cipher Protocol
# WARN: Stream Cipher Protocol is proved to be insecured
# https://github.com/shadowsocks/shadowsocks-rust/issues/373
# Users should always avoid using these ciphers in practice
stream-cipher = ["shadowsocks/stream-cipher"]

[dependencies]
log = "0.4"
log4rs = "1.0"
Expand Down
1 change: 1 addition & 0 deletions crates/shadowsocks-service/src/local/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub async fn run(mut config: Config) -> io::Result<()> {
trace!("{:?}", config);

// Warning for Stream Ciphers
#[cfg(feature = "stream-cipher")]
for server in config.server.iter() {
if server.method().is_stream() {
warn!("stream cipher {} for server {} have inherent weaknesses (see discussion in https://github.com/shadowsocks/shadowsocks-org/issues/36). \
Expand Down
1 change: 1 addition & 0 deletions crates/shadowsocks-service/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ pub async fn run(config: Config) -> io::Result<()> {
trace!("{:?}", config);

// Warning for Stream Ciphers
#[cfg(feature = "stream-cipher")]
for server in config.server.iter() {
if server.method().is_stream() {
warn!("stream cipher {} for server {} have inherent weaknesses (see discussion in https://github.com/shadowsocks/shadowsocks-org/issues/36). \
Expand Down
10 changes: 8 additions & 2 deletions crates/shadowsocks/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ default = [
# Uses trust-dns instead of tokio's builtin DNS resolver
trust-dns = ["trust-dns-resolver"]

# Enable Stream Cipher Protocol
# WARN: Stream Cipher Protocol is proved to be insecured
# https://github.com/shadowsocks/shadowsocks-rust/issues/373
# Users should always avoid using these ciphers in practice
stream-cipher = ["shadowsocks-crypto/v1-stream"]

[dependencies]
log = "0.4"

Expand Down Expand Up @@ -50,10 +56,10 @@ tokio = { version = "1.0", features = ["io-util", "macros", "net", "parking_lot"
trust-dns-resolver = { version = "0.20", optional = true }

[target.'cfg(any(target_arch = "x86_64", target_arch = "aarch64"))'.dependencies]
shadowsocks-crypto = { version = "0.1", features = ["ring"] }
shadowsocks-crypto = { version = "0.1.2", features = ["ring"] }

[target.'cfg(not(any(target_arch = "x86_64", target_arch = "aarch64")))'.dependencies]
shadowsocks-crypto = { version = "0.1", features = [] }
shadowsocks-crypto = { version = "0.1.2", features = [] }


[target.'cfg(windows)'.dependencies]
Expand Down
15 changes: 11 additions & 4 deletions crates/shadowsocks/src/relay/tcprelay/crypto_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,23 @@ use crate::{
crypto::v1::{random_iv_or_salt, CipherCategory, CipherKind},
};

use super::{
aead::{DecryptedReader as AeadDecryptedReader, EncryptedWriter as AeadEncryptedWriter},
stream::{DecryptedReader as StreamDecryptedReader, EncryptedWriter as StreamEncryptedWriter},
};
use super::aead::{DecryptedReader as AeadDecryptedReader, EncryptedWriter as AeadEncryptedWriter};
#[cfg(feature = "stream-cipher")]
use super::stream::{DecryptedReader as StreamDecryptedReader, EncryptedWriter as StreamEncryptedWriter};

/// Reader for reading encrypted data stream from shadowsocks' tunnel
pub enum DecryptedReader {
None,
Aead(AeadDecryptedReader),
#[cfg(feature = "stream-cipher")]
Stream(StreamDecryptedReader),
}

impl DecryptedReader {
/// Create a new reader for reading encrypted data
pub fn new(method: CipherKind, key: &[u8]) -> DecryptedReader {
match method.category() {
#[cfg(feature = "stream-cipher")]
CipherCategory::Stream => DecryptedReader::Stream(StreamDecryptedReader::new(method, key)),
CipherCategory::Aead => DecryptedReader::Aead(AeadDecryptedReader::new(method, key)),
CipherCategory::None => DecryptedReader::None,
Expand All @@ -51,6 +52,7 @@ impl DecryptedReader {
S: AsyncRead + Unpin + ?Sized,
{
match *self {
#[cfg(feature = "stream-cipher")]
DecryptedReader::Stream(ref mut reader) => reader.poll_read_decrypted(cx, context, stream, buf),
DecryptedReader::Aead(ref mut reader) => reader.poll_read_decrypted(cx, context, stream, buf),
DecryptedReader::None => Pin::new(stream).poll_read(cx, buf),
Expand All @@ -62,13 +64,15 @@ impl DecryptedReader {
pub enum EncryptedWriter {
None,
Aead(AeadEncryptedWriter),
#[cfg(feature = "stream-cipher")]
Stream(StreamEncryptedWriter),
}

impl EncryptedWriter {
/// Create a new writer for writing encrypted data
pub fn new(method: CipherKind, key: &[u8], nonce: &[u8]) -> EncryptedWriter {
match method.category() {
#[cfg(feature = "stream-cipher")]
CipherCategory::Stream => EncryptedWriter::Stream(StreamEncryptedWriter::new(method, key, nonce)),
CipherCategory::Aead => EncryptedWriter::Aead(AeadEncryptedWriter::new(method, key, nonce)),
CipherCategory::None => EncryptedWriter::None,
Expand All @@ -87,6 +91,7 @@ impl EncryptedWriter {
S: AsyncWrite + Unpin + ?Sized,
{
match *self {
#[cfg(feature = "stream-cipher")]
EncryptedWriter::Stream(ref mut writer) => writer.poll_write_encrypted(cx, stream, buf),
EncryptedWriter::Aead(ref mut writer) => writer.poll_write_encrypted(cx, stream, buf),
EncryptedWriter::None => Pin::new(stream).poll_write(cx, buf),
Expand All @@ -113,12 +118,14 @@ impl<S> CryptoStream<S> {
}

let prev_len = match category {
#[cfg(feature = "stream-cipher")]
CipherCategory::Stream => method.iv_len(),
CipherCategory::Aead => method.salt_len(),
CipherCategory::None => 0,
};

let iv = match category {
#[cfg(feature = "stream-cipher")]
CipherCategory::Stream => {
let local_iv = loop {
let mut iv = vec![0u8; prev_len];
Expand Down
1 change: 1 addition & 0 deletions crates/shadowsocks/src/relay/tcprelay/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ mod aead;
pub mod crypto_io;
pub mod proxy_listener;
pub mod proxy_stream;
#[cfg(feature = "stream-cipher")]
mod stream;
pub mod utils;
8 changes: 6 additions & 2 deletions crates/shadowsocks/src/relay/tcprelay/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,18 @@ where
pub fn alloc_encrypted_read_buffer(method: CipherKind) -> Box<[u8]> {
match method.category() {
CipherCategory::Aead => vec![0u8; super::aead::MAX_PACKET_SIZE + method.tag_len()].into_boxed_slice(),
CipherCategory::Stream | CipherCategory::None => vec![0u8; 1 << 14].into_boxed_slice(),
#[cfg(feature = "stream-cipher")]
CipherCategory::Stream => vec![0u8; 1 << 14].into_boxed_slice(),
CipherCategory::None => vec![0u8; 1 << 14].into_boxed_slice(),
}
}

/// Create a buffer for reading from plain channel (not encrypted), for copying data into encrypted channel
pub fn alloc_plain_read_buffer(method: CipherKind) -> Box<[u8]> {
match method.category() {
CipherCategory::Aead => vec![0u8; super::aead::MAX_PACKET_SIZE].into_boxed_slice(),
CipherCategory::Stream | CipherCategory::None => vec![0u8; 1 << 14].into_boxed_slice(),
#[cfg(feature = "stream-cipher")]
CipherCategory::Stream => vec![0u8; 1 << 14].into_boxed_slice(),
CipherCategory::None => vec![0u8; 1 << 14].into_boxed_slice(),
}
}
5 changes: 5 additions & 0 deletions crates/shadowsocks/src/relay/udprelay/crypto_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,13 @@ pub fn encrypt_payload(
addr.write_to_buf(dst);
dst.put_slice(payload);
}
#[cfg(feature = "stream-cipher")]
CipherCategory::Stream => encrypt_payload_stream(context, method, key, addr, payload, dst),
CipherCategory::Aead => encrypt_payload_aead(context, method, key, addr, payload, dst),
}
}

#[cfg(feature = "stream-cipher")]
fn encrypt_payload_stream(
context: &Context,
method: CipherKind,
Expand Down Expand Up @@ -157,10 +159,13 @@ pub async fn decrypt_payload(
}
}
}
#[cfg(feature = "stream-cipher")]
CipherCategory::Stream => decrypt_payload_stream(context, method, key, payload).await,
CipherCategory::Aead => decrypt_payload_aead(context, method, key, payload).await,
}
}

#[cfg(feature = "stream-cipher")]
async fn decrypt_payload_stream(
context: &Context,
method: CipherKind,
Expand Down
1 change: 1 addition & 0 deletions crates/shadowsocks/tests/tcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ async fn tcp_tunnel_aead() {
.unwrap();
}

#[cfg(feature = "stream-cipher")]
#[tokio::test]
async fn tcp_tunnel_stream() {
let _ = env_logger::try_init();
Expand Down
1 change: 1 addition & 0 deletions crates/shadowsocks/tests/udp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ async fn udp_tunnel_aead() {
.unwrap();
}

#[cfg(feature = "stream-cipher")]
#[tokio::test]
async fn udp_tunnel_stream() {
let _ = env_logger::try_init();
Expand Down
1 change: 1 addition & 0 deletions tests/socks5.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ impl Socks5TestServer {
}
}

#[cfg(feature = "stream-cipher")]
#[tokio::test]
async fn socks5_relay_stream() {
let _ = env_logger::try_init();
Expand Down

0 comments on commit 9000b31

Please sign in to comment.