Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug Report: Error while calling importJsonWebKey #66

Closed
leanhdaoit opened this issue Mar 8, 2023 · 7 comments
Closed

Bug Report: Error while calling importJsonWebKey #66

leanhdaoit opened this issue Mar 8, 2023 · 7 comments
Labels
invalid This doesn't seem right

Comments

@leanhdaoit
Copy link

Description

When calling the function EcdhPrivateKey.importJsonWebKey with a specific private key JWK object, the following error occurs:
FormatException: JWK property "y" should hold 32 bytes

Steps to Reproduce

  1. Use the following input
String privateKeyHex: "03ecc060e7fc29863166f3fa6a490be9236304fca0b2d0bb5402cfd9cba1d100";
Map<String, String> privateKeyJWK = {
     "kty": "EC",
     "crv": "P-256",
     "x":
         "h0rbX_vxGVgx-u-ITxnydyy7W9VNGFITcqltL7hDa5A=",
     "y":
         "iQdcRQ8aghMIKnnE1RZEzrfag4c2PHBq8kVea6tK6w==",
     "d": "A-zAYOf8KYYxZvP6akkL6SNjBPygstC7VALP2cuh0QA="
   };

  1. Call the function EcdhPrivateKey.importJsonWebKey(privateKeyJWK, EllipticCurve.p256);

Expected Behavior

The function EcdhPrivateKey.importJsonWebKey should import the private key JWK object without errors.

Actual Behavior

The function EcdhPrivateKey.importJsonWebKey throws a FormatException with the message "JWK property "y" should hold 32 bytes".

Additional Context

  • y is only 31 bytes but paramSize is 32 bytes (line 169 in impl_ffi.ec_common.dart)
  • When i delete line 176 in impl_ffi.ec_common.dart it works normally
// Delete this line
 _checkData(
        bytes.length == paramSize,
        message: 'JWK property "$prop" should hold $paramSize bytes',
);

Environment

  • webcrypto: ^0.5.3
  • flutter doctor -v
[✓] Flutter (Channel stable, 3.7.6, on macOS 13.2.1 22D68 darwin-x64, locale en)
    • Flutter version 3.7.6 on channel stable at /Users/user/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 12cb4eb7a0 (7 days ago), 2023-03-01 10:29:26 -0800
    • Engine revision ada363ee93
    • Dart version 2.19.3
    • DevTools version 2.20.1

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.2)
    • Android SDK at /Users/user/Library/Android/sdk
    • Platform android-33, build-tools 33.0.2
    • ANDROID_HOME = /Users/user/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.15+0-b2043.56-8887301)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.2)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14C18
    • CocoaPods version 1.11.3

[✓] Android Studio (version 2022.1)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.15+0-b2043.56-8887301)

[✓] VS Code (version 1.76.0)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.60.0

[✓] Connected device (1 available)
    • iPhone 14 Pro Max (mobile) • CB20F049-B979-48DD-8754-A9D4F2477ECA • ios • com.apple.CoreSimulator.SimRuntime.iOS-16-2 (simulator)

[✓] HTTP Host Availability
    • All required HTTP hosts are available
@leanhdaoit
Copy link
Author

cc @jonasfj 🙏

@jonasfj
Copy link
Member

jonasfj commented Mar 9, 2023

What happens if you do this in a various browsers?

window.crypto.subtle.importKey(
  'jwk', {
    "kty": "EC",
    "crv": "P-256",
    "x": "h0rbX_vxGVgx-u-ITxnydyy7W9VNGFITcqltL7hDa5A=",
    "y": "iQdcRQ8aghMIKnnE1RZEzrfag4c2PHBq8kVea6tK6w==",
    "d": "A-zAYOf8KYYxZvP6akkL6SNjBPygstC7VALP2cuh0QA="
  }, {
    "name": "ECDH",
     "namedCurve": "P-256"
  }, true, ['deriveKey']
).then(
  k => console.log(k)
).catch(
  err => console.log(err)
);

Fails for me on both Chrome and Firefox.


This indicates to me that the JWK might be malformed or incorrectly formatted.

@jonasfj
Copy link
Member

jonasfj commented Mar 9, 2023

BASE64URL(OCTETS) denotes the base64url encoding of OCTETS, per
Section 2 of [JWS].

From: https://datatracker.ietf.org/doc/html/rfc7517#section-1.1

Base64url Encoding
Base64 encoding using the URL- and filename-safe character set
defined in Section 5 of RFC 4648 [RFC4648], with all trailing '='
characters omitted (as permitted by Section 3.2) and without the
inclusion of any line breaks, whitespace, or other additional
characters. Note that the base64url encoding of the empty octet
sequence is the empty string. (See Appendix C for notes on
implementing base64url encoding without padding.)

From: https://www.rfc-editor.org/rfc/rfc7515.html#section-2

@jonasfj
Copy link
Member

jonasfj commented Mar 9, 2023

Is there any reason to think this is a valid JWK? What other libraries that understand JWKs will accept this JWK?

@leanhdaoit
Copy link
Author

leanhdaoit commented Mar 10, 2023

What happens if you do this in a various browsers?

window.crypto.subtle.importKey(
  'jwk', {
    "kty": "EC",
    "crv": "P-256",
    "x": "h0rbX_vxGVgx-u-ITxnydyy7W9VNGFITcqltL7hDa5A=",
    "y": "iQdcRQ8aghMIKnnE1RZEzrfag4c2PHBq8kVea6tK6w==",
    "d": "A-zAYOf8KYYxZvP6akkL6SNjBPygstC7VALP2cuh0QA="
  }, {
    "name": "ECDH",
     "namedCurve": "P-256"
  }, true, ['deriveKey']
).then(
  k => console.log(k)
).catch(
  err => console.log(err)
);

Fails for me on both Chrome and Firefox.

This indicates to me that the JWK might be malformed or incorrectly formatted.

I also fail on Chrome with error DOMException: The JWK member "x" could not be base64url decoded or contained padding. If remove = in x, y, d it's will show DOMException: The JWK's "y" member defines an octet string of length 31 bytes but should be 32.
But when i run it on node js with library packages below it works fine.

Here is my example project: https://github.com/leanhdaoit/chat-e2e-example
Logs of example:

===========Import key by old @peculiar/webcrypto=============
jwk: {"kty":"EC","crv":"P-256","x":"h0rbX_vxGVgx-u-ITxnydyy7W9VNGFITcqltL7hDa5A","y":"iQdcRQ8aghMIKnnE1RZEzrfag4c2PHBq8kVea6tK6w","d":"A-zAYOf8KYYxZvP6akkL6SNjBPygstC7VALP2cuh0QA"}
privateKey:[object CryptoKey]
===========Import key by old webcrypto=============
jwk: {"kty":"EC","crv":"P-256","x":"h0rbX_vxGVgx-u-ITxnydyy7W9VNGFITcqltL7hDa5A","y":"iQdcRQ8aghMIKnnE1RZEzrfag4c2PHBq8kVea6tK6w","d":"A-zAYOf8KYYxZvP6akkL6SNjBPygstC7VALP2cuh0QA"}
privateKey:[object Object]

maybe the old libraries don't check the format of the x and y points, but it can still do deriveKey and encrypt/decrypt (AES-GCM) 🤔🤔

@jonasfj
Copy link
Member

jonasfj commented Mar 10, 2023

I don't find it hard to believe that there are other libraries out there that'll accept JWKs that are somewhat malformed.

I'm guessing that you could simply remove the padding and possibly prepend the y member with zeros. But the fact that the y member has the wrong length, should probably be a warning flag -- maybe there is something wrong with the key.

In any case, unless we can somehow reasonably say that the input is a valid JWK and would be accepted by most other webcrypto implementations, then I suggest we close this issue.

@jonasfj jonasfj closed this as completed Mar 10, 2023
@jonasfj jonasfj added invalid This doesn't seem right and removed need info labels Mar 10, 2023
@leanhdaoit
Copy link
Author

leanhdaoit commented Mar 13, 2023

I can confirm that after padLeft zero with length 32 of y it works fine.
Code Example:

import 'package:webcrypto/webcrypto.dart';
import 'package:elliptic/elliptic.dart' as elliptic;

  Map<String, String> convertPrivateKeyHexToJWK(String privateKeyHex) {
    final privateKeyEC = convertPrivateKeyHexToEC(privateKeyHex);
    final encodedX = encodeBigInt(privateKeyEC.publicKey.X);
    final encodedXPadLeft = padLeft(encodedX, 32, 0);
    final encodedY = encodeBigInt(privateKeyEC.publicKey.Y);
    final encodedYPadLeft = padLeft(encodedY, 32, 0);
    final encodedD = encodeBigInt(privateKeyEC.D);
    final encodedDPadLeft = padLeft(encodedD, 32, 0);
    final privateKeyJWK = {
      'kty': 'EC',
      'crv': 'P-256',
      'x': base64UrlEncode(encodedXPadLeft),
      'y': base64UrlEncode(encodedYPadLeft),
      'd': base64UrlEncode(encodedDPadLeft)
    };
    return privateKeyJWK;
  }
  
  elliptic.PrivateKey convertPrivateKeyHexToEC(String privateKeyHex) =>
      elliptic.PrivateKey.fromHex(elliptic.getP256(), privateKeyHex);
      
  Uint8List encodeBigInt(BigInt number) {
    final _byteMask = BigInt.from(0xff);
    final size = (number.bitLength + 7) >> 3;
    final result = new Uint8List(size);
    for (int i = 0; i < size; i++) {
      result[size - i - 1] = (number & _byteMask).toInt();
      number = number >> 8;
    }
    return result;
  }
  
  Uint8List padLeft(
      Uint8List input, int desiredLength, int paddingValue) {
    if (input.length >= desiredLength) {
      return input;
    } else {
      var padding = Uint8List(desiredLength - input.length);
      padding.fillRange(0, padding.length, paddingValue);
      return Uint8List.fromList([...padding, ...input]);
    }
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants