Re: Missing signed-off for pkg chacha20 patches

[Jon Simons <jon@xxxxxxxxxxxxx>]

> I'm sorry but I'm still having issues running the pkd tests :-(
>
> $ cat tests/pkd/torture_pkd_dropbear_dsa_hmac_sha2_512.err
> WARNING: Ignoring unknown option -v
> dbclient: This Dropbear program does not support 'hmac-sha2-512' MAC algorithm
>
> dbclient: Exited: No valid MACs specified for '-m'
> $ rpm -q dropbear
> dropbear-2018.76-2.fc28.x86_64
>
> dbclient -m help foo
> dbclient: Available MACs:
> hmac-sha1-96,hmac-sha1,hmac-sha2-256
>
>
> Somehow hmac-sha2-512 doesn't seem to be supported. I've opened:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1593457

In the attached patch I've moved hmac-sha2-512 to an OpenSSH-only
section of the tests.  With this I observe that the tests are
passing with the dropbear package as used on each of Fedora 26
and Fedora 28.

> Also can you disable the dropbear tests if dbclient hasn't been found?

There is code already which skips the dropbear tests if no
`dbclient` is found when the test is run; see the function
`is_dropbear_client_enabled`.

> You can also use find_program() in cmake and set it in a 'pkd_config.h' like
> file if it is available or not.
>
> You can also do parsing of ssh options with cmake if this makes it easier ...

I've attached a patch which reorganizes the MAC lists, fixes
a test problem I observe with dropbear, and also fixes the
`-v` error above.  Please let me know if you think I should
adjust it further.


Thanks,
-Jon

From 66b3ccf9c4734b818af5b534b4a4a009ddb60741 Mon Sep 17 00:00:00 2001
From: Jon Simons <jon@xxxxxxxxxxxxx>
Date: Wed, 20 Jun 2018 20:12:39 -0700
Subject: [PATCH 1/3] pkd: omit `-v` for `dbclient` by default

The `-v` is only recognized by `dbclient` when dropbear is built
in its DEBUG_TRACE mode.  Omit that flag by default to avoid a
warning log emitted to stderr.

Signed-off-by: Jon Simons <jon@xxxxxxxxxxxxx>
---
 tests/pkd/pkd_client.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/pkd/pkd_client.h b/tests/pkd/pkd_client.h
index 4f9b48b..da40a7c 100644
--- a/tests/pkd/pkd_client.h
+++ b/tests/pkd/pkd_client.h
@@ -82,7 +82,6 @@
     DROPBEAR_BINARY " "      \
     "-y -y "                 \
     "-i " CLIENT_ID_FILE " " \
-    "-v "                    \
     "1> %s.out "             \
     "2> %s.err "
 
-- 
2.1.4


From 39cd0ba21b0c4f9d264f38db81764973bda1b090 Mon Sep 17 00:00:00 2001
From: Jon Simons <jon@xxxxxxxxxxxxx>
Date: Wed, 20 Jun 2018 20:46:59 -0700
Subject: [PATCH 2/3] pkd: break out early in final `!ssh_is_connected` loop

When testing with dropbear I observe that it is likely that the
loop removed here will encounter an error because the channel
at hand has already been closed by the dropbear client-side.

To fix the problem, break out early if `ssh_is_connected` is
no longer true upon completion of the poll.

The loop removed here intends to gracefully wait for the
connection to close; it can be removed without affecting the
validity of the test.  Connection state will be cleaned up in
the `goto out` exit path.

Signed-off-by: Jon Simons <jon@xxxxxxxxxxxxx>
---
 tests/pkd/pkd_daemon.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/tests/pkd/pkd_daemon.c b/tests/pkd/pkd_daemon.c
index a128c8e..9d7e1b2 100644
--- a/tests/pkd/pkd_daemon.c
+++ b/tests/pkd/pkd_daemon.c
@@ -368,19 +368,11 @@ static int pkd_exec_hello(int fd, struct pkd_daemon_args *args) {
            (pkd_state.eof_received == 0) &&
            (pkd_state.close_received == 0)) {
         rc = ssh_event_dopoll(e, 1000 /* milliseconds */);
-        if (rc == SSH_ERROR) {
-            pkderr("ssh_event_dopoll for eof + close: %s\n", ssh_get_error(s));
-            break;
-        } else {
+        if (ssh_is_connected(s) == 0) {
             rc = 0;
-        }
-    }
-
-    while ((ctx.keep_going != 0) &&
-           (ssh_is_connected(s))) {
-        rc = ssh_event_dopoll(e, 1000 /* milliseconds */);
-        if (rc == SSH_ERROR) {
-            pkderr("ssh_event_dopoll for session connection: %s\n", ssh_get_error(s));
+            break;
+        } else if (rc == SSH_ERROR) {
+            pkderr("ssh_event_dopoll for eof + close: %s\n", ssh_get_error(s));
             break;
         } else {
             rc = 0;
-- 
2.1.4


From 429e6d1f1fdece56ed2f577365e10b2413a0ca90 Mon Sep 17 00:00:00 2001
From: Jon Simons <jon@xxxxxxxxxxxxx>
Date: Thu, 21 Jun 2018 00:35:51 -0400
Subject: [PATCH 3/3] pkd: move `hmac-sha2-256` to OpenSSH-only lists

Signed-off-by: Jon Simons <jon@xxxxxxxxxxxxx>
---
 tests/pkd/pkd_hello.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/tests/pkd/pkd_hello.c b/tests/pkd/pkd_hello.c
index 15edf16..76a877d 100644
--- a/tests/pkd/pkd_hello.c
+++ b/tests/pkd/pkd_hello.c
@@ -354,7 +354,8 @@ static int torture_pkd_setup_ecdsa_521(void **state) {
     f(client, dsa_hmac_sha2_256,        maccmd("hmac-sha2-256"),  setup_dsa,        teardown) \
     f(client, ecdsa_256_hmac_sha2_256,  maccmd("hmac-sha2-256"),  setup_ecdsa_256,  teardown) \
     f(client, ecdsa_384_hmac_sha2_256,  maccmd("hmac-sha2-256"),  setup_ecdsa_384,  teardown) \
-    f(client, ecdsa_521_hmac_sha2_256,  maccmd("hmac-sha2-256"),  setup_ecdsa_521,  teardown) \
+    f(client, ecdsa_521_hmac_sha2_256,  maccmd("hmac-sha2-256"),  setup_ecdsa_521,  teardown)
+#define PKDTESTS_MAC_OPENSSHONLY(f, client, maccmd) \
     f(client, rsa_hmac_sha2_512,        maccmd("hmac-sha2-512"),  setup_rsa,        teardown) \
     f(client, dsa_hmac_sha2_512,        maccmd("hmac-sha2-512"),  setup_dsa,        teardown) \
     f(client, ecdsa_256_hmac_sha2_512,  maccmd("hmac-sha2-512"),  setup_ecdsa_256,  teardown) \
@@ -370,7 +371,8 @@ static int torture_pkd_setup_ecdsa_521(void **state) {
     f(client, rsa_hmac_sha2_256,        maccmd("hmac-sha2-256"),  setup_rsa,        teardown) \
     f(client, ecdsa_256_hmac_sha2_256,  maccmd("hmac-sha2-256"),  setup_ecdsa_256,  teardown) \
     f(client, ecdsa_384_hmac_sha2_256,  maccmd("hmac-sha2-256"),  setup_ecdsa_384,  teardown) \
-    f(client, ecdsa_521_hmac_sha2_256,  maccmd("hmac-sha2-256"),  setup_ecdsa_521,  teardown) \
+    f(client, ecdsa_521_hmac_sha2_256,  maccmd("hmac-sha2-256"),  setup_ecdsa_521,  teardown)
+#define PKDTESTS_MAC_OPENSSHONLY(f, client, maccmd) \
     f(client, rsa_hmac_sha2_512,        maccmd("hmac-sha2-512"),  setup_rsa,        teardown) \
     f(client, ecdsa_256_hmac_sha2_512,  maccmd("hmac-sha2-512"),  setup_ecdsa_256,  teardown) \
     f(client, ecdsa_384_hmac_sha2_512,  maccmd("hmac-sha2-512"),  setup_ecdsa_384,  teardown) \
@@ -429,6 +431,7 @@ PKDTESTS_KEX(emit_keytest, openssh_dsa, OPENSSH_KEX_CMD)
 PKDTESTS_CIPHER(emit_keytest, openssh_dsa, OPENSSH_CIPHER_CMD)
 PKDTESTS_CIPHER_OPENSSHONLY(emit_keytest, openssh_dsa, OPENSSH_CIPHER_CMD)
 PKDTESTS_MAC(emit_keytest, openssh_dsa, OPENSSH_MAC_CMD)
+PKDTESTS_MAC_OPENSSHONLY(emit_keytest, openssh_dsa, OPENSSH_MAC_CMD)
 #undef CLIENT_ID_FILE
 #endif
 
@@ -438,6 +441,7 @@ PKDTESTS_KEX(emit_keytest, openssh_rsa, OPENSSH_KEX_CMD)
 PKDTESTS_CIPHER(emit_keytest, openssh_rsa, OPENSSH_CIPHER_CMD)
 PKDTESTS_CIPHER_OPENSSHONLY(emit_keytest, openssh_rsa, OPENSSH_CIPHER_CMD)
 PKDTESTS_MAC(emit_keytest, openssh_rsa, OPENSSH_MAC_CMD)
+PKDTESTS_MAC_OPENSSHONLY(emit_keytest, openssh_rsa, OPENSSH_MAC_CMD)
 #undef CLIENT_ID_FILE
 
 #define CLIENT_ID_FILE OPENSSH_ECDSA256_TESTKEY
@@ -446,6 +450,7 @@ PKDTESTS_KEX(emit_keytest, openssh_e256, OPENSSH_KEX_CMD)
 PKDTESTS_CIPHER(emit_keytest, openssh_e256, OPENSSH_CIPHER_CMD)
 PKDTESTS_CIPHER_OPENSSHONLY(emit_keytest, openssh_e256, OPENSSH_CIPHER_CMD)
 PKDTESTS_MAC(emit_keytest, openssh_e256, OPENSSH_MAC_CMD)
+PKDTESTS_MAC_OPENSSHONLY(emit_keytest, openssh_e256, OPENSSH_MAC_CMD)
 #undef CLIENT_ID_FILE
 
 /* Could add these passes, too: */
@@ -458,6 +463,7 @@ PKDTESTS_KEX(emit_keytest, openssh_ed, OPENSSH_KEX_CMD)
 PKDTESTS_CIPHER(emit_keytest, openssh_ed, OPENSSH_CIPHER_CMD)
 PKDTESTS_CIPHER_OPENSSHONLY(emit_keytest, openssh_ed, OPENSSH_CIPHER_CMD)
 PKDTESTS_MAC(emit_keytest, openssh_ed, OPENSSH_MAC_CMD)
+PKDTESTS_MAC_OPENSSHONLY(emit_keytest, openssh_ed, OPENSSH_MAC_CMD)
 #undef CLIENT_ID_FILE
 
 #define CLIENT_ID_FILE DROPBEAR_RSA_TESTKEY
@@ -495,6 +501,7 @@ struct {
     PKDTESTS_CIPHER(emit_testmap, openssh_dsa, OPENSSH_CIPHER_CMD)
     PKDTESTS_CIPHER_OPENSSHONLY(emit_testmap, openssh_dsa, OPENSSH_CIPHER_CMD)
     PKDTESTS_MAC(emit_testmap, openssh_dsa, OPENSSH_MAC_CMD)
+    PKDTESTS_MAC_OPENSSHONLY(emit_testmap, openssh_dsa, OPENSSH_MAC_CMD)
 #endif
 
     PKDTESTS_DEFAULT(emit_testmap, openssh_rsa, OPENSSH_CMD)
@@ -502,18 +509,21 @@ struct {
     PKDTESTS_CIPHER(emit_testmap, openssh_rsa, OPENSSH_CIPHER_CMD)
     PKDTESTS_CIPHER_OPENSSHONLY(emit_testmap, openssh_rsa, OPENSSH_CIPHER_CMD)
     PKDTESTS_MAC(emit_testmap, openssh_rsa, OPENSSH_MAC_CMD)
+    PKDTESTS_MAC_OPENSSHONLY(emit_testmap, openssh_rsa, OPENSSH_MAC_CMD)
 
     PKDTESTS_DEFAULT(emit_testmap, openssh_e256, OPENSSH_CMD)
     PKDTESTS_KEX(emit_testmap, openssh_e256, OPENSSH_KEX_CMD)
     PKDTESTS_CIPHER(emit_testmap, openssh_e256, OPENSSH_CIPHER_CMD)
     PKDTESTS_CIPHER_OPENSSHONLY(emit_testmap, openssh_e256, OPENSSH_CIPHER_CMD)
     PKDTESTS_MAC(emit_testmap, openssh_e256, OPENSSH_MAC_CMD)
+    PKDTESTS_MAC_OPENSSHONLY(emit_testmap, openssh_e256, OPENSSH_MAC_CMD)
 
     PKDTESTS_DEFAULT(emit_testmap, openssh_ed, OPENSSH_CMD)
     PKDTESTS_KEX(emit_testmap, openssh_ed, OPENSSH_KEX_CMD)
     PKDTESTS_CIPHER(emit_testmap, openssh_ed, OPENSSH_CIPHER_CMD)
     PKDTESTS_CIPHER_OPENSSHONLY(emit_testmap, openssh_ed, OPENSSH_CIPHER_CMD)
     PKDTESTS_MAC(emit_testmap, openssh_ed, OPENSSH_MAC_CMD)
+    PKDTESTS_MAC_OPENSSHONLY(emit_testmap, openssh_ed, OPENSSH_MAC_CMD)
 
     /* Dropbear */
     PKDTESTS_DEFAULT(emit_testmap, dropbear, DROPBEAR_CMD)
@@ -542,6 +552,7 @@ static int pkd_run_tests(void) {
         PKDTESTS_CIPHER(emit_unit_test_comma, openssh_dsa, OPENSSH_CIPHER_CMD)
         PKDTESTS_CIPHER_OPENSSHONLY(emit_unit_test_comma, openssh_dsa, OPENSSH_CIPHER_CMD)
         PKDTESTS_MAC(emit_unit_test_comma, openssh_dsa, OPENSSH_MAC_CMD)
+        PKDTESTS_MAC_OPENSSHONLY(emit_unit_test_comma, openssh_dsa, OPENSSH_MAC_CMD)
 #endif
 
         PKDTESTS_DEFAULT(emit_unit_test_comma, openssh_rsa, OPENSSH_CMD)
@@ -549,18 +560,21 @@ static int pkd_run_tests(void) {
         PKDTESTS_CIPHER(emit_unit_test_comma, openssh_rsa, OPENSSH_CIPHER_CMD)
         PKDTESTS_CIPHER_OPENSSHONLY(emit_unit_test_comma, openssh_rsa, OPENSSH_CIPHER_CMD)
         PKDTESTS_MAC(emit_unit_test_comma, openssh_rsa, OPENSSH_MAC_CMD)
+        PKDTESTS_MAC_OPENSSHONLY(emit_unit_test_comma, openssh_rsa, OPENSSH_MAC_CMD)
 
         PKDTESTS_DEFAULT(emit_unit_test_comma, openssh_e256, OPENSSH_CMD)
         PKDTESTS_KEX(emit_unit_test_comma, openssh_e256, OPENSSH_KEX_CMD)
         PKDTESTS_CIPHER(emit_unit_test_comma, openssh_e256, OPENSSH_CIPHER_CMD)
         PKDTESTS_CIPHER_OPENSSHONLY(emit_unit_test_comma, openssh_e256, OPENSSH_CIPHER_CMD)
         PKDTESTS_MAC(emit_unit_test_comma, openssh_e256, OPENSSH_MAC_CMD)
+        PKDTESTS_MAC_OPENSSHONLY(emit_unit_test_comma, openssh_e256, OPENSSH_MAC_CMD)
 
         PKDTESTS_DEFAULT(emit_unit_test_comma, openssh_ed, OPENSSH_CMD)
         PKDTESTS_KEX(emit_unit_test_comma, openssh_ed, OPENSSH_KEX_CMD)
         PKDTESTS_CIPHER(emit_unit_test_comma, openssh_ed, OPENSSH_CIPHER_CMD)
         PKDTESTS_CIPHER_OPENSSHONLY(emit_unit_test_comma, openssh_ed, OPENSSH_CIPHER_CMD)
         PKDTESTS_MAC(emit_unit_test_comma, openssh_ed, OPENSSH_MAC_CMD)
+        PKDTESTS_MAC_OPENSSHONLY(emit_unit_test_comma, openssh_ed, OPENSSH_MAC_CMD)
     };
 
     const struct CMUnitTest dropbear_tests[] = {
-- 
2.1.4