Allow optional data after attestation data

Optional extension info may follow attestation data. In such case the
credential public key CBOR detects garbage at the end. This commit makes
the parser interpret only public key part and leave the rest for further
processing.

Change-Id: Ie1b227c5d1a2e1d30317df1baf62e77fabee76d3

diff --git a/srcs/cbor_parsing.cpp b/srcs/cbor_parsing.cpp
index 6db2f2ae6866276001213df5f1ed02fd0467107e..02b36a7dedfd10ab0fdaee7f9042fc1c682a6f7b 100644 (file)
--- a/srcs/cbor_parsing.cpp
+++ b/srcs/cbor_parsing.cpp
@@ -19,12 +19,6 @@
 
 #include <utility>
 
-namespace {
-constexpr auto CBOR_FLAGS = CborValidateShortestIntegrals | CborValidateNoIndeterminateLength |
-    CborValidateMapIsSorted | CborValidateNoTags | CborValidateMapKeysAreUnique |
-    CborValidateCompleteData;
-} // namespace
-
 namespace CborParsing {
 
 const Key CURRENT_KEY;
@@ -371,12 +365,13 @@ KeyCopy SortedMap::PeekKey()
     THROW_UNKNOWN("Unexpected key type");
 }
 
-Parser::Parser(std::unique_ptr<CborParser> &&parser, CborValue &&root)
-: Container(std::move(root)), m_parser(std::move(parser))
+Parser::Parser(std::unique_ptr<CborParser> &&parser, CborValue &&root, const uint8_t *start)
+: Container(std::move(root)), m_parser(std::move(parser)), m_start(start)
 {
+    assert(m_start);
 }
 
-Parser Parser::Create(const uint8_t *input, size_t size)
+Parser Parser::Create(const uint8_t *input, size_t size, uint32_t flags)
 {
     if (!input && size != 0)
         THROW_UNKNOWN("input is NULL");
@@ -389,11 +384,11 @@ Parser Parser::Create(const uint8_t *input, size_t size)
     if (err != CborNoError)
         THROW_UNKNOWN("cbor_parser_init failed with " << static_cast<int>(err));
 
-    err = cbor_value_validate(&root, CBOR_FLAGS);
+    err = cbor_value_validate(&root, flags);
     if (err != CborNoError)
         THROW_UNKNOWN("CBOR validation failed with " << static_cast<int>(err));
 
-    return {std::move(parser), std::move(root)};
+    return {std::move(parser), std::move(root), input};
 }
 
 } // namespace CborParsing

diff --git a/srcs/cbor_parsing.h b/srcs/cbor_parsing.h
index 210ae65b88f91bde88d029347901d334c0f81f31..88e0f5373b9d44872a14728171fb2e6dfd082fc0 100644 (file)
--- a/srcs/cbor_parsing.h
+++ b/srcs/cbor_parsing.h
@@ -96,12 +96,22 @@ public:
 
 class Parser : public Container {
 public:
-    static Parser Create(const uint8_t *input, size_t size);
+    static constexpr uint32_t DEFAULT_FLAGS = CborValidateShortestIntegrals |
+        CborValidateNoIndeterminateLength | CborValidateMapIsSorted | CborValidateNoTags |
+        CborValidateMapKeysAreUnique | CborValidateCompleteData;
+
+    static Parser Create(const uint8_t *input, size_t size, uint32_t flags = DEFAULT_FLAGS);
+
+    [[nodiscard]] size_t BytesRead() const noexcept
+    {
+        return cbor_value_get_next_byte(&m_it) - m_start;
+    }
 
 private:
-    Parser(std::unique_ptr<CborParser> &&parser, CborValue &&root);
+    Parser(std::unique_ptr<CborParser> &&parser, CborValue &&root, const uint8_t *start);
 
     std::unique_ptr<CborParser> m_parser;
+    const uint8_t *m_start;
 };
 
 } // namespace CborParsing

diff --git a/srcs/message.cpp b/srcs/message.cpp
index 05ad9e8285e584d11bc88dde5486b38bddb3e3ee..59ea3ccd57d400a2d8eb17474e308b13d22ee336 100644 (file)
--- a/srcs/message.cpp
+++ b/srcs/message.cpp
@@ -618,8 +618,15 @@ CtapResponse::AuthData::AttestationData::AttestationData(BufferView &attestation
     m_credentialId = BufferView(attestationDataView.data(), credentialIdLen);
     attestationDataView.remove_prefix(credentialIdLen);
 
-    auto publicKeyParser =
-        CborParsing::Parser::Create(attestationDataView.data(), attestationDataView.size());
+    /*
+     * Allow redundant CBOR data as optional extensions may follow attestation data.
+     * See: https://www.w3.org/TR/webauthn-3/#sctn-authenticator-data.
+     */
+    using CborParsing::Parser;
+    auto publicKeyParser = Parser::Create(attestationDataView.data(),
+                                          attestationDataView.size(),
+                                          Parser::DEFAULT_FLAGS & ~CborValidateCompleteData);
+
     auto credentialPublicKeyMap = publicKeyParser.EnterMap();
     // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
     auto kty = credentialPublicKeyMap.GetInt64At(KEY_KTY).value(); // Key Type
@@ -663,6 +670,7 @@ CtapResponse::AuthData::AttestationData::AttestationData(BufferView &attestation
         // TODO other formats?
         THROW_UNKNOWN("Invalid 'kty' value " << kty);
     }
+    attestationDataView.remove_prefix(publicKeyParser.BytesRead());
 }
 
 void CtapResponse::AuthData::Deserialize(const Buffer &authDataRaw)
@@ -688,6 +696,10 @@ void CtapResponse::AuthData::Deserialize(const Buffer &authDataRaw)
 
     if (!authDataView.empty())
         m_attestationData = AttestationData(authDataView);
+
+    if (!authDataView.empty()) {
+        // TODO extensions
+    }
 }
 
 void CtapResponse::DeserializeAuthData(Buffer authData)

diff --git a/srcs/message.h b/srcs/message.h
index cd016a13b9a67661e290e6d7da29e39f5a28efec..f3c0f1b1483d043b01dcb38ec68d25ee7fe82f8b 100644 (file)
--- a/srcs/message.h
+++ b/srcs/message.h
@@ -20,6 +20,7 @@
 
 #include <cstdint>
 #include <optional>
+#include <string>
 #include <unordered_map>
 #include <webauthn-types.h>
 

diff --git a/tests/cbor_tests.cpp b/tests/cbor_tests.cpp
index efbe940c99c6ae53a427a59c33d9da4d83f8ec8d..6568c4f1253ee50783cfee245c74f4528a865124 100644 (file)
--- a/tests/cbor_tests.cpp
+++ b/tests/cbor_tests.cpp
@@ -639,6 +639,31 @@ TEST(Cbor, DecodingGarbageAtEnd_Negative)
     EXPECT_THROW(CborParsing::Parser::Create(buffer.data(), buffer.size()), Unknown);
 }
 
+TEST(Cbor, DecodingGarbageAtEnd_Positive)
+{
+    Buffer buffer(4);
+    static constexpr int64_t VALUE = 24;
+    auto encoder = CborEncoding::Encoder::Create(buffer.data(), buffer.size());
+    {
+        auto map = encoder.OpenMap(1);
+        EXPECT_NO_THROW(map.AppendInt64At(0x00, VALUE));
+    }
+
+    EXPECT_EQ(encoder.GetBufferSize(), buffer.size());
+    buffer.emplace_back(0x00);
+
+    auto parser =
+        CborParsing::Parser::Create(buffer.data(),
+                                    buffer.size(),
+                                    CborParsing::Parser::DEFAULT_FLAGS & ~CborValidateCompleteData);
+
+    {
+        auto parserMap = parser.EnterMap();
+        EXPECT_EQ(parserMap.GetInt64At(0), VALUE);
+    }
+    EXPECT_EQ(parser.BytesRead(), buffer.size() - 1);
+}
+
 TEST(Cbor, DecodingMapUnknownIntKey_Positive)
 {
     Buffer buffer(10);
@@ -936,8 +961,23 @@ TEST(Cbor, SkippingUndecodedEntriesUponContainerClose_Positive)
         static constexpr auto data = std::string_view{expectedValue};                             \
         auto parser = CborParsing::Parser::Create(reinterpret_cast<const uint8_t *>(data.data()), \
                                                   data.size());                                   \
-        containerType;                                                                            \
-        assertStatement;                                                                          \
+        {                                                                                         \
+            containerType;                                                                        \
+            assertStatement;                                                                      \
+        }                                                                                         \
+        EXPECT_EQ(data.size(), parser.BytesRead());                                               \
+    }
+
+#define TEST_CBOR_PARSING_ENTER_THROWS(name, expectedValue, enterContainerExpression)             \
+    TEST(CborParsing, name)                                                                       \
+    {                                                                                             \
+        static constexpr auto data = std::string_view{expectedValue};                             \
+        auto parser = CborParsing::Parser::Create(reinterpret_cast<const uint8_t *>(data.data()), \
+                                                  data.size());                                   \
+        {                                                                                         \
+            EXPECT_THROW(enterContainerExpression, Unknown);                                      \
+        }                                                                                         \
+        EXPECT_EQ(0, parser.BytesRead());                                                         \
     }
 
 #define TEST_CBOR_PARSING_PREMATURE_END(name, expectedValue, parserStatement) \
@@ -1487,22 +1527,14 @@ TEST_CBOR_PARSING(ContainerEmptyMapInEnterMap_Positive,
                   "\xA0",
                   auto container = parser.EnterMap(),
                   EXPECT_EQ(container.GetUint64At(0), std::nullopt))
-TEST_CBOR_PARSING(ContainerNullValueInEnterMap_Negative,
-                  "\xF6",
-                  EXPECT_THROW(parser.EnterMap(), Unknown),
-                  {})
-TEST_CBOR_PARSING(ContainerUndefinedValueInEnterMap_Negative,
-                  "\xF7",
-                  EXPECT_THROW(parser.EnterMap(), Unknown),
-                  {})
-TEST_CBOR_PARSING(ContainerDifferentTypeValueInEnterMap_Negative,
-                  "\x40",
-                  EXPECT_THROW(parser.EnterMap(), Unknown),
-                  {})
-TEST_CBOR_PARSING(ContainerArrayInEnterMap_Negative,
-                  "\x81\x01",
-                  EXPECT_THROW(parser.EnterMap(), Unknown),
-                  {})
+TEST_CBOR_PARSING_ENTER_THROWS(ContainerNullValueInEnterMap_Negative, "\xF6", parser.EnterMap())
+TEST_CBOR_PARSING_ENTER_THROWS(ContainerUndefinedValueInEnterMap_Negative,
+                               "\xF7",
+                               parser.EnterMap())
+TEST_CBOR_PARSING_ENTER_THROWS(ContainerDifferentTypeValueInEnterMap_Negative,
+                               "\x40",
+                               parser.EnterMap())
+TEST_CBOR_PARSING_ENTER_THROWS(ContainerArrayInEnterMap_Negative, "\x81\x01", parser.EnterMap())
 
 // EnterArray tests
 TEST_CBOR_PARSING(ContainerCorrectValueInEnterArray_Positive,
@@ -1513,22 +1545,16 @@ TEST_CBOR_PARSING(ContainerEmptyArrayInEnterArray_Negative,
                   "\x80",
                   auto container = parser.EnterArray(),
                   EXPECT_THROW(container.GetUint64(), Unknown))
-TEST_CBOR_PARSING(ContainerNullValueInEnterArray_Negative,
-                  "\xF6",
-                  EXPECT_THROW(parser.EnterArray(), Unknown),
-                  {})
-TEST_CBOR_PARSING(ContainerUndefinedValueInEnterArray_Negative,
-                  "\xF7",
-                  EXPECT_THROW(parser.EnterArray(), Unknown),
-                  {})
-TEST_CBOR_PARSING(ContainerDifferentTypeValueInEnterArray_Negative,
-                  "\x40",
-                  EXPECT_THROW(parser.EnterArray(), Unknown),
-                  {})
-TEST_CBOR_PARSING(ContainerMapInEnterArray_Negative,
-                  "\xA1\x01\x01",
-                  EXPECT_THROW(parser.EnterArray(), Unknown),
-                  {})
+TEST_CBOR_PARSING_ENTER_THROWS(ContainerNullValueInEnterArray_Negative, "\xF6", parser.EnterArray())
+TEST_CBOR_PARSING_ENTER_THROWS(ContainerUndefinedValueInEnterArray_Negative,
+                               "\xF7",
+                               parser.EnterArray())
+TEST_CBOR_PARSING_ENTER_THROWS(ContainerDifferentTypeValueInEnterArray_Negative,
+                               "\x40",
+                               parser.EnterArray())
+TEST_CBOR_PARSING_ENTER_THROWS(ContainerMapInEnterArray_Negative,
+                               "\xA1\x01\x01",
+                               parser.EnterArray())
 
 // EnterArrayAt tests
 TEST_CBOR_PARSING(SortedMapNullKeyInEnterArrayAt_Negative,