Bug 171103

Created attachment 307705 [details] Patch

Comment on attachment 307705 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307705&action=review Doesn't this fix any test currently skipped? > Source/WebCore/crypto/gcrypt/CryptoAlgorithmECDSAGCrypt.cpp:79 > +std::optional<Vector<uint8_t>> mpiData(gcry_sexp_t paramSexp) should this be static?

Created attachment 307935 [details] Patch for landing Also marks the majority of ECDSA layout tests under crypto/subtle/ as passing.

Planning to land this? Jiewen, do you mind sanity-checking it please? Thanks for looking over these patches. :)

Comment on attachment 307935 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=307935&action=review Looks good to me. > Source/WebCore/crypto/gcrypt/CryptoAlgorithmECDSAGCrypt.cpp:151 > + Vector<uint8_t> signature; Not sure if you could always assume that r and s equal to key size as this is in fact not the case for CommonCrypto. From RFC 6090:https://www.ietf.org/rfc/rfc6090.txt, I didn't see this guarantee as well.

Comment on attachment 307935 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=307935&action=review >> Source/WebCore/crypto/gcrypt/CryptoAlgorithmECDSAGCrypt.cpp:151 >> + Vector<uint8_t> signature; > > Not sure if you could always assume that r and s equal to key size as this is in fact not the case for CommonCrypto. > From RFC 6090:https://www.ietf.org/rfc/rfc6090.txt, I didn't see this guarantee as well. We check that the retrieved data is of proper size below, before appending the r and s data to the signature Vector.

Comment on attachment 307935 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=307935&action=review >>> Source/WebCore/crypto/gcrypt/CryptoAlgorithmECDSAGCrypt.cpp:151 >>> + Vector<uint8_t> signature; >> >> Not sure if you could always assume that r and s equal to key size as this is in fact not the case for CommonCrypto. >> From RFC 6090:https://www.ietf.org/rfc/rfc6090.txt, I didn't see this guarantee as well. > > We check that the retrieved data is of proper size below, before appending the r and s data to the signature Vector. Sure. What I mean is a valid r and s pair does not necessarily meet the check below.

Comment on attachment 307935 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=307935&action=review >>>> Source/WebCore/crypto/gcrypt/CryptoAlgorithmECDSAGCrypt.cpp:151 >>>> + Vector<uint8_t> signature; >>> >>> Not sure if you could always assume that r and s equal to key size as this is in fact not the case for CommonCrypto. >>> From RFC 6090:https://www.ietf.org/rfc/rfc6090.txt, I didn't see this guarantee as well. >> >> We check that the retrieved data is of proper size below, before appending the r and s data to the signature Vector. > > Sure. What I mean is a valid r and s pair does not necessarily meet the check below. Also see: https://www.w3.org/TR/WebCryptoAPI/#dfn-convert-integer-to-octet-string

(In reply to Jiewen Tan from comment #8) > Comment on attachment 307935 [details] > Patch for landing > > View in context: > https://bugs.webkit.org/attachment.cgi?id=307935&action=review > > >>>> Source/WebCore/crypto/gcrypt/CryptoAlgorithmECDSAGCrypt.cpp:151 > >>>> + Vector<uint8_t> signature; > >>> > >>> Not sure if you could always assume that r and s equal to key size as this is in fact not the case for CommonCrypto. > >>> From RFC 6090:https://www.ietf.org/rfc/rfc6090.txt, I didn't see this guarantee as well. > >> > >> We check that the retrieved data is of proper size below, before appending the r and s data to the signature Vector. > > > > Sure. What I mean is a valid r and s pair does not necessarily meet the check below. > > Also see: > https://www.w3.org/TR/WebCryptoAPI/#dfn-convert-integer-to-octet-string Ah, understood. I'll add a FIXME that points to a new bug.

Created attachment 308802 [details] Patch for landing

Committed r216062: <http://trac.webkit.org/changeset/216062>