1529950 - Improve NSS S/MIME tests for Thunderbird

Status: RESOLVED FIXED

(NSS :: Test, enhancement)

Description

[Kai Engert (:KaiE:)]

NSS already has an S/MIME test script. But actually it's really just a CMS test, because it doesn't do any MIME.

We'd like to add automated testing for S/MIME message processing to Thunderbird.

I had the choice to use openssl or NSS to generate the test data that is required by Thunderbird. I'd prefer to use NSS.

The NSS test suite already produces most of the CMS data that Thunderbird needs. With just a little bit of extra string processing, we can produce the message files, too.

I'll attach an enhancement for the smime.sh script, which creates the message for TB in an extra subdirectory. With this, Thunderbird can simply run the NSS test suite and copy those files into the Thunderbird tree.

I've modified the existing alice.txt file, to make it compatible with both the NSS CMS plaintext needs, and also the TB MIME message needs. Because the NSS CMS test doesn't need a real email message, I think it should be OK to remove the email headers from alice.txt

Comment 1

[Kai Engert (:KaiE:)]

Attachment: 1529950-v1.patch

Comment 2

[Kai Engert (:KaiE:)]

Comment on attachment 9045981 [details] [diff] [review]
1529950-v1.patch

Note this patch doesn't change the existing tests. Only one detail, the plaintext used by the existing tests will now be the modified alice.txt file. Everything else is just additions, and in the p7 function, changes to avoid a filename reuse.

Comment 3

[Kai Engert (:KaiE:)]

I see I have a duplicate "mkdir tb", I'll remove that before checkin.

Comment 4

[J.C. Jones [:jcj] (he/him)]

Note that I had started an s/mime GTest in Bug 1521174 that we'll land in just over a week. That might be a lot simpler than maintaining the shell scripts.

Comment 5

[Kai Engert (:KaiE:)]

Attachment: 1529950-v2.patch

Comment 6

[Kai Engert (:KaiE:)]

(In reply to J.C. Jones [:jcj] (he/him) from comment #4)

Note that I had started an s/mime GTest in Bug 1521174 that we'll land in just over a week. That might be a lot simpler than maintaining the shell scripts.

Thanks for reminding me, that's good to know.

Your tests are CMS correctness tests. I think it's fair to use the c++ gtest for this kind of tests. However, the producing of the additional files that I need to do is more easily done with a shell script.

(I'm doing it here for convenience, because doing it inside the Thunderbird tree would be more work. I think the vast majority of time spent in the NSS test is for the certificate generation and crypto operations. Creating these files here should be very fast, and shouldn't notably affect the time required for executing the NSS test suite.)

Comment 7

[J.C. Jones [:jcj] (he/him)]

Comment on attachment 9046120 [details] [diff] [review]
1529950-v2.patch

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

Just these. Sorry to be pedantic about the shell scripts, but trying to work a bunch of these from FreeBSD and from MacOS makes me a bit more particular :) Thanks!

::: tests/smime/smime.sh
@@ +154,5 @@
> +
> +mime_init()
> +{
> +  OUT="tb/alice.mime"
> +  echo "$header_clearsigned" >>$OUT

The rest of the file uses ${var} form. It'd be nice to be consistent, and I'm not much for the naked $var form anyway.

@@ +156,5 @@
> +{
> +  OUT="tb/alice.mime"
> +  echo "$header_clearsigned" >>$OUT
> +  cat alice.txt >>$OUT
> +  sed -i "s/\$/$CR/" $OUT

All these uses of GNU-specific sed features across the project eventually need repair. BSD sed mandates an extension after -i, even if the extension is '' like
  sed -i "" "s/\$/${CR}/" ${OUT}

@@ +184,4 @@
>  
> +  ${PROFTOOL} ${BINDIR}/cmsutil -S -T -N Alice ${HASH_CMD} -i tb/alice.mime -d ${P_R_ALICEDIR} -p nss -o tb/alice.mime.d${SIG}
> +
> +  OUT="tb/alice.d$SIG.multipart"

For these in particular, please ${SIG} it, because d$SIG looks like an assembly instruction ;)

Comment 8

[Kai Engert (:KaiE:)]

Attachment: 1529950-v3.patch

Thanks for the careful review, I appreciate it! I didn't know about the FreeBSD requirements, it's good you do.
I made the changes you requested.
Note sed -i "" gave some errors, I had to use -i"" (no space).

Comment 9

[J.C. Jones [:jcj] (he/him)]

Comment on attachment 9046419 [details] [diff] [review]
1529950-v3.patch

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

Thanks Kai :)

Comment 10

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/projects/nss/rev/65c17f18469d