wip: ppc64/ppc support by q66 · Pull Request #819 · briansmith/ring

q66 · 2019-05-08T18:48:38Z

I decided to do my own take at ppc64 (and maybe 32, but comments suggest that might not be practical) support...

Current status/TODO:

complete testsuite pass on big and little endian ppc64
figure out why ChaCha20_ctr32_vsx segfaults [edit: it's because ring does not check length for zero and the vsx assembly assumes non-zero, fixed by checking separately in GFp_ChaCha20_ctr32]
add decision logic in build.rs to support big endian ppc64 (ELFv2 and ELFv1) and 32-bit ppc
clean up build hacks
import AES ppc assembly bits and integrate them
add CI bits [powerpc64le only, there is no powerpc64 toolchain in enivornment as ancient as ubuntu 14.04]
minimize the assembly code (remove AES decryption and so on)
rename remaining functions to avoid conflicts
ppc32 support? ecp_nistz256 missing (no portable impl, no ppc32 assembly)

The big endian byteswapping hacks in chacha.rs and poly1305.rs are most likely wrong. I don't know what the right fix is though.

q66 · 2019-05-09T00:11:39Z

Build now supports all perlasm formats for ppc64 correctly. Still need to test though (and maybe fill in missing code). I think this may also work on FreeBSD ppc64 (assuming linux 64-bit big endian does), by the way (the assembly code should not differ, the ELF ABIs are the same). Someone would have to test it, though.

q66 · 2019-05-09T00:32:55Z

The ecp_nistz256 funcs don't have any 32-bit ppc assembly or generic portable implementations in OpenSSL, which might be a problem.

tarcieri · 2019-05-09T02:47:36Z

See the previous issue I opened about PPC32 support: #532

Most PPC32s cannot do constant time multiplication, and provide a "fast" multiply by 0 and 1. Together with their generally low clock speed, this can be leveraged as an exploitable timing sidechannel.

q66 · 2019-05-09T02:50:07Z

I guess the options are either to leave the 32-bit support out, or conditionally leave out the specific parts for ppc32 then. The latter seemed like it would take a fair amount of changes.

q66 · 2019-05-13T21:59:44Z

The newly added commit "fixes" testsuite on ppc64be. I kinda doubt that this solution is correct or anything, but it's where it fails. A better (or well, actual) solution would be much appreciated.

This imports the necessary assembly code as well as modifies the ring code to match and does the necessary build system integration and CI bits addition. I agree to license my contributions to each file under the terms given at the top of each file I changed.

briansmith

Thanks. This is impressive work.

I see three main phases of this:

Add big-endian support to ring.
Support some PowerPC targets in CI.
Provide optimized implementations of some algorithms for which there is a non-assembly fallback (e.g. AES).
Provide an implementation of the algorithms that are normally only available in assembly language in ring.

#4 is the one I have the most concern about because people are just now finding and fixing bugs in the PPC assembly code in OpenSSL, and BoringSSL is not even trying to use that assembly code, even though they do try to support PPC.

I suggest we break this up into four pieces:

One PR that changes base.h and adds the CI changes. For now the CI should be marked "failures allowed".
Another PR that implements all the changes to get big-endian mode to work. I am hoping the PowerPC and MIPS people can work together on this. I was surprised how little needed to be changed to do this.
Another PR that adds just the VPAES encryption, comprised of two commits: 1. The original OpenSSL code and then the diff that removes everything not needed by VPAES encrypt.
We'll need to investigate more regarding who is using the rest of the assembly code and how confident they are that it works. We may want to explore some alternatives to using the assembly code.

I propose we do #1, #2, and #3 while we figure out #4.

WDYT?

briansmith · 2019-05-18T02:27:23Z

crypto/fipsmodule/ec/ecp_nistz256_table.c

@@ -0,0 +1,9542 @@
+/*


This table is already in ring and we won't need this file.

The PPC select functions are using the scatter/gather technique, which we don't need in ring. It will be better to just remove the "scatter-gather subroutines" section of the ecp_nistz256-ppc64.pl file and then remove ecp_nistz256_table.c too.

briansmith · 2019-05-18T02:29:21Z

include/GFp/base.h

 #elif defined(__mips__) && defined(__LP64__)
 #define OPENSSL_64_BIT
 #define OPENSSL_MIPS64
+#elif defined(__powerpc64__)


Please split the changes to this file out into the "base PR."

briansmith · 2019-05-18T02:31:08Z

pregenerate_asm/Cargo.toml

 # Keep this in sync with `[build-dependencies]` in ../Cargo.toml.
 [dependencies]
 cc = "1.0.26"
+tempfile = "3.0"


Please add a comment explaining why this dependency was added. If it is only needed for PPC then the dependency should be limited to PPC.

briansmith · 2019-05-18T02:32:19Z

src/aead/chacha.rs

+            GFp_ChaCha20_ctr32(output, input, in_out_len, &key, &ctr);
+        } else {
+            GFp_ChaCha20_ctr32(output, input, in_out_len, key, ctr);
+        }


Please split all changes to Rust code needed to support big-endian to a separate "big endian" PR. I expect this will be used by MIPS big endian too.

briansmith · 2019-05-18T02:32:58Z

src/aead/poly1305.rs

+            Nonce(Block::from(nonce))
+        } else {
+            Nonce(key_and_nonce[1].clone())
+        };


This should go in the "big endian" PR too.

briansmith · 2019-05-18T02:33:49Z

src/ec/suite_b/ops.rs

+        ]
+    };
+}
+


Please move this to the "big endian" PR.

briansmith · 2019-05-18T02:33:57Z

src/ec/suite_b/ops.rs

+        ]
+    };
+}
+


q66 · 2019-05-18T09:29:54Z

Hmm, as far as I can see, the only thing that does have a portable fallback in the tree is AES encryption. As for the assembly code, I think it should be safe to use, it passes OpenSSL's testsuite, ring's testsuite, and is used on ppc64le distros all over the place, without any issues. Not sure which specific bugs you are talking about; if you mean the ELFv2 support bit i raised recently, that's not as much of a bug as a target previously unsupported in assembly (however, all the code is written cleanly, so it was trivial to add, ppc64 assembly code is typically endian independent, what's not endian independent was already properly separated, and the ELFv2 support was already there because of little-endian)

What about splitting it into three, like this:

only the code required to get little endian ppc64 running
followed by another PR that gets big endian working
followed by another PR that adds VPAES and HWAES encryption in (split into multiple commits so that one adds the support and one minimizes the asm)
maybe followed later by something for 32-bit ppc...

Still not sure about 32-bit ppc, I have no way to test that (only modern POWER hardware here) and AFAIK it's a pain to get a ppc32 rust toolchain running in the first place...

q66 · 2019-05-20T16:14:41Z

I got a 32-bit ppc environment available now (I compiled my distribution for ppc32, then made up a chroot of that in a 64-bit big endian environment, it works fine, including Rust) so I can try handling that as well. Looks like BoringSSL has some code that will possibly enable ppc32 to work.

edelsohn · 2020-01-07T18:43:01Z

You can request a VM or direct Jenkins CI account at OSUOSL for CI access.
OSUOSL Power Development Hosting.
You can list me (David Edelsohn) as the IBM Advocate.

johnjmar · 2020-02-04T17:53:52Z

Building using ring v0.14.6 (/home/ubuntu/ring_q66) in a Power 9 (ppc64le) system is yielding several
error: trait objects without an explicit 'dyn' are deprecated

q66 · 2020-02-05T20:44:54Z

i don't really care for working on this anymore, sorry

anyone who feels like picking up on it themselves can take my changes, but i won't be putting in any more active effort

The powerpc64le assembly implementations for certain algorithms are not directly callable, unlike on other architectures. Therefore, they require a small amount of preprocessing handled by some C code. This commit adds a new file derived from OpenSSL's 'ppccap.c', which provides the expected entry point functions. This commit is also derived from the original ppc64 support pull request by q66: briansmith#819 I agree to license my contributions to each file under the terms given at the top of each file I changed.

The powerpc64le assembly implementations for certain algorithms are not directly callable, unlike on other architectures. Therefore, they require a small amount of preprocessing handled by some C code. This commit adds a new file derived from OpenSSL's 'ppccap.c', which provides the expected entry point functions. This commit is also derived from the original ppc64 support pull request by q66: briansmith#819 I agree to license my contributions to each file under the terms given at the top of each file I changed. Signed-off-by: Eric Richter <[email protected]>

The powerpc64le assembly implementations for certain algorithms are not directly callable, unlike on other architectures. Therefore, they require a small amount of preprocessing handled by some C code. This commit adds a new file derived from OpenSSL's 'ppccap.c', which provides the expected entry point functions. This commit is also derived from the original ppc64 support pull request by q66: briansmith#819 I agree to license my contributions to each file under the terms given at the top of each file I changed.

The powerpc64le assembly implementations for certain algorithms are not directly callable, unlike on other architectures. Therefore, they require a small amount of preprocessing handled by some C code. This commit adds a new file derived from OpenSSL's 'ppccap.c', which provides the expected entry point functions. This commit is also derived from the original ppc64 support pull request by q66: briansmith#819 I agree to license my contributions to each file under the terms given at the top of each file I changed. Signed-off-by: Eric Richter <[email protected]>

The powerpc64le assembly implementations for certain algorithms are not directly callable, unlike on other architectures. Therefore, they require a small amount of preprocessing handled by some C code. This commit adds a new file derived from OpenSSL's 'ppccap.c', which provides the expected entry point functions. This commit is also derived from the original ppc64 support pull request by q66: briansmith#819 I agree to license my contributions to each file under the terms given at the top of each file I changed.

q66 mentioned this pull request May 8, 2019

add powerpc target #814

Closed

q66 force-pushed the ppc64 branch 3 times, most recently from 689e362 to 0720c76 Compare May 8, 2019 22:10

flier mentioned this pull request May 9, 2019

impl Digest for ring RustCrypto/hashes#82

Closed

q66 force-pushed the ppc64 branch 3 times, most recently from d7b11fc to c694afa Compare May 9, 2019 22:34

q66 force-pushed the ppc64 branch 3 times, most recently from 9d0a3c4 to 7a09426 Compare May 13, 2019 23:00

q66 mentioned this pull request May 13, 2019

Support Power/PowerPC #389

Closed

q66 force-pushed the ppc64 branch 2 times, most recently from 0a51d26 to 0d03c03 Compare May 15, 2019 16:22

q66 added 2 commits May 15, 2019 19:12

[do not merge] hacks for big endian testsuite pass

c948970

q66 force-pushed the ppc64 branch from 0d03c03 to c948970 Compare May 15, 2019 17:13

briansmith requested changes May 18, 2019

View reviewed changes

josephlr mentioned this pull request Jun 18, 2019

Use libc::SYS_getrandom #839

Closed

q66 closed this Feb 5, 2020

erichte-ibm mentioned this pull request Sep 18, 2020

Initial ppc64le Support #1057

Merged

Conversation

q66 commented May 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

q66 commented May 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

q66 commented May 9, 2019

Uh oh!

tarcieri commented May 9, 2019

Uh oh!

q66 commented May 9, 2019

Uh oh!

q66 commented May 13, 2019

Uh oh!

briansmith left a comment

Choose a reason for hiding this comment

Uh oh!

briansmith May 18, 2019

Choose a reason for hiding this comment

Uh oh!

briansmith May 18, 2019

Choose a reason for hiding this comment

Uh oh!

briansmith May 18, 2019

Choose a reason for hiding this comment

Uh oh!

briansmith May 18, 2019

Choose a reason for hiding this comment

Uh oh!

briansmith May 18, 2019

Choose a reason for hiding this comment

Uh oh!

briansmith May 18, 2019

Choose a reason for hiding this comment

Uh oh!

briansmith May 18, 2019

Choose a reason for hiding this comment

Uh oh!

q66 commented May 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

q66 commented May 20, 2019

Uh oh!

edelsohn commented Jan 7, 2020

Uh oh!

johnjmar commented Feb 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

q66 commented Feb 5, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

q66 commented May 8, 2019 •

edited

Loading

q66 commented May 9, 2019 •

edited

Loading

q66 commented May 18, 2019 •

edited

Loading

johnjmar commented Feb 4, 2020 •

edited

Loading