902171 - Support choosing the appropriate subset of the cipher suite to run according to the type of build

Status: RESOLVED FIXED

(NSS :: Test, defect)

Description

[Elio Maldonado]

Attachment: Modifies cipher.sh so it chooses the crypto tests to run according to what's available with the build.

We need to support choosing the appropriate cipher test to run according to the type of build. This is an issue anly for downstream builds.

On Fedora and Red Hat Enterprise Linux, we take advantage of two built time environment variables, NSS_BUILD_WITHOUT_SOFTOKEN and NSS_BUILD_SOFTOKEN_ONLY, respectively build, as their name imply, nss without softoken sources in the build source and softoken without the rest of nss. As part of each build we run the appropriate subset of the test to run. We can run all of the tests. 

As of 3.15.1 there was a change of the cyptographic tests. Before, all of the crypto test were run via blaptest and now we also run gcm tests via pk11gcmtest. The blapitest uses the low level blapi API whereas pk11gcmtest uses the high-level PK11_ wrapper API. This causes a problem downstream because when doing an softoken-only builds PK11_ wrappers are not available to our downstream builds.

Comment 1

[Elio Maldonado]

Attachment: Choose gcm or blapi test according to what's available in the build

Removed unwanted portions used only on fedora.

Comment 2

[Ryan Sleevi]

Elio: I also noticed today when updating cipher.txt to support PSS and OAEP that the ECDSA tests are not being run.

Would you be amenable to updating your patch to include support for NSS_ENABLE_ECC in some way?

Comment 3

[Elio Maldonado]

(In reply to Ryan Sleevi from comment #2)
> Elio: I also noticed today when updating cipher.txt to support PSS and OAEP
> that the ECDSA tests are not being run.
> 
> Would you be amenable to updating your patch to include support for
> NSS_ENABLE_ECC in some way?

I don't think it's a good idea because we shoulld keep separate concerns separate. Phis patch is meant to address selecting a subset of test to run according to the type of build whereas adding support for NSS_ENABLE_ECC is to the expand test data using by bltest which is a separate issue best handled with its own bug. I suggest filing a separate bug for it.

- Elio

Comment 4

[Wan-Teh Chang]

Comment on attachment 786509 [details] [diff] [review]
Choose gcm or blapi test according to what's available in the build

Review of attachment 786509 [details] [diff] [review]:
-----------------------------------------------------------------

r=wtc. I suggest restructuring the if-elseif-else statement
to make it easier to accommodate new cipher_xxx tests in
the future.

Nit: capitalize "NSS" in the log messages or comments.

::: tests/cipher/cipher.sh
@@ +136,5 @@
> +    echo "Skipping cipher_main because this an nss without softoken build"
> +    cipher_gcm
> +else
> +    # default is to run both
> +    cipher_init

Delete this duplicate cipher_init call. (This may have been a merge error.)

@@ +139,5 @@
> +    # default is to run both
> +    cipher_init
> +    cipher_main
> +    cipher_gcm
> +fi

I suggest rewriting this whole thing without using "else".
This allows us to handle all three cases using two "if"
statements.

# Skip cipher_main if this an NSS without softoken build.
if [ "${NSS_BUILD_WITHOUT_SOFTOKEN}" != "1" ]; then
    cipher_main
fi
# Skip cipher_gcm if this is a softoken only build.
if [ "${NSS_BUILD_SOFTOKEN_ONLY}" != "1" ]; then
    cipher_gcm
fi

This structure will make it easier to handle more cipher_xxx
tests in cipher.sh in the future. If it is important to emit
the messages, you can write use two if-else statements. For
example, the first "if" statement would become:

if [ "${NSS_BUILD_WITHOUT_SOFTOKEN}" = "1" ]; then
    echo "Skipping cipher_main because this an NSS without softoken build"
else
    cipher_main
fi

Comment 5

[Elio Maldonado]

Thank you Wan-Teh, I opted for your first proposal.
Pushed: https://hg.mozilla.org/projects/nss/rev/b694614a2e89