Support PowerPC architecture for sigv4 signature (#1847)

* Use hmac and sha2 instead of ring on powerpc * Enable aws-sig-auth in CI * Update CHANGELOG * Run tests against exotic platforms * Run tests only against aws rust runtime * PowerPC 32 and 64 bit should be fully testable now * Maybe this time build and test will work * Add licence header to hmac.rs * Properly use finalized_fixed * Revert leftover * Temporary disable crc32c test on powerpc * Temporary disable system_time_conversion_test on 32bit CPUs * Disable other 3 tests on 32bit * Temporarily disable last test * Update CHANGELOG and document TODOs with issues * Run aws-smithy-client tests in CI with crosscompiled local openssl * Simplify CI script * Use correct curl options * Use the right OS for i686 * Looks like I finally foung the right os type for i686 * Add `tcp` feature to `hyper` to get tests compiling. * Enable verbose logging to debug CI failure in cross. * Use pre-built openSSL on i686 * Fix empty spaces. * Set environment variables based on matrix.target * Remove all usages of `ring` from `aws-sigv4`. It ensures broader platform compatibility and higher performance. * Update changelog entries. * Remove redundant dev dependencies. Co-authored-by: Luca Palmieri <[email protected]>
smithy-lang · Oct 25, 2022 · 7e666da · 7e666da
1 parent 1d9aede
commit 7e666da
Show file tree

Hide file tree

Showing 10 changed files with 192 additions and 43 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -144,28 +144,41 @@ jobs:
         done
 
   # We make sure that Smithy-rs can be compiled on platforms that are not natively supported by GitHub actions.
-  # We do not run tests on those platforms (yet) because it'd require a more complicated setup involving architecture
-  # emulation via QEMU, likely to cause a significant degradation on CI completion time.
+  # We run as many tests we can on those platforms because they require a more complicated setup involving architecture
+  # emulation.
   test-exotic-platform-support:
     name: Exotic platform support
     runs-on: ubuntu-latest
     strategy:
       fail-fast: false
       matrix:
         include:
+        # We always exclude aws-smithy-http-server-python since the Python framework is experimental.
+        # We only build the `native-tls` feature here because `rustls` depends on `ring` which in turn
+        # does not support powerpc as a target platform (see https://github.com/briansmith/ring/issues/389)
         - target: i686-unknown-linux-gnu
-          non_aws_features: --all-features
-          aws_excludes: ''
-        # We only test `native-tls` here because `rustls` depends on `ring` which in turn does not support powerpc
-        # as a target platform (see https://github.com/briansmith/ring/issues/389)
-        # We also exclude all first-party crates that have a non-optional dependency on `ring`.
+          build_smithy_rs_features: --all-features
+          build_aws_exclude: ''
+          build_smithy_rs_exclude: --exclude aws-smithy-http-server-python
+          test_smithy_rs_features: --all-features
+          test_aws_exclude: ''
+          test_smithy_rs_exclude: --exclude aws-smithy-http-server-python
         - target: powerpc-unknown-linux-gnu
-          non_aws_features: --features native-tls
-          aws_excludes: --exclude aws-inlineable --exclude aws-sigv4 --exclude aws-sig-auth
+          build_smithy_rs_features: --features native-tls
+          build_aws_exclude: --exclude aws-inlineable
+          build_smithy_rs_exclude: --exclude aws-smithy-http-server-python
+          test_smithy_rs_features: --features native-tls
+          test_aws_exclude: --exclude aws-inlineable
+          test_smithy_rs_exclude: --exclude aws-smithy-http-server-python
+        - target: powerpc64-unknown-linux-gnu
+          build_smithy_rs_features: --features native-tls
+          build_aws_exclude: --exclude aws-inlineable
+          build_smithy_rs_exclude: --exclude aws-smithy-http-server-python
+          test_smithy_rs_features: --features native-tls
+          test_aws_exclude: --exclude aws-inlineable
+          test_smithy_rs_exclude: --exclude aws-smithy-http-server-python
     env:
       CROSS_CONFIG: Cross.toml
-      OPENSSL_LIB_DIR: /usr/lib/i386-linux-gnu
-      OPENSSL_INCLUDE_DIR: /usr/include/i386-linux-gnu
     steps:
     - name: Checkout
       uses: actions/checkout@v1
@@ -181,30 +194,61 @@ jobs:
         profile: minimal
         override: true
         target: ${{ matrix.target }}
+    - name: Sets OpenSSL env vars on i686
+      run: |
+        echo "OPENSSL_LIB_DIR=/usr/lib/i386-linux-gnu" >> $GITHUB_ENV
+        echo "OPENSSL_INCLUDE_DIR=/usr/include/i386-linux-gnu" >> $GITHUB_ENV
+      if: matrix.target == 'i686-unknown-linux-gnu'
+    - name: Sets OpenSSL env vars on ppc and ppc64
+      run: |
+        echo "OPENSSL_DIR=/openssl" >> $GITHUB_ENV
+      if: matrix.target != 'i686-unknown-linux-gnu'
     - name: Configure cross
       shell: bash
+      # configure and cross compile openssl locally on ppc and ppc64 to be able to run aws-smithy-client tests.
+      # since cross dropped support for openssl, we use the build script from version 0.16.
       run: |
         cat > Cross.toml << EOF
-        [build]
+        [target.i686-unknown-linux-gnu]
         pre-build = ["dpkg --add-architecture i386", "apt-get update && apt-get install --assume-yes pkg-config:i386 libssl-dev:i386"]
-        [build.env]
+        [target.i686-unknown-linux-gnu.env]
         passthrough = [
             "OPENSSL_LIB_DIR",
             "OPENSSL_INCLUDE_DIR",
         ]
+        [target.powerpc-unknown-linux-gnu]
+        pre-build = ["curl -L -s -o /tmp/openssl.sh https://github.com/cross-rs/cross/raw/c183ee37a9dc6b0e6b6a6ac9c918173137bad4ef/docker/openssl.sh && bash /tmp/openssl.sh linux-ppc powerpc-linux-gnu-"]
+        [target.powerpc-unknown-linux-gnu.env]
+        passthrough = ["OPENSSL_DIR"]
+        [target.powerpc64-unknown-linux-gnu]
+        pre-build = ["curl -L -s -o /tmp/openssl.sh https://github.com/cross-rs/cross/raw/c183ee37a9dc6b0e6b6a6ac9c918173137bad4ef/docker/openssl.sh && bash /tmp/openssl.sh linux-ppc64 powerpc64-linux-gnu-"]
+        [target.powerpc64-unknown-linux-gnu.env]
+        passthrough = ["OPENSSL_DIR"]
         EOF
-    - name: Build rust-runtime crates
+    - name: Build Smithy-rs rust-runtime crates
       uses: actions-rs/cargo@v1
       with:
         use-cross: true
         command: build
-        args: --target ${{ matrix.target }} --manifest-path "rust-runtime/Cargo.toml" --exclude aws-smithy-http-server-python --workspace ${{ matrix.non_aws_features }}
+        args: -vv --target ${{ matrix.target }} --manifest-path "rust-runtime/Cargo.toml" ${{ matrix.build_smithy_rs_exclude }} --workspace ${{ matrix.build_smithy_rs_features }}
     - name: Build AWS rust-runtime crates
       uses: actions-rs/cargo@v1
       with:
         use-cross: true
         command: build
-        args: --target ${{ matrix.target }} --manifest-path "aws/rust-runtime/Cargo.toml" ${{ matrix.aws_excludes }} --workspace
+        args: -vv --target ${{ matrix.target }} --manifest-path "aws/rust-runtime/Cargo.toml" ${{ matrix.build_aws_exclude }} --workspace
+    - name: Test Smithy-rs rust-runtime crates
+      uses: actions-rs/cargo@v1
+      with:
+        use-cross: true
+        command: test
+        args: --target ${{ matrix.target }} --manifest-path "rust-runtime/Cargo.toml" ${{ matrix.test_smithy_rs_exclude }} --workspace ${{ matrix.test_smithy_rs_features }}
+    - name: Test AWS rust-runtime crates
+      uses: actions-rs/cargo@v1
+      with:
+        use-cross: true
+        command: test
+        args: --target ${{ matrix.target }} --manifest-path "aws/rust-runtime/Cargo.toml" ${{ matrix.test_aws_exclude }} --workspace
 
   # This job is split out from the rest since it is not required to pass for merge
   check-sdk-examples:

diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml
@@ -9,4 +9,16 @@
 # message = "Fix typos in module documentation for generated crates"
 # references = ["smithy-rs#920"]
 # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}
-# author = "rcoh"
+# author = "rcoh"
+
+[[smithy-rs]]
+message = "Support Sigv4 signature generation on PowerPC 32 and 64 bit. This architecture cannot compile `ring`, so the implementation has been updated to rely on `hamc` + `sha2` to achive the same result with broader platform compatibility and higher performance. We also updated the CI which is now running as many tests as possible against i686 and PowerPC 32 and 64 bit."
+references = ["smithy-rs#1847"]
+meta = { "breaking" = false, "tada" = false, "bug" = true }
+author = "crisidev"
+
+[[aws-sdk-rust]]
+message = "Support Sigv4 signature generation on PowerPC 32 and 64 bit. This architecture cannot compile `ring`, so the implementation has been updated to rely on `hamc` + `sha2` to achive the same result with broader platform compatibility and higher performance. We also updated the CI which is now running as many tests as possible against i686 and PowerPC 32 and 64 bit."
+references = ["smithy-rs#1847"]
+meta = { "breaking" = true, "tada" = false, "bug" = true }
+author = "crisidev"
diff --git a/aws/rust-runtime/aws-sigv4/Cargo.toml b/aws/rust-runtime/aws-sigv4/Cargo.toml
@@ -23,17 +23,26 @@ http = { version = "0.2", optional = true }
 once_cell = "1.8"
 percent-encoding = { version = "2.1", optional = true }
 regex = "1.5"
-ring = "0.16"
 time = "0.3.5"
 tracing = "0.1"
+hmac = "0.12"
+sha2 = "0.10"
 
 [dev-dependencies]
+criterion = "0.4"
 bytes = "1"
 httparse = "1.5"
 pretty_assertions = "1.0"
 proptest = "1"
 time = { version = "0.3.4", features = ["parsing"] }
 
+[target.'cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))'.dev-dependencies]
+ring = "0.16"
+
+[[bench]]
+name = "hmac"
+harness = false
+
 [package.metadata.docs.rs]
 all-features = true
 targets = ["x86_64-unknown-linux-gnu"]

diff --git a/aws/rust-runtime/aws-sigv4/benches/hmac.rs b/aws/rust-runtime/aws-sigv4/benches/hmac.rs
@@ -0,0 +1,69 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+use criterion::{criterion_group, criterion_main, Criterion};
+use hmac::digest::FixedOutput;
+use hmac::{Hmac, Mac};
+#[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))]
+use ring::hmac::{sign, Context, Key, HMAC_SHA256};
+use sha2::Sha256;
+
+pub fn hmac(c: &mut Criterion) {
+    c.bench_function("hmac", |b| {
+        b.iter(|| {
+            let mut mac = Hmac::<Sha256>::new_from_slice(b"secret").unwrap();
+
+            mac.update(b"hello, world");
+            mac.finalize_fixed()
+        })
+    });
+}
+
+#[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))]
+pub fn ring_multipart(c: &mut Criterion) {
+    c.bench_function("ring_multipart", |b| {
+        b.iter(|| {
+            let k = Key::new(HMAC_SHA256, b"secret");
+            let mut ctx = Context::with_key(&k);
+
+            for slice in ["hello", ", ", "world"] {
+                ctx.update(slice.as_ref());
+            }
+
+            ctx.sign()
+        })
+    });
+}
+
+#[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))]
+pub fn ring_one_shot(c: &mut Criterion) {
+    c.bench_function("ring_one_shot", |b| {
+        b.iter(|| {
+            let k = Key::new(HMAC_SHA256, b"secret");
+
+            sign(&k, b"hello, world")
+        })
+    });
+}
+
+#[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))]
+criterion_group! {
+    name = benches;
+
+    config = Criterion::default();
+
+    targets = hmac, ring_multipart, ring_one_shot
+}
+
+#[cfg(any(target_arch = "powerpc", target_arch = "powerpc64"))]
+criterion_group! {
+    name = benches;
+
+    config = Criterion::default();
+
+    targets = hmac
+}
+
+criterion_main!(benches);
diff --git a/aws/rust-runtime/aws-sigv4/external-types.toml b/aws/rust-runtime/aws-sigv4/external-types.toml
@@ -6,9 +6,6 @@ allowed_external_types = [
     "http::request::Request",
     "http::uri::Uri",
 
-    # TODO(https://github.com/awslabs/smithy-rs/issues/1193): Don't expose on `ring`
-    "ring::hmac::Tag",
-
     # TODO(https://github.com/awslabs/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `event-stream` feature
     "aws_smithy_eventstream::frame::Message",
 ]
diff --git a/aws/rust-runtime/aws-sigv4/src/date_time.rs b/aws/rust-runtime/aws-sigv4/src/date_time.rs
@@ -95,6 +95,8 @@ mod tests {
     use crate::date_time::test_parsers::{parse_date, parse_date_time};
     use time::format_description::well_known::Rfc3339;
 
+    // TODO(https://github.com/awslabs/smithy-rs/issues/1857)
+    #[cfg(not(any(target_arch = "powerpc", target_arch = "x86")))]
     #[test]
     fn date_format() {
         let time: SystemTime = OffsetDateTime::parse("2039-02-04T23:01:09.104Z", &Rfc3339)
@@ -107,6 +109,8 @@ mod tests {
         assert_eq!("01000102", format_date(time));
     }
 
+    // TODO(https://github.com/awslabs/smithy-rs/issues/1857)
+    #[cfg(not(any(target_arch = "powerpc", target_arch = "x86")))]
     #[test]
     fn date_time_format() {
         let time: SystemTime = OffsetDateTime::parse("2039-02-04T23:01:09.104Z", &Rfc3339)
@@ -131,6 +135,8 @@ mod tests {
         assert_eq!("20150830", format_date(time));
     }
 
+    // TODO(https://github.com/awslabs/smithy-rs/issues/1857)
+    #[cfg(not(any(target_arch = "powerpc", target_arch = "x86")))]
     #[test]
     fn test_truncate_subsecs() {
         let time: SystemTime = OffsetDateTime::parse("2039-02-04T23:01:09.104Z", &Rfc3339)

diff --git a/aws/rust-runtime/aws-sigv4/src/sign.rs b/aws/rust-runtime/aws-sigv4/src/sign.rs
@@ -6,24 +6,24 @@
 //! Functions to create signing keys and calculate signatures.
 
 use crate::date_time::format_date;
-use ring::{
-    digest::{self},
-    hmac::{self, Key, Tag},
-};
+use hmac::{digest::FixedOutput, Hmac, Mac};
+use sha2::{Digest, Sha256};
 use std::time::SystemTime;
 
 /// HashedPayload = Lowercase(HexEncode(Hash(requestPayload)))
 #[allow(dead_code)] // Unused when compiling without certain features
 pub(crate) fn sha256_hex_string(bytes: impl AsRef<[u8]>) -> String {
-    // hex::encode returns a lowercase string
-    hex::encode(digest::digest(&digest::SHA256, bytes.as_ref()))
+    let mut hasher = Sha256::new();
+    hasher.update(bytes);
+    hex::encode(hasher.finalize_fixed())
 }
 
 /// Calculates a Sigv4 signature
-pub fn calculate_signature(signing_key: Tag, string_to_sign: &[u8]) -> String {
-    let s_key = Key::new(hmac::HMAC_SHA256, signing_key.as_ref());
-    let tag = hmac::sign(&s_key, string_to_sign);
-    hex::encode(tag)
+pub fn calculate_signature(signing_key: impl AsRef<[u8]>, string_to_sign: &[u8]) -> String {
+    let mut mac = Hmac::<Sha256>::new_from_slice(signing_key.as_ref())
+        .expect("HMAC can take key of any size");
+    mac.update(string_to_sign);
+    hex::encode(mac.finalize_fixed())
 }
 
 /// Generates a signing key for Sigv4
@@ -32,28 +32,33 @@ pub fn generate_signing_key(
     time: SystemTime,
     region: &str,
     service: &str,
-) -> hmac::Tag {
+) -> impl AsRef<[u8]> {
     // kSecret = your secret access key
     // kDate = HMAC("AWS4" + kSecret, Date)
     // kRegion = HMAC(kDate, Region)
     // kService = HMAC(kRegion, Service)
     // kSigning = HMAC(kService, "aws4_request")
 
     let secret = format!("AWS4{}", secret);
-    let secret = hmac::Key::new(hmac::HMAC_SHA256, secret.as_bytes());
-    let tag = hmac::sign(&secret, format_date(time).as_bytes());
+    let mut mac =
+        Hmac::<Sha256>::new_from_slice(secret.as_ref()).expect("HMAC can take key of any size");
+    mac.update(format_date(time).as_bytes());
+    let tag = mac.finalize_fixed();
 
     // sign region
-    let key = hmac::Key::new(hmac::HMAC_SHA256, tag.as_ref());
-    let tag = hmac::sign(&key, region.as_bytes());
+    let mut mac = Hmac::<Sha256>::new_from_slice(&tag).expect("HMAC can take key of any size");
+    mac.update(region.as_bytes());
+    let tag = mac.finalize_fixed();
 
     // sign service
-    let key = hmac::Key::new(hmac::HMAC_SHA256, tag.as_ref());
-    let tag = hmac::sign(&key, service.as_bytes());
+    let mut mac = Hmac::<Sha256>::new_from_slice(&tag).expect("HMAC can take key of any size");
+    mac.update(service.as_bytes());
+    let tag = mac.finalize_fixed();
 
     // sign request
-    let key = hmac::Key::new(hmac::HMAC_SHA256, tag.as_ref());
-    hmac::sign(&key, "aws4_request".as_bytes())
+    let mut mac = Hmac::<Sha256>::new_from_slice(&tag).expect("HMAC can take key of any size");
+    mac.update("aws4_request".as_bytes());
+    mac.finalize_fixed()
 }
 
 #[cfg(test)]

diff --git a/rust-runtime/aws-smithy-checksums/src/lib.rs b/rust-runtime/aws-smithy-checksums/src/lib.rs
@@ -340,6 +340,9 @@ mod tests {
         assert_eq!(decoded_checksum, expected_checksum);
     }
 
+    // TODO(https://github.com/zowens/crc32c/issues/34)
+    // TODO(https://github.com/awslabs/smithy-rs/issues/1857)
+    #[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))]
     #[test]
     fn test_crc32c_checksum() {
         let mut checksum = Crc32c::default();

diff --git a/rust-runtime/aws-smithy-client/Cargo.toml b/rust-runtime/aws-smithy-client/Cargo.toml
@@ -24,8 +24,11 @@ bytes = "1"
 fastrand = "1.4.0"
 http = "0.2.3"
 http-body = "0.4.4"
-hyper = { version = "0.14.12", features = ["client", "http2", "http1"], optional = true }
-hyper-rustls = { version = "0.23.0", optional = true, features = ["rustls-native-certs", "http2"] }
+hyper = { version = "0.14.12", features = ["client", "http2", "http1", "tcp"], optional = true }
+# cargo does not support optional test dependencies, so to completely disable rustls when
+# the native-tls feature is enabled, we need to add the webpki-roots feature here.
+# https://github.com/rust-lang/cargo/issues/1596
+hyper-rustls = { version = "0.23.0", optional = true, features = ["rustls-native-certs", "http2", "webpki-roots"] }
 hyper-tls = { version = "0.5.0", optional = true }
 lazy_static = { version = "1", optional = true }
 pin-project-lite = "0.2.7"
@@ -36,7 +39,6 @@ tracing = "0.1"
 
 [dev-dependencies]
 aws-smithy-async = { path = "../aws-smithy-async", features = ["rt-tokio"] }
-hyper-rustls = { version = "0.23.0", features = ["webpki-roots"] }
 serde = { version = "1", features = ["derive"] }
 serde_json = "1"
 tokio = { version = "1.8.4", features = ["full", "test-util"] }

diff --git a/rust-runtime/aws-smithy-types/src/date_time/mod.rs b/rust-runtime/aws-smithy-types/src/date_time/mod.rs
@@ -526,6 +526,8 @@ mod test {
         assert!(DateTime::from_nanos(10_000_000_000_000_000_000_999_999_999_i128).is_err());
     }
 
+    // TODO(https://github.com/awslabs/smithy-rs/issues/1857)
+    #[cfg(not(any(target_arch = "powerpc", target_arch = "x86")))]
     #[test]
     fn system_time_conversions() {
         // Check agreement