Bug 233011 – [WebAuthn] Stop serializing BufferSource and Vector<uint8_t> duplicates of identifiers

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Brent Fulgham]

The original WebAuthn logic converted WebCore::BufferSource objects to Vector<uint8_t> for serialization, which created a weird design where some code dealt with BufferSource objects, and other with Vectors, and lots of converting to and from these types. It also caused WebAuthn data structures to have two places where this information might live, with the UIProcess using one representation, and the WebContent process using another.

This patch revises the code as follows:

1. The identifiers are always stored as BufferSource, and the same member is used in UIProcess and WebContent process when accessing this information.
2. We now serialize BufferSource directly.

[Radar WebKit Bug Importer]

<rdar://problem/85313807>

[Brent Fulgham]

Created attachment 444005 [details]
Patch

[Chris Dumez]

Comment on attachment 444005 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=444005&action=review

r=me, very nice!

> Source/WebCore/Modules/webauthn/fido/U2fCommandConstructor.cpp:88
> +    return constructU2fSignCommand(applicationParameter, challengeParameter, keyVector, checkOnly);

Doesn't need to be done in this patch but constructU2fSignCommand() should probably take in a WTF::Span<const uint8_t> instead of a Vector<uint8_t>.

> Source/WebCore/bindings/js/BufferSource.h:99
> +    auto dataSize = CheckedSize { *size } * sizeof(uint8_t);

nit: The `* sizeof(uint8_t)` doesn't look terribly useful :)

> Source/WebCore/bindings/js/BufferSource.h:106
> +    return BufferSource(JSC::ArrayBuffer::tryCreate(reinterpret_cast<const void*>(data), dataSize.value()));

nit: Would a static_cast<const void*> work?

> Source/WebCore/bindings/js/BufferSource.h:117
> +    return BufferSource(JSC::ArrayBuffer::tryCreate(reinterpret_cast<const uint8_t*>(data.bytes), data.length));

static_cast<> should suffice since bytes returns a const void*

> Source/WebCore/bindings/js/BufferSource.h:128
> +using WebCore::toBufferSource;

I am not opposed to it but I didn't realize we were doing this outside of WTF.

[Brent Fulgham]

Created attachment 444024 [details]
Patch for landing

[EWS]

Committed r285698 (244165@main): <https://commits.webkit.org/244165@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 444024 [details].