valimail/arc_test_suite

arc_test_suite's Issues

Signing suite documentation needed around standard formatting

The test suite readme.md makes it clear that certain standard formatting is required for the signing suite to run.

However, once the test suite is installed, it is never clear that running the signing suite requires the references implementation to match said formatting. The suite itself (either as part of the -h output, or when running the signing suite) should make it clear what is needed for the signing suite to properly run.

the arc_test_suite should not have hardcoded paths to external environments

The openarc runners all reference files that exist within https://github.com/ValiMail/OpenARCDev, but this relationship should not be assumed.

Both of these files contains hardcoded paths that should be removed:

• https://github.com/ValiMail/arc_test_suite/blob/master/runners/signopenarc.py
• https://github.com/ValiMail/arc_test_suite/blob/master/runners/validateopenarc.py

validateopenarc.py also has two other issues:

• a hardcoded user of "vagrant", which is never correct unless running within OpenARCDev.
• a referenced pem file from the dev environment that actually already lives in https://github.com/ValiMail/arc_test_suite/blob/master/tests/arc-draft-sign-tests.yml

It might make sense to have a configuration file for these, and then OpenARCDev can just have its own settings file with appropriate paths.

arc test suite issues with new dkimpy

I've recently released dkimpy 0.7.0. When I try to run the test suite against it, I get quite a number of errors for signing and the verification tests just hang. It's possible I'm doing it wrong, but it would be nice to see the test suite updated to match the current ARC document revisions and some feedback about potential issues in dkimpy before I release 0.7.1 (there's already one bug identified in the ARC code that I should fix soon).

Add ARC result to AR (Section 5.2)

Need to add signing test to verify ARC results are added to AR

The end result of this Validation algorithm SHOULD be included within
the Authentication-Results header field for the ADMD.

Update current test cases for add ARC result to AR (Section 5.2)

Need to update current test cases to include ARC results in ADMD AR.

Must work with “oldest-pass” value, since it's part of the protocol (Section 5.2, Step 5)

Need to add signing/verification tests for proper usage.

Test suite openarc runners need appropriate mode flags

The configs loaded in the runners now need to set the appropriate mode flags (s / v)

Updates for RFC 8617

Now that the IETF published RFC 8617, we should update the test suite.

In particular, the instance tag for AMS and AS headers now MUST appear before all other tags (e.g. see the ABNF in § 4.1.3)

c= tag is not required

The result for ams_fields_c_na is incorrect. c= is not a required tag in a DKIM signature and there's nothing in ARC to specify it should be otherwise for AMS.

Test suite needs a better mechanism for dealing with different signing schemes

Right now, the test suite signing suite needs the validated implementation to sign in a very specific way, detailed here (https://github.com/ValiMail/arc_test_suite#signing-header-format-standardization).

Realistically, this is a bad idea, as it creates too high a bar for implementations to check their signing status at best, and at worst introduces divergent code paths for testing within implementations.

Instead, there should be a way to define how the signed messages are generated by the implementation, and the test suite should be able to generate signing tests to match.

While this will generate a suite of signing tests instead of a single test, this will make the test suite far more useable for those with differing implementations.

Use smtp.remote-ip, not obsolete terms (Optional, but need to use correctly)

Use of smtp.remote-ip is optional, so explicit testing for it doesn't need to be added to the test suite, but it changed names during ARC development. Use of obsolete names should be tested for and raise an error.

Presence of smtp.remote-ip should not cause problems when verifying.

Test for CFWS around authserv-id

From RFC7601, section 2.2

authres-header = "Authentication-Results:" [CFWS] authserv-id
[ CFWS authres-version ]
( no-result / 1*resinfo ) [CFWS] CRLF

For example
Authentication-Results: (testing) lists.example.org (test); arc=none;

Is a valid Authentication-Results header, Mail::DKIM was not doing the right thing with an authserv-id followed by a comment.

arc-draft-sign-tests.yml should include this use case, and possible one with an authres-version, and should set an expectation for what should be added in the corresponding AAR.

Deprecate SHA-1 cv passes

The consensus of the DMARC work group (https://www.ietf.org/mail-archive/web/dmarc/current/msg04040.html) is that SHA-1 tests should fail the test suite now that DCRUP has deprecated SHA-1.

Due to this, two tests should be switched from cv=pass to cv=fail:

• ams_fields_a_sha1
• as_fields_a_sha1

ARC does not use h= tag from DKIM for AS

Need new signing and verification tests for this

Must not add DKIM h= tag (Section 4.1.3)
Must fail if DKIM h= tag is present (Section 4.1.3)
Must verify without using h= tag to determine signed headers (Section 4.1.3)

The only supported tags are "i" (from Section 4.2.1 of this
document), and "a", "b", "d, "s", "t" from [RFC6376] Section 3.5.
Note especially that the DKIM "h" tag is NOT allowed and if found,
MUST result in a cv status of "fail" (for more information see
Section 5.1.1);

Test suite output needs to be parseable

Right now, the test suite output is poorly formatted and impossible to parse programmatically due to inconsistent quoting of strings and random newlines in the output.

There should be consistent output (on either stdout or stderr) that can be parsed appropriately and separately from all other output.

Suggested output, for each test, per line:
[test name] | [test status] | [test output]

e.g.:

cv_fail_i1_as_na | fail | '' != 'fail'

Must work without "oldest-pass" value, since it's optional (Section 5.2, Step 5)

Need to document the test case that officially covers this requirement (none of them have oldest-pass now, so any would do).

All ADMD results in AAR (Section 4.1.1)

Need signing tests to validate this is done.

Because there is only one AAR allowed per ARC set, the AAR MUST
contain the combined authres-payload with all of the authentication
results from within the participating ADMD, regardless of how many
Authentication-Results header fields are attached to the message.

Test case "cv_fail_i2_as1_fail" has a missing cv value to check

The cv values for the following three test cases are all missing, and should all be fail:

• cv_fail_i2_as1_fail
• cv_fail_i2_as2_fail
• cv_fail_i1_as_cv_fail

Do not seal if previous cv=fail (Section 5.1, Step 2)

Need new signing test for this

If the message already contains an Authenticated Received Chain
with the most recent AS reporting "cv=fail", then there is no
need to proceed and the algorithm stops here.

AR Fields MUST NOT included in AMS (Section 4.1.2)

Need signing tests for this.

Authentication-Results header fields MUST NOT be included in AMS
signatures as they are likely to be deleted by downstream ADMDs
(per [I-D-7601bis] Section 5).

test suite openarc validation check needs to use A-R and not ARC-Seal for determining cv value

Now that validation/sealing have been split in openarc, the test suite's method of determining the validation cv value is incorrect. It is looking cv=[status] in the ARC-Seal, which is no longer appropriate. The runner now needs to find the appropriate arc=[status] in the A-R for the local srv_id.

All previous cv MUST != fail (Section 5.2, Step 3.3)

Need verification tests for this.

The "cv" value for all ARC-Seal header fields MUST NOT be
"fail". For ARC Sets with instance values > 1, the values
MUST be "pass". For the ARC Set with instance value = 1, the
value MUST be "none".