fix(CertificateRotation): Encode chain with explicit tagging (#222)

Per the spec. Found bug whilst testing compatibility with JS implementation.
relaycorp · Feb 22, 2022 · c9bf51a · c9bf51a
1 parent d434aae
commit c9bf51a
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 7 deletions.
diff --git a/src/main/kotlin/tech/relaycorp/relaynet/messages/CertificateRotation.kt b/src/main/kotlin/tech/relaycorp/relaynet/messages/CertificateRotation.kt
@@ -1,6 +1,5 @@
 package tech.relaycorp.relaynet.messages
 
-import org.bouncycastle.asn1.ASN1TaggedObject
 import org.bouncycastle.asn1.DEROctetString
 import org.bouncycastle.asn1.DERSequence
 import tech.relaycorp.relaynet.wrappers.asn1.ASN1Exception
@@ -11,8 +10,7 @@ import tech.relaycorp.relaynet.wrappers.x509.CertificateException
 class CertificateRotation(val subjectCertificate: Certificate, val chain: List<Certificate>) {
     fun serialize(): ByteArray {
         val chainSequence = ASN1Utils.makeSequence(
-            chain.map { DEROctetString(it.serialize()) },
-            false
+            chain.map { DEROctetString(it.serialize()) }
         )
         val sequence = ASN1Utils.serializeSequence(
             listOf(
@@ -72,10 +70,12 @@ class CertificateRotation(val subjectCertificate: Certificate, val chain: List<C
                 throw InvalidMessageException("Chain is malformed", exc)
             }
             val chain = try {
-                chainSequence.map { ASN1Utils.getOctetString(it as ASN1TaggedObject) }
+                chainSequence.map { DEROctetString.getInstance(it) }
                     .map { Certificate.deserialize(it.octets) }
             } catch (exc: CertificateException) {
                 throw InvalidMessageException("Chain contains malformed certificate", exc)
+            } catch (exc: IllegalArgumentException) {
+                throw InvalidMessageException("Chain contains malformed certificate", exc)
             }
 
             return CertificateRotation(subjectCertificate, chain)

diff --git a/src/test/kotlin/tech/relaycorp/relaynet/messages/CertificateRotationTest.kt b/src/test/kotlin/tech/relaycorp/relaynet/messages/CertificateRotationTest.kt
@@ -3,7 +3,6 @@ package tech.relaycorp.relaynet.messages
 import kotlin.test.assertEquals
 import kotlin.test.assertTrue
 import org.bouncycastle.asn1.ASN1Sequence
-import org.bouncycastle.asn1.ASN1TaggedObject
 import org.bouncycastle.asn1.DEROctetString
 import org.bouncycastle.asn1.DERVisibleString
 import org.junit.jupiter.api.Nested
@@ -71,7 +70,7 @@ class CertificateRotationTest {
             val chainSequence = ASN1Sequence.getInstance(sequenceItems[1], false)
             assertEquals(1, chainSequence.size())
             val issuerSerialized =
-                ASN1Utils.getOctetString(chainSequence.first() as ASN1TaggedObject).octets
+                DEROctetString.getInstance(chainSequence.first()).octets
             assertEquals(issuerCertificate, Certificate.deserialize(issuerSerialized))
         }
     }
@@ -161,7 +160,7 @@ class CertificateRotationTest {
                 listOf(
                     DEROctetString(subjectCertificate.serialize()),
                     ASN1Utils.makeSequence(
-                        listOf(DERVisibleString("malformed")), false
+                        listOf(DEROctetString("malformed".toByteArray()))
                     )
                 ),
                 false
@@ -175,6 +174,26 @@ class CertificateRotationTest {
             assertTrue(exception.cause is CertificateException)
         }
 
+        @Test
+        fun `Chain certificates should be OCTET STRINGs`() {
+            val serialization = CertificateRotation.FORMAT_SIGNATURE + ASN1Utils.serializeSequence(
+                listOf(
+                    DEROctetString(subjectCertificate.serialize()),
+                    ASN1Utils.makeSequence(
+                        listOf(DERVisibleString("malformed"))
+                    )
+                ),
+                false
+            )
+
+            val exception = assertThrows<InvalidMessageException> {
+                CertificateRotation.deserialize(serialization)
+            }
+
+            assertEquals("Chain contains malformed certificate", exception.message)
+            assertTrue(exception.cause is IllegalArgumentException)
+        }
+
         @Test
         fun `A new instance should be returned if serialization is valid`() {
             val rotation = CertificateRotation(subjectCertificate, listOf(issuerCertificate))