You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2020/05/22 04:59:33 UTC

[mina-sshd] branch master updated (ab02b9c -> a97e853)

This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git.


    from ab02b9c  Fixed timeout value for ServerProxyAcceptorTest
     new 982c842  [SSHD-997] Fixed OpenSSH RSA private key decoder
     new 5b38e81  [SSHD-997] Fixed OpenSSH ed25519 private key decoder
     new a97e853  [SSHD-997] A few minor code fixes in OpenSSHKeyPairResourceWriterTest

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../openssh/OpenSSHRSAPrivateKeyDecoder.java       |   5 +-
 .../OpenSSHEd25519PrivateKeyEntryDecoder.java      |  11 +-
 .../openssh/OpenSSHKeyPairResourceWriterTest.java  | 147 ++++++++++++++++++---
 3 files changed, 130 insertions(+), 33 deletions(-)


[mina-sshd] 01/03: [SSHD-997] Fixed OpenSSH RSA private key decoder

Posted by lg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git

commit 982c842929aa6a76bcacd22f537532a35e656daf
Author: Thomas Wolf <th...@paranor.ch>
AuthorDate: Fri May 22 07:48:10 2020 +0300

    [SSHD-997] Fixed OpenSSH RSA private key decoder
---
 .../config/keys/loader/openssh/OpenSSHRSAPrivateKeyDecoder.java      | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/openssh/OpenSSHRSAPrivateKeyDecoder.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/openssh/OpenSSHRSAPrivateKeyDecoder.java
index 9e6173b..096a413 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/openssh/OpenSSHRSAPrivateKeyDecoder.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/openssh/OpenSSHRSAPrivateKeyDecoder.java
@@ -31,7 +31,6 @@ import java.security.interfaces.RSAPrivateKey;
 import java.security.interfaces.RSAPublicKey;
 import java.security.spec.InvalidKeySpecException;
 import java.security.spec.RSAPrivateCrtKeySpec;
-import java.security.spec.RSAPrivateKeySpec;
 import java.security.spec.RSAPublicKeySpec;
 import java.util.Collections;
 import java.util.Objects;
@@ -80,9 +79,9 @@ public class OpenSSHRSAPrivateKeyDecoder extends AbstractPrivateKeyEntryDecoder<
         if (!Objects.equals(n, modulus)) {
             log.warn("decodePrivateKey({}) mismatched modulus values: encoded={}, calculated={}", keyType, n, modulus);
         }
-
         try {
-            return generatePrivateKey(new RSAPrivateKeySpec(n, d));
+            return generatePrivateKey(new RSAPrivateCrtKeySpec(
+                    n, e, d, p, q, d.mod(p.subtract(BigInteger.ONE)), d.mod(q.subtract(BigInteger.ONE)), inverseQmodP));
         } finally {
             // get rid of sensitive data a.s.a.p
             d = null;


[mina-sshd] 02/03: [SSHD-997] Fixed OpenSSH ed25519 private key decoder

Posted by lg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git

commit 5b38e8124e6c59beb8c11a6939f57b5ec9b819a4
Author: Thomas Wolf <th...@paranor.ch>
AuthorDate: Fri May 22 07:51:24 2020 +0300

    [SSHD-997] Fixed OpenSSH ed25519 private key decoder
---
 .../OpenSSHEd25519PrivateKeyEntryDecoder.java      |  11 +-
 .../openssh/OpenSSHKeyPairResourceWriterTest.java  | 135 ++++++++++++++++++---
 2 files changed, 123 insertions(+), 23 deletions(-)

diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/security/eddsa/OpenSSHEd25519PrivateKeyEntryDecoder.java b/sshd-common/src/main/java/org/apache/sshd/common/util/security/eddsa/OpenSSHEd25519PrivateKeyEntryDecoder.java
index 5e72064..b4cd0f6 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/util/security/eddsa/OpenSSHEd25519PrivateKeyEntryDecoder.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/util/security/eddsa/OpenSSHEd25519PrivateKeyEntryDecoder.java
@@ -101,15 +101,8 @@ public class OpenSSHEd25519PrivateKeyEntryDecoder extends AbstractPrivateKeyEntr
             }
 
             byte[] sk = Arrays.copyOf(keypair, SK_SIZE);
-            EdDSAPrivateKey privateKey;
-            try {
-                // create the private key
-                EdDSAParameterSpec params = EdDSANamedCurveTable.getByName(EdDSASecurityProviderUtils.CURVE_ED25519_SHA512);
-                privateKey = generatePrivateKey(new EdDSAPrivateKeySpec(sk, params));
-            } finally {
-                // get rid of sensitive data a.s.a.p
-                Arrays.fill(sk, (byte) 0);
-            }
+            EdDSAParameterSpec params = EdDSANamedCurveTable.getByName(EdDSASecurityProviderUtils.CURVE_ED25519_SHA512);
+            EdDSAPrivateKey privateKey = generatePrivateKey(new EdDSAPrivateKeySpec(sk, params));
 
             // the private key class contains the calculated public key (Abyte)
             // pointers to the corresponding code:
diff --git a/sshd-common/src/test/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriterTest.java b/sshd-common/src/test/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriterTest.java
index 050615a..f3c03e2 100644
--- a/sshd-common/src/test/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriterTest.java
+++ b/sshd-common/src/test/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriterTest.java
@@ -38,9 +38,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
-import java.util.Objects;
 
-import net.i2p.crypto.eddsa.EdDSAKey;
 import net.i2p.crypto.eddsa.spec.EdDSAGenParameterSpec;
 import org.apache.sshd.common.config.keys.AuthorizedKeyEntry;
 import org.apache.sshd.common.config.keys.FilePasswordProvider;
@@ -130,14 +128,6 @@ public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
     }
 
     private boolean compare(KeyPair a, KeyPair b) {
-        if ("EDDSA".equals(data.algorithm)) {
-            // Bug in net.i2p.crypto.eddsa and in sshd? Both also compare the
-            // seed of the private key, but for a generated key, this is some
-            // random value, while it is all zeroes for a key read from a file.
-            return KeyUtils.compareKeys(a.getPublic(), b.getPublic())
-                    && Objects.equals(((EdDSAKey) a.getPrivate()).getParams(),
-                            ((EdDSAKey) b.getPrivate()).getParams());
-        }
         // Compares both public and private keys.
         return KeyUtils.compareKeyPairs(a, b);
     }
@@ -156,6 +146,115 @@ public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
     }
 
     @Test
+    public void testFileRoundtripNoEncryption() throws Exception {
+        Path tmp = getTemporaryOutputFile();
+        try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
+            OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(testKey, "a comment", null, out);
+            writeToFile(tmp, out.toByteArray());
+        }
+        try (InputStream in = Files.newInputStream(tmp)) {
+            KeyPair key = SecurityUtils.loadKeyPairIdentities(null,
+                    new PathResource(tmp), in, null).iterator().next();
+            assertNotNull("No key pair parsed", key);
+            assertKeyPairEquals("Mismatched recovered keys", testKey, key);
+            assertTrue("Keys should be equal", compare(key, testKey));
+            Path tmp2 = getTemporaryOutputFile("again");
+            try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
+                OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(key, "a comment", null, out);
+                writeToFile(tmp2, out.toByteArray());
+            }
+            try (InputStream in2 = Files.newInputStream(tmp2)) {
+                KeyPair key2 = SecurityUtils.loadKeyPairIdentities(null,
+                        new PathResource(tmp2), in2, null).iterator().next();
+                assertNotNull("No key pair parsed", key2);
+                assertKeyPairEquals("Mismatched recovered keys", testKey, key2);
+                assertTrue("Keys should be equal", compare(key2, testKey));
+
+                assertKeyPairEquals("Mismatched recovered keys", key, key2);
+                assertTrue("Keys should be equal", compare(key2, key));
+            }
+        }
+    }
+
+    @Test
+    public void testFileRoundtripWithEncryption() throws Exception {
+        Path tmp = getTemporaryOutputFile();
+        try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
+            OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
+            options.setPassword("nonsense");
+            options.setCipherName("AES");
+            options.setCipherMode("CTR");
+            options.setCipherType("256");
+            OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(testKey, "a comment", options, out);
+            writeToFile(tmp, out.toByteArray());
+        }
+        try (InputStream in = Files.newInputStream(tmp)) {
+            KeyPair key = SecurityUtils.loadKeyPairIdentities(null,
+                    new PathResource(tmp), in, FilePasswordProvider.of("nonsense")).iterator().next();
+            assertNotNull("No key pair parsed", key);
+            assertKeyPairEquals("Mismatched recovered keys", testKey, key);
+            assertTrue("Keys should be equal", compare(key, testKey));
+            Path tmp2 = getTemporaryOutputFile("again");
+            try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
+                OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
+                options.setPassword("nonsense");
+                options.setCipherName("AES");
+                options.setCipherMode("CTR");
+                options.setCipherType("256");
+                OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(key, "a comment", options, out);
+                writeToFile(tmp2, out.toByteArray());
+            }
+            try (InputStream in2 = Files.newInputStream(tmp2)) {
+                KeyPair key2 = SecurityUtils.loadKeyPairIdentities(null,
+                        new PathResource(tmp2), in2, FilePasswordProvider.of("nonsense")).iterator().next();
+                assertNotNull("No key pair parsed", key2);
+                assertKeyPairEquals("Mismatched recovered keys", testKey, key2);
+                assertTrue("Keys should be equal", compare(key2, testKey));
+
+                assertKeyPairEquals("Mismatched recovered keys", key, key2);
+                assertTrue("Keys should be equal", compare(key2, key));
+            }
+        }
+    }
+
+    @Test
+    public void testFileRoundtripAsymmetric() throws Exception {
+        // Write first unencrypted, then encrypted. read both and compare.
+        Path tmp = getTemporaryOutputFile();
+        try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
+            OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(testKey, "a comment", null, out);
+            writeToFile(tmp, out.toByteArray());
+        }
+        try (InputStream in = Files.newInputStream(tmp)) {
+            KeyPair key = SecurityUtils.loadKeyPairIdentities(null,
+                    new PathResource(tmp), in, null).iterator().next();
+            assertNotNull("No key pair parsed", key);
+            assertKeyPairEquals("Mismatched recovered keys", testKey, key);
+            assertTrue("Keys should be equal", compare(key, testKey));
+            Path tmp2 = getTemporaryOutputFile("again");
+            try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
+                OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
+                options.setPassword("nonsense");
+                options.setCipherName("AES");
+                options.setCipherMode("CTR");
+                options.setCipherType("256");
+                OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(key, "a comment", options, out);
+                writeToFile(tmp2, out.toByteArray());
+            }
+            try (InputStream in2 = Files.newInputStream(tmp2)) {
+                KeyPair key2 = SecurityUtils.loadKeyPairIdentities(null,
+                        new PathResource(tmp2), in2, FilePasswordProvider.of("nonsense")).iterator().next();
+                assertNotNull("No key pair parsed", key2);
+                assertKeyPairEquals("Mismatched recovered keys", testKey, key2);
+                assertTrue("Keys should be equal", compare(key2, testKey));
+
+                assertKeyPairEquals("Mismatched recovered keys", key, key2);
+                assertTrue("Keys should be equal", compare(key2, key));
+            }
+        }
+    }
+
+    @Test
     public void testWritePrivateKeyNoEncryption() throws Exception {
         Path tmp = getTemporaryOutputFile();
         try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
@@ -330,22 +429,30 @@ public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
         checkPublicKey(tmp, null);
     }
 
-    private Path getTemporaryOutputFile() throws IOException {
+    private Path getTemporaryOutputFile(String suffix) throws IOException {
         Path dir = createTempClassFolder();
         String testName = getCurrentTestName();
         int pos = testName.indexOf('[');
-        Path file;
+        String fileName;
         if (pos > 0) {
             String baseName = testName.substring(0, pos);
             String paramName = testName.substring(pos + 1, testName.length() - 1);
-            file = dir.resolve(baseName + "-" + paramName.replace('(', '-').replace(")", "").trim());
+            fileName = baseName + "-" + paramName.replace('(', '-').replace(")", "").trim();
         } else {
-            file = dir.resolve(testName);
+            fileName = testName;
         }
+        if (suffix != null) {
+            fileName += suffix;
+        }
+        Path file = dir.resolve(fileName);
         Files.deleteIfExists(file);
         return file;
     }
 
+    private Path getTemporaryOutputFile() throws IOException {
+        return getTemporaryOutputFile(null);
+    }
+
     @SuppressWarnings("checkstyle:VisibilityModifier")
     private static class TestData {
         public final String algorithm;


[mina-sshd] 03/03: [SSHD-997] A few minor code fixes in OpenSSHKeyPairResourceWriterTest

Posted by lg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git

commit a97e853e2f3f8ab8ac9952701ceb2e6005a29ff6
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Fri May 22 07:59:03 2020 +0300

    [SSHD-997] A few minor code fixes in OpenSSHKeyPairResourceWriterTest
---
 .../openssh/OpenSSHKeyPairResourceWriterTest.java  | 40 ++++++++++------------
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/sshd-common/src/test/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriterTest.java b/sshd-common/src/test/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriterTest.java
index f3c03e2..83f345c 100644
--- a/sshd-common/src/test/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriterTest.java
+++ b/sshd-common/src/test/java/org/apache/sshd/common/config/keys/writer/openssh/OpenSSHKeyPairResourceWriterTest.java
@@ -19,6 +19,7 @@
 
 package org.apache.sshd.common.config.keys.writer.openssh;
 
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -179,15 +180,17 @@ public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
     @Test
     public void testFileRoundtripWithEncryption() throws Exception {
         Path tmp = getTemporaryOutputFile();
-        try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
-            OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
-            options.setPassword("nonsense");
-            options.setCipherName("AES");
-            options.setCipherMode("CTR");
-            options.setCipherType("256");
+        OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
+        options.setPassword("nonsense");
+        options.setCipherName("AES");
+        options.setCipherMode("CTR");
+        options.setCipherType("256");
+
+        try (ByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
             OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(testKey, "a comment", options, out);
             writeToFile(tmp, out.toByteArray());
         }
+
         try (InputStream in = Files.newInputStream(tmp)) {
             KeyPair key = SecurityUtils.loadKeyPairIdentities(null,
                     new PathResource(tmp), in, FilePasswordProvider.of("nonsense")).iterator().next();
@@ -195,12 +198,7 @@ public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
             assertKeyPairEquals("Mismatched recovered keys", testKey, key);
             assertTrue("Keys should be equal", compare(key, testKey));
             Path tmp2 = getTemporaryOutputFile("again");
-            try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
-                OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
-                options.setPassword("nonsense");
-                options.setCipherName("AES");
-                options.setCipherMode("CTR");
-                options.setCipherType("256");
+            try (ByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
                 OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(key, "a comment", options, out);
                 writeToFile(tmp2, out.toByteArray());
             }
@@ -221,7 +219,7 @@ public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
     public void testFileRoundtripAsymmetric() throws Exception {
         // Write first unencrypted, then encrypted. read both and compare.
         Path tmp = getTemporaryOutputFile();
-        try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
+        try (ByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
             OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(testKey, "a comment", null, out);
             writeToFile(tmp, out.toByteArray());
         }
@@ -232,7 +230,7 @@ public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
             assertKeyPairEquals("Mismatched recovered keys", testKey, key);
             assertTrue("Keys should be equal", compare(key, testKey));
             Path tmp2 = getTemporaryOutputFile("again");
-            try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
+            try (ByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
                 OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
                 options.setPassword("nonsense");
                 options.setCipherName("AES");
@@ -257,7 +255,7 @@ public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
     @Test
     public void testWritePrivateKeyNoEncryption() throws Exception {
         Path tmp = getTemporaryOutputFile();
-        try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
+        try (ByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
             OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(testKey, "a comment", null, out);
             writeToFile(tmp, out.toByteArray());
         }
@@ -273,7 +271,7 @@ public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
     @Test
     public void testWritePrivateKeyNoPassword() throws Exception {
         Path tmp = getTemporaryOutputFile();
-        try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
+        try (ByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
             OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
             OpenSSHKeyPairResourceWriter.INSTANCE.writePrivateKey(testKey, "a comment", options, out);
             writeToFile(tmp, out.toByteArray());
@@ -290,7 +288,7 @@ public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
     @Test
     public void testWritePrivateKeyEncryptedAesCbc128() throws Exception {
         Path tmp = getTemporaryOutputFile();
-        try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
+        try (ByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
             OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
             options.setPassword("nonsense");
             options.setCipherName("AES");
@@ -313,7 +311,7 @@ public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
     @Test
     public void testWritePrivateKeyEncryptedAesCtr256() throws Exception {
         Path tmp = getTemporaryOutputFile();
-        try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
+        try (ByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
             OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
             options.setPassword("nonsense");
             options.setCipherName("AES");
@@ -336,7 +334,7 @@ public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
     @Test
     public void testWritePrivateKeyEncryptedWrongPassword() throws Exception {
         Path tmp = getTemporaryOutputFile();
-        try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
+        try (ByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
             OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
             options.setPassword("nonsense");
             options.setCipherName("AES");
@@ -358,7 +356,7 @@ public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
     @Test
     public void testWritePrivateKeyEncryptedNoPassword() throws Exception {
         Path tmp = getTemporaryOutputFile();
-        try (SecureByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
+        try (ByteArrayOutputStream out = new SecureByteArrayOutputStream()) {
             OpenSSHKeyEncryptionContext options = new OpenSSHKeyEncryptionContext();
             options.setPassword("nonsense");
             options.setCipherName("AES");
@@ -380,7 +378,7 @@ public class OpenSSHKeyPairResourceWriterTest extends JUnitTestSupport {
         AuthorizedKeyEntry entry = keysRead.get(0);
         String readComment = entry.getComment();
         if (comment == null || comment.isEmpty()) {
-            assertTrue("Unexpected comment", readComment == null || readComment.isEmpty());
+            assertTrue("Unexpected comment: " + readComment, readComment == null || readComment.isEmpty());
         } else {
             assertEquals("Unexpected comment", comment, readComment);
         }