Add mipsel to Github Actions CI setup

[LinusU]

replaces actions/toolkit#804

[LinusU]

Hmmm, seems like "allow failure" isn't a feature of Github Actions

We could add continue-on-error, but it will still show the PRs with a red cross unfortunately...

@briansmith how do you want me to proceed?

[briansmith]

I factored out the first commit into its own PR; I retained you as the author of the commit: PR actions/toolkit#1121.

[briansmith]

I see that CI/CD fails in the build.rs step. In this PR think we should fix that, and make whatever other changes we need to make until we get to the point where the build fails at the linking step. This will help us identify which features we need non-assembly fallback implementations for.

[briansmith]

PTAL at PR actions/toolkit#1122 which is an attempt to adapt build.rs to support platforms that don't have any assembly code in ring.

[bltavares]

May I help with this issue somehow? Is it ok if I create another branch with both this changes and the changes of actions/toolkit#1122 in attempt to make both work together?

[briansmith]

May I help with this issue somehow? Is it ok if I create another branch with both this changes and the changes of actions/toolkit#1122 in attempt to make both work together?

The best way to help in the immediate term would be to review the PRs that are already posted related to this work, starting with PR actions/toolkit#1122. If you can help with the reviews, then I'll update the other PRs that are required to get MIPS working and ping you for review on them too.

[bltavares]

With this new commit, built on top of actions/toolkit#1174 I've been able to run cross build --target mipsel-unknown-linux-musl successfully.

Running cross test --target mipsel-unknown-linux-musl led to errors on linking tho.

Linking errors on mips32 test

  = note: /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.52blfpsz4tzhjxcu.rcgu.o): In function `ring::aead::chacha::Key::encrypt::hcc8681e34446970f':
          /project/src/aead/chacha.rs:122: undefined reference to `GFp_ChaCha20_ctr32'
          /project/src/aead/chacha.rs:122: undefined reference to `GFp_ChaCha20_ctr32'


  = note: /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.2c2d5g13z24iodtw.rcgu.o): In function `ring::ec::curve25519::ed25519::signing::Ed25519KeyPair::from_seed_::ha24bdb15a5b5627e':
          /project/src/ec/curve25519/ed25519/signing.rs:168: undefined reference to `GFp_x25519_ge_scalarmult_base'
          /project/src/ec/curve25519/ed25519/signing.rs:168: undefined reference to `GFp_x25519_ge_scalarmult_base'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.3dpq0r9qo5ktpflo.rcgu.o): In function `_$LT$ring..ec..curve25519..ed25519..verification..EdDSAParameters$u20$as$u20$ring..signature..VerificationAlgorithm$GT$::verify::h078f9ae371c63d92':
          /project/src/ec/curve25519/ed25519/verification.rs:66: undefined reference to `GFp_x25519_ge_double_scalarmult_vartime'
          /project/src/ec/curve25519/ed25519/verification.rs:66: undefined reference to `GFp_x25519_ge_double_scalarmult_vartime'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.3sycwd2vcnqu7sq7.rcgu.o): In function `ring::ec::curve25519::ed25519::signing::Ed25519KeyPair::sign::_$u7b$$u7b$closure$u7d$$u7d$::hd63105d3186043c8':
          /project/src/ec/curve25519/ed25519/signing.rs:202: undefined reference to `GFp_x25519_ge_scalarmult_base'
          /project/src/ec/curve25519/ed25519/signing.rs:202: undefined reference to `GFp_x25519_ge_scalarmult_base'
          /project/src/ec/curve25519/ed25519/signing.rs:208: undefined reference to `GFp_x25519_sc_muladd'
          /project/src/ec/curve25519/ed25519/signing.rs:208: undefined reference to `GFp_x25519_sc_muladd'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.fl9rjju7kqpxfnb.rcgu.o): In function `ring::ec::curve25519::ops::Elem$LT$ring..ec..curve25519..ops..T$GT$::negate::h695ca2320a434fc3':
          /project/src/ec/curve25519/ops.rs:52: undefined reference to `GFp_x25519_fe_neg'
          /project/src/ec/curve25519/ops.rs:52: undefined reference to `GFp_x25519_fe_neg'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.fl9rjju7kqpxfnb.rcgu.o): In function `ring::ec::curve25519::ops::ExtPoint::from_encoded_point_vartime::h8438ba0d9425fd2c':
          /project/src/ec/curve25519/ops.rs:88: undefined reference to `GFp_x25519_ge_frombytes_vartime'
          /project/src/ec/curve25519/ops.rs:88: undefined reference to `GFp_x25519_ge_frombytes_vartime'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.fl9rjju7kqpxfnb.rcgu.o): In function `ring::ec::curve25519::ops::encode_point::h21823fb03fda3244':
          /project/src/ec/curve25519/ops.rs:129: undefined reference to `GFp_x25519_fe_invert'
          /project/src/ec/curve25519/ops.rs:129: undefined reference to `GFp_x25519_fe_invert'
          /project/src/ec/curve25519/ops.rs:132: undefined reference to `GFp_x25519_fe_mul_ttt'
          /project/src/ec/curve25519/ops.rs:132: undefined reference to `GFp_x25519_fe_mul_ttt'
          /project/src/ec/curve25519/ops.rs:135: undefined reference to `GFp_x25519_fe_mul_ttt'
          /project/src/ec/curve25519/ops.rs:135: undefined reference to `GFp_x25519_fe_mul_ttt'
          /project/src/ec/curve25519/ops.rs:136: undefined reference to `GFp_x25519_fe_tobytes'
          /project/src/ec/curve25519/ops.rs:136: undefined reference to `GFp_x25519_fe_tobytes'
          /project/src/ec/curve25519/ops.rs:138: undefined reference to `GFp_x25519_fe_isnegative'
          /project/src/ec/curve25519/ops.rs:138: undefined reference to `GFp_x25519_fe_isnegative'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.2064hr0x4dqbkh9y.rcgu.o): In function `ring::ec::curve25519::scalar::Scalar::from_sha512_digest_reduced::h92b95fd5a646905f':
          /project/src/ec/curve25519/scalar.rs:52: undefined reference to `GFp_x25519_sc_reduce'
          /project/src/ec/curve25519/scalar.rs:52: undefined reference to `GFp_x25519_sc_reduce'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.2064hr0x4dqbkh9y.rcgu.o): In function `ring::ec::curve25519::scalar::MaskedScalar::from_bytes_masked::hfde28776e4396156':
          /project/src/ec/curve25519/scalar.rs:66: undefined reference to `GFp_x25519_sc_mask'
          /project/src/ec/curve25519/scalar.rs:66: undefined reference to `GFp_x25519_sc_mask'

  = note: /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.226bs52jheiyzlc9.rcgu.o): In function `ring::ec::suite_b::ops::PrivateKeyOps::point_mul::h36e25cc80f6d8797':
          /project/src/ec/suite_b/ops.rs:188: undefined reference to `GFp_nistz384_point_mul'
          /project/src/ec/suite_b/ops.rs:188: undefined reference to `GFp_nistz384_point_mul'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.226bs52jheiyzlc9.rcgu.o): In function `ring::ec::suite_b::ops::p384::GFp_p384_elem_sqr_mont::hc32d118b71f44ef5':
          /project/src/ec/suite_b/ops/p384.rs:333: undefined reference to `GFp_p384_elem_mul_mont'
          /project/src/ec/suite_b/ops/p384.rs:333: undefined reference to `GFp_p384_elem_mul_mont'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.226bs52jheiyzlc9.rcgu.o):(.data.rel.ro._ZN4ring2ec7suite_b3ops4p38410COMMON_OPS17h553893f89b7aad9eE+0xf4): undefined reference to `GFp_p384_elem_add'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.226bs52jheiyzlc9.rcgu.o):(.data.rel.ro._ZN4ring2ec7suite_b3ops4p38410COMMON_OPS17h553893f89b7aad9eE+0xf8): undefined reference to `GFp_p384_elem_mul_mont'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.226bs52jheiyzlc9.rcgu.o):(.data.rel.ro._ZN4ring2ec7suite_b3ops4p38410COMMON_OPS17h553893f89b7aad9eE+0x100): undefined reference to `GFp_nistz384_point_add'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.226bs52jheiyzlc9.rcgu.o):(.data.rel.ro._ZN4ring2ec7suite_b3ops4p38415PRIVATE_KEY_OPS17h1493245f6dba2109E+0xc): undefined reference to `GFp_nistz384_point_mul'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.25a3hl59oajpdczn.rcgu.o): In function `ring::ec::curve25519::x25519::x25519_public_from_private::ha1994a03b915194c':
          /project/src/ec/curve25519/x25519.rs:86: undefined reference to `GFp_x25519_public_from_private_generic_masked'
          /project/src/ec/curve25519/x25519.rs:86: undefined reference to `GFp_x25519_public_from_private_generic_masked'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.25a3hl59oajpdczn.rcgu.o): In function `ring::ec::curve25519::x25519::x25519_ecdh::scalar_mult::hf0f4fff0d87913b2':
          /project/src/ec/curve25519/x25519.rs:127: undefined reference to `GFp_x25519_scalar_mult_generic_masked'
          /project/src/ec/curve25519/x25519.rs:127: undefined reference to `GFp_x25519_scalar_mult_generic_masked'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.4mw0impwp9p643ip.rcgu.o): In function `ring::ec::suite_b::ops::p256::p256_point_mul_base_impl::hc0d27046becfd9bb':
          /project/src/ec/suite_b/ops/p256.rs:168: undefined reference to `GFp_nistz256_point_mul'
          /project/src/ec/suite_b/ops/p256.rs:168: undefined reference to `GFp_nistz256_point_mul'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.4mw0impwp9p643ip.rcgu.o):(.data.rel.ro._ZN4ring2ec7suite_b3ops4p25610COMMON_OPS17h9140bdc05cd1adf2E+0xf4): undefined reference to `GFp_nistz256_add'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.4mw0impwp9p643ip.rcgu.o):(.data.rel.ro._ZN4ring2ec7suite_b3ops4p25610COMMON_OPS17h9140bdc05cd1adf2E+0xf8): undefined reference to `GFp_nistz256_mul_mont'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.4mw0impwp9p643ip.rcgu.o):(.data.rel.ro._ZN4ring2ec7suite_b3ops4p25610COMMON_OPS17h9140bdc05cd1adf2E+0xfc): undefined reference to `GFp_nistz256_sqr_mont'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.4mw0impwp9p643ip.rcgu.o):(.data.rel.ro._ZN4ring2ec7suite_b3ops4p25610COMMON_OPS17h9140bdc05cd1adf2E+0x100): undefined reference to `GFp_nistz256_point_add'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.4mw0impwp9p643ip.rcgu.o):(.data.rel.ro._ZN4ring2ec7suite_b3ops4p25615PRIVATE_KEY_OPS17h337a5c398a1c70d1E+0xc): undefined reference to `GFp_nistz256_point_mul'
          /target/mipsel-unknown-linux-musl/debug/deps/libring-01c6a71ba3329db9.rlib(ring-01c6a71ba3329db9.2064hr0x4dqbkh9y.rcgu.o): In function `ring::ec::curve25519::scalar::MaskedScalar::from_bytes_masked::hfde28776e4396156':
          /project/src/ec/curve25519/scalar.rs:66: undefined reference to `GFp_x25519_sc_mask'
          /project/src/ec/curve25519/scalar.rs:66: undefined reference to `GFp_x25519_sc_mask'

and more...

[briansmith]

Running cross test --target mipsel-unknown-linux-musl led to errors on linking tho.

Yes, that's to be expected, because we don't have platform-agnostic fallbacks for every primitive yet. PRs like actions/toolkit#1011 will help with that. I think ChaCha20-Poly1305 will be the last missing one and then we'd be good to go, IIRC.

[bltavares]

actions/toolkit#943 is looking a bit stale, but could it be used to help with ChaCha20-Poly1305?

[briansmith]

Now that actions/toolkit#1174 is merged, I think if we rebase this then it will work?

[codecov]

Codecov Report

Merging #1120 (2a0a616) into main (9accd87) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    actions/toolkit#1120      +/-   ##
==========================================
- Coverage   92.88%   92.87%   -0.01%     
==========================================
  Files         116      116              
  Lines       18270    18270              
==========================================
- Hits        16970    16969       -1     
- Misses       1300     1301       +1     

Impacted Files	Coverage Δ
src/aead/aes.rs	77.43% <0.00%> (-0.45%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9accd87...2a0a616. Read the comment docs.

[LinusU]

@briansmith I've rebased but something seems up with codecov 🤔

[briansmith]

@briansmith I've rebased but something seems up with codecov 🤔

Codecov sometimes reports the wrong results when only a subset of the jobs have reported coverage to it. It seems like it's fine now?

Anyway, it does seem like the mips jobs are failing due to warnings. It seems those failures kill the entire test matrix so we can't merge this as-is. I wonder if we should fix that in this PR or whether we should merge this with PR actions/toolkit#1181 or something else.

What do y'all think?

[briansmith]

This PR will need to be rebased on top of PR #1274 once that is merged. That should fix the warnings/errors in rand.rs.

[briansmith]

In addition to adding an entry to the test: matrix, we should add a similar entry to the coverage: matrix. (One of my primary interests in this PR is increasing the test coverage of the fallback implementations.)

[briansmith]

Anyway, it does seem like the mips jobs are failing due to warnings. It seems those failures kill the entire test matrix so we can't merge this as-is.

As I mentioned above, I think PR actions/toolkit#1274 will probably fix the build errors/warnings, or at least some of them.

I wonder if we should fix that in this PR or whether we should merge this with PR actions/toolkit#1181 or something else.

What do y'all think?

When reviewing PR actions/toolkit#1181, and in the ensuing discussion, I realized that PR actions/toolkit#1181 is probably almost completely unneeded now. So I think we should just extend this PR with whatever is needed to get the MIPS job to turn green.

[bltavares]

@briansmith I'm trying to port the changes but I'm getting quite a few undefined reference ring_core_dev_bn_mul_mont.

It seems to be related to the recent changes on prefixes. Any tips on how to troubleshoot why mips-mont.pl is not getting the prefix?

[briansmith]

@briansmith I'm trying to port the changes but I'm getting quite a few undefined reference ring_core_dev_bn_mul_mont.

It seems to be related to the recent changes on prefixes. Any tips on how to troubleshoot why mips-mont.pl is not getting the prefix?

In build.rs:

    (&[AARCH64, ARM, X86_64, X86], "crypto/curve25519/curve25519.c"),
    (&[AARCH64, ARM, X86_64, X86], "crypto/fipsmodule/ec/ecp_nistz.c"),
    (&[AARCH64, ARM, X86_64, X86], "crypto/fipsmodule/ec/gfp_p256.c"),
    (&[AARCH64, ARM, X86_64, X86], "crypto/fipsmodule/ec/gfp_p384.c"),
    (&[AARCH64, ARM, X86_64, X86], "crypto/fipsmodule/ec/p256.c"),

Replace [AARCH64, ARM, X86_64, X86] with [] and I think it will start working.

[bltavares]

I've done a similar change already, but it seems to be only happening on test mode. cross build works as expected.

[briansmith]

@LinusU Thank you very much for your changes and sorry for the extreme delay in getting to this. I made some slight changes to this and incorporated it into PR #1558, and I'm going to close this in favor of that PR.