Skip to content
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

CI: 32bit testing #3903

Closed
dougch opened this issue Mar 22, 2023 · 4 comments
Closed

CI: 32bit testing #3903

dougch opened this issue Mar 22, 2023 · 4 comments

Comments

@dougch
Copy link
Contributor

dougch commented Mar 22, 2023

Security issue notifications

If you discover a potential security issue in s2n we ask that you notify
AWS Security via our vulnerability reporting page. Please do not create a public github issue.

Problem:

We'd like to be testing on more platforms, including 32bit Linux. Recent issues (#3896) highlight there is clearly a need.

Solution:

As called out in a GHA issue 32bit would likely involve qemu.

An rough test with qemu-user-static on CodeBuild does appear to be one possible solution.

More discovery is needed; what are other projects with this need using ?

Requirements / Acceptance Criteria:

What must a solution address in order to solve the problem? How do we know the solution is complete? Unit tests run and pass on a 32bit linux platform.

Out of scope:

Is there anything the solution will intentionally NOT address?

@camshaft
Copy link
Contributor

@jmayclin
Copy link
Contributor

The current plan for this is to add a cross-compiling 32 bit build into our CI.

This allows us to get 32 bit build coverage without having to add the complexity of an emulator into our build platform. The build would probably be run on a multi-arch Ubuntu 22.04? docker container which could install the libssl-dev:i386 package. I was able to confirm that this build reproduced the build errors seen in the earlier 32 bit CI issues.

We also want to be running unit tests for the 32 bit platform (we could just use 32 bit emulation on a 64 bit platform, which many platforms have native support for), but some of those are currently broken. The above PR address one of the failures (related to s2n_connection struct size), but the other failures are related to 2038 issues.

These 2038 issues happen in two broad categories

  1. won't affect us until 2038 (time_t will only be the current system time)
  2. affects us now (time_t is some arbitrary date that may be in the future)

We experience both type 1 and type 2 failures in our unit tests, because we mock the system clock to be arbitrary future dates which are past 2038.

There are a number of ways to approach these problems, but my current preferred solution is to use the glibc 2038 proofing approach which allows us to set time_t to be 64 bits on a 32 bit platform. This is also supported by musl. This enables us to fix all of the remaining unit tests without any code refactoring.

However, this approach requires that the _TIME_BITS=64 compile definition is added for all libraries that pass time_t over api boundaries to s2n. In api/s2n.h there aren't any methods that use time_t, so this requirement would only affect libcrypto.

This means that libcrypto would need to be compiled from source with the _TIME_BITS=64 compile definition. I was able to successfully test this with AWS-LC, but we would want them to add a nice toggle option for this in the CMakeLists.txt.

@jmayclin
Copy link
Contributor

Individual unit test tracking issue: #3948

@jmayclin
Copy link
Contributor

jmayclin commented May 6, 2023

There is now a 32 bit cross-compiling job running in CI. The 32BitBuildAndTest is run as part of the s2nGeneralBatch CI job. I'll be closing this issue out, although the unit test tracking issue will remain open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants
@camshaft @toidiu @lrstewart @jmayclin @dougch and others