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

Add unknown variant to KeyAlgorithm #400

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ThatNerdUKnow
Copy link

@ThatNerdUKnow ThatNerdUKnow commented Aug 20, 2024

Adding an unknown variant to KeyAlgorithm to fix deserialization for future key algorithm variants.

In my use case I have an Identity provider that is giving us a key in the JwkSet of type ES512 which is currently not supported by the library. If even a single JWK in the JWKSet uses an algorithm that is not in the KeyAlgorithm enum it breaks deserialization of the entire JWKSet. I figure that by providing a catch-all variant in the KeyAlgorithm enum it should be flexible enough to allow this library to properly deserialize JWKSets even if some of the variants use algorithms that the library doesn't yet support.

src/jwk.rs Outdated Show resolved Hide resolved
src/jwk.rs Show resolved Hide resolved
Brandon Piña and others added 2 commits August 21, 2024 08:18
Co-authored-by: Vincent Prouillet <balthek@gmail.com>
Copy link
Owner

@Keats Keats left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Keats
Copy link
Owner

Keats commented Aug 26, 2024

Looks like maybe proptest doesn't work on wasm?

@ThatNerdUKnow
Copy link
Author

ThatNerdUKnow commented Aug 26, 2024

Looks like maybe proptest doesn't work on wasm?

[dev-dependencies.proptest] 
version = "$proptestVersion" 
# The default feature set includes things like process forking which are not 
# supported in Web Assembly. 
default-features = false 
# Enable using the `std` crate. 
features = ["std"]

It looks like by default protest uses some stuff that isn't available on wasm. Turning these features off might fix testing in wasm. How do I test for the wasm target so I can verify that this works?

@Keats
Copy link
Owner

Keats commented Aug 26, 2024

I think running those steps locally should do: https://github.com/Keats/jsonwebtoken/blob/master/.github/workflows/ci.yml#L75-L79

@ThatNerdUKnow
Copy link
Author

I think running those steps locally should do: https://github.com/Keats/jsonwebtoken/blob/master/.github/workflows/ci.yml#L75-L79

I'll give it a try in the morning

@ThatNerdUKnow
Copy link
Author

ThatNerdUKnow commented Aug 26, 2024

Applied the changes - the wasm build doesn't seem to work for me locally but I'm getting a different error than the CI:

The following warnings were emitted during compilation:

warning: ring@0.17.8: error: unable to create target: 'No available targets are compatible with triple "wasm32-unknown-unknown"'
warning: ring@0.17.8: 1 error generated.

error: failed to run custom build command for `ring v0.17.8`

Caused by:
  process didn't exit successfully: `/Users/brandon/repos/jsonwebtoken/target/debug/build/ring-b97b751a8a1f7caa/build-script-build` (exit status: 1)
  --- stdout
  cargo:rerun-if-env-changed=RING_PREGENERATE_ASM
  cargo:rustc-env=RING_CORE_PREFIX=ring_core_0_17_8_
  OPT_LEVEL = Some(0)
  TARGET = Some(wasm32-unknown-unknown)
  OUT_DIR = Some(/Users/brandon/repos/jsonwebtoken/target/wasm32-unknown-unknown/debug/build/ring-8e0c4347f70f1976/out)
  HOST = Some(aarch64-apple-darwin)
  cargo:rerun-if-env-changed=CC_wasm32-unknown-unknown
  CC_wasm32-unknown-unknown = None
  cargo:rerun-if-env-changed=CC_wasm32_unknown_unknown
  CC_wasm32_unknown_unknown = None
  cargo:rerun-if-env-changed=TARGET_CC
  TARGET_CC = None
  cargo:rerun-if-env-changed=CC
  CC = None
  cargo:rerun-if-env-changed=CC_ENABLE_DEBUG_OUTPUT
  RUSTC_WRAPPER = None
  cargo:rerun-if-env-changed=CRATE_CC_NO_DEFAULTS
  CRATE_CC_NO_DEFAULTS = None
  cargo:rerun-if-env-changed=WASI_SYSROOT
  WASI_SYSROOT = None
  DEBUG = Some(true)
  cargo:rerun-if-env-changed=CFLAGS_wasm32-unknown-unknown
  CFLAGS_wasm32-unknown-unknown = None
  cargo:rerun-if-env-changed=CFLAGS_wasm32_unknown_unknown
  CFLAGS_wasm32_unknown_unknown = None
  cargo:rerun-if-env-changed=TARGET_CFLAGS
  TARGET_CFLAGS = None
  cargo:rerun-if-env-changed=CFLAGS
  CFLAGS = None
  cargo:warning=error: unable to create target: 'No available targets are compatible with triple "wasm32-unknown-unknown"'
  cargo:warning=1 error generated.

  --- stderr


  error occurred: Command "clang" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-fno-exceptions" "-g" "-fno-omit-frame-pointer" "--target=wasm32-unknown-unknown" "-I" "include" "-I" "/Users/brandon/repos/jsonwebtoken/target/wasm32-unknown-unknown/debug/build/ring-8e0c4347f70f1976/out" "-Wall" "-Wextra" "-fvisibility=hidden" "-std=c1x" "-Wall" "-Wbad-function-cast" "-Wcast-align" "-Wcast-qual" "-Wconversion" "-Wmissing-field-initializers" "-Wmissing-include-dirs" "-Wnested-externs" "-Wredundant-decls" "-Wshadow" "-Wsign-compare" "-Wsign-conversion" "-Wstrict-prototypes" "-Wundef" "-Wuninitialized" "-g3" "-nostdlibinc" "-DNDEBUG" "-DRING_CORE_NOSTDLIBINC=1" "-o" "/Users/brandon/repos/jsonwebtoken/target/wasm32-unknown-unknown/debug/build/ring-8e0c4347f70f1976/out/fad98b632b8ce3cc-curve25519.o" "-c" "crypto/curve25519/curve25519.c" with args clang did not execute successfully (status code exit status: 1).


warning: build failed, waiting for other jobs to finish...
Error: Compilation of your program failed
Caused by: Compilation of your program failed
Caused by: failed to execute `cargo build`: exited with exit status: 101
  full command: cd "/Users/brandon/repos/jsonwebtoken" && "cargo" "build" "--tests" "--target" "wasm32-unknown-unknown"

Given that this is happening in a build step and that this library already depends on ring (albeit a slightly earlier revision: 0.17.4) I'd imagine that this likely has more to do with my environment setup rather than the repo itself.

EDIT: reading that error message again and given that the compiler being invoked here is CLANG - I wonder if the problem is that my CLANG out of the box isn't able to target wasm32-unknown-unknown

EDIT: the problem is I'm on macOS and the default CLANG does not support wasm32-unknown-unknown
briansmith/ring#1824

@Keats
Copy link
Owner

Keats commented Aug 27, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants