Merge jwe decrypt

[effedib]

merge JWE decryption and little refactor to optimize the shared code.

To add in the next version:

• - error handling with thiserror
• - more alg to decrypt jwe
• - more key format
• - tests

Closes #46

[lmammino]

Thank you so much for taking the time to submit these most welcome improvements!

Great work so far, but I think we need to iron out some details related to consistency and some other implementation details.

Let me know if it's all clear.

[lmammino]

I think here we are swallowing a possible error. Not sure this is the best option...

We should probably print the error to stderr and exist with an error status code

[lmammino]

Duplicate module tree between the binary and the library crate

Both src/cli.rs (the binary) and src/main.rs (the library) declare the same module tree:

  pub mod jw_error;
  pub mod jw_parser;
  pub mod jwe;
  pub mod jwt;

Rust compiles these as separate crates, this means every module (jw_error, jw_parser, jwe, jwt) is compiled twice into two completely independent copies. The binary no longer imports anything from the library crate... it was previously doing use jwtinfo::jwt which is a better approach here...

Suggested change

pub mod jwt;

Suggestion: remove the mod declarations here

[lmammino]

minor: I think we can get rid of TryInto since it should be auto-imported in recent editions of Rust. Worth double checking

[lmammino]

I was thinking that there was something a bit off here and I did a couple of AI reviews with a few different models. I really liked this suggestion which I think is worth considering:

Error type inconsistencies

• JweError::StringError(String) is a catch-all that loses type information. It's used as a From<String> impl, which means any String can become a JweError... this is overly broad.
• JweError::Internal(Box<dyn Error + Send + Sync>) is another opaque catch-all.
• JWTParseError no longer implements Error (only Display via thiserror), but JWTParsePartError also only derives Error through thiserror without being explicitly listed. This works but the old impl Error for JWTParsePartError {} was removed without verifying all downstream consumers. (note: this one is my fault when I quickly added ThisError 😅 )
• MissingSection() and MissingParts() use empty tuple variant syntax () when unit variants would be cleaner.

Suggested approach to address these issues

The root cause is a two-layer error design that doesn't quite fit together. The crypto internals (algorithms.rs) use CryptoResult<T> = Result<T, Box<dyn Error + Send + Sync>>, producing opaque boxed errors. Then JweError needs to accept those, so it has StringError(String) and Internal(Box<dyn Error>) as catch-all buckets, plus From<String> and From<Box<dyn Error>> blanket conversions. The result is that most error context is erased by the time it reaches the caller.

I'd suggest one of two approaches:

Option A: Typed errors all the way down (cleaner, more work)

Replace CryptoResult with a dedicated JweCryptoError enum that covers the actual failure modes:

  #[derive(Debug, Error)]
  pub enum JweCryptoError {
      #[error("CEK length mismatch: expected {expected} bytes, got {actual}")]
      CekLengthMismatch { expected: usize, actual: usize },
      #[error("IV length invalid: expected 12 bytes")]
      InvalidIvLength,
      #[error("Unsupported key length: {0}")]
      UnsupportedKeyLength(usize),
      #[error("Decryption failed: {0}")]
      DecryptionFailed(String),
      #[error("Invalid RSA key: {0}")]
      InvalidRsaKey(String),
      #[error("Unsupported algorithm: {0}")]
      UnsupportedAlgorithm(String),
  }

Then have the traits return Result<Vec<u8>, JweCryptoError>, and add a JweError::Crypto(#[from] JweCryptoError) variant. This removes StringError, Internal, and both From blanket impls entirely.

Option B: Just use Box<dyn Error> in JweError (pragmatic, less work)

If you want to keep the crypto layer using Box<dyn Error + Send + Sync> for flexibility, then simplify JweError to have a single opaque variant instead of two:

  #[derive(Debug, Error)]
  pub enum JweError {
      #[error("{0}")]
      Parse(#[from] JweParseError),
      #[error("JSON error: {0}")]
      Json(#[from] serde_json::Error),
      #[error("Invalid UTF-8: {0}")]
      InvalidUtf8(#[from] string::FromUtf8Error),
      #[error("{0}")]
      Crypto(Box<dyn Error + Send + Sync + 'static>),
  }

Then replace the From<String> and From<Box<dyn Error>> impls with a single explicit conversion:

  impl From<Box<dyn Error + Send + Sync + 'static>> for JweError {
      fn from(e: Box<dyn Error + Send + Sync + 'static>) -> Self {
          JweError::Crypto(e)
      }
  }

This removes StringError and the From<String> impl (which is the most dangerous one. Any stray String silently becoming a JweError is a footgun). The AlgorithmFactory::get_key_decryptor / get_content_decryptor methods that currently return Result<_, String> would need to return Result<_, JweError> or Result<_, Box<dyn Error + Send + Sync>> instead.

Two smaller things either approach should also fix:

• MissingSection(), MissingParts(), TooManyParts(), UnexpectedPart(): drop the empty parens and use unit variants (MissingSection, MissingParts, etc.)
• #[error("not serialized error")] on JsonError: this is a misleading hardcoded message. It should be #[error("JSON error: {0}")] like the JWT version.

Recommendation

I'd recommend Option A if this is heading toward a proper release — the failure modes in the crypto layer are well-known and finite, so a typed enum is worth it. Option B is fine if you just want to clean up the obvious issues quickly.

[lmammino]

I think the changes proposed here come with some inconsistencies that might be nice to address. Here's a list of examples that showcase how using certain combinations of tokens and flags can lead to confusing or unexpected results for the user:

Scenario 1: JWE with plaintext payload "works"...

... but the flow is probably not what we want (also considering a previous comment about swallowing a possible error):

cargo run -- --key src/jwe/examples/example_priv.pem "$(cat src/jwe/examples/example_token.txt)"

Output:

Questo e' un messaggio super segreto!

Exit code: 0

This looks correct but the control flow is:

• decrypt JWE
• get plaintext string
• jwt::parse() fails because it's not a JWT
• error is silently discarded
• raw string is printed.

It works only because of the error-swallowing Err(_) => println!("{}", token) discussed before.

Scenario 2: Display flags (--pretty, --full, --header) are silently ignored for non-JWT payloads

Already hinted at this possible problem in a previous comment, but here are a few practical examples that illustrate the discrepancies:

cargo run -- --pretty --key src/jwe/examples/example_priv.pem "$(cat src/jwe/examples/example_token.txt)"
cargo run -- --full   --key src/jwe/examples/example_priv.pem "$(cat src/jwe/examples/example_token.txt)"
cargo run -- --header --key src/jwe/examples/example_priv.pem "$(cat src/jwe/examples/example_token.txt)"

All three produce the exact same output: (Questo e' un messaggio super segreto! (exit 0)). The --pretty, --full, and --header flags are completely ignored because they only apply inside the Ok branch, but we always land in the Err branch for non-JWT payloads.

Scenario 3: Regular JWT with --key gives a cryptic error

cargo run -- --key src/jwe/examples/example_priv.pem "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJmb28iOiJiYXIifQ.dtxWM6MIcgoeMgH87tGvsNDY6cHWL6MGW4LeYvnm1JA"

Output:

Error: ParseError(MissingParts)

Exit code: 1

The user gets an opaque JWE parsing error instead of something helpful like "This is a JWT, not a JWE. The --key flag is only applicable to JWE tokens."

Perhaps this is something we can address with a more generic parser that can automatically distinguish between JWS (what we call JWT here) and JWE and give us the result in some kind of JWToken enum (that can be either JWS or JWT). This would allow us to then check if specific options apply (given the detected token type) and probably help us to come up with more generic ways to address the flags discrepancies mentioned above.

Scenario 4: Invalid input succeeds silently (the point 1 regression)

cargo run -- "not-a-token-at-all"

Output:

not-a-token-at-all

Exit code: 0 (wrong!)

Previously this printed an error to stderr and exited with code 1. Now it echoes back the garbage input as if it were valid, with a success exit code. Any script using jwtinfo in a pipeline can no longer detect failures.

[lmammino]

Just to clarify some of the previous comments, this what, at high level, I think would be a more flexible design to fix most of the issues/inconsistencies.

• Define a JWToken enum with JWS (JWT) and JWE variants
• Design a generic JWT parser that automatically parses either to a JWS or a JWT variant (or fails with a clear parser error). For this I think using a progressive parser combinator like winnow might give us a pretty good abstraction and also simplify all the parsing errors
• Then based on the resulting token type we can decide how to apply all the CLI flags, see if they are relevant or not (and fail in case they aren't) and decide how to display it

So high level the flow would be:

parse token -> check cli args (based on token type) -> format/display (based on token type)