Skip to content

redbean: Add LuaCrypto compatible functions (plus some auxiliary functions) #1422

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

Draft
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

mterron
Copy link
Contributor

@mterron mterron commented Jun 1, 2025

Implements part of Issue #1136

Copy link
Collaborator

@pkulchenko pkulchenko left a comment

Choose a reason for hiding this comment

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

Thank you for submitting this; looks great! Couple of suggestions on the API:

  • I'd consider the following renaming for consistency: csrGenerate to generateCsr (or createCsr), generatekeypair to generateKeyPair and PemToJwk to convertPemToJwk.
  • I wonder if generatekeypair can make its first parameter optional. I'd still allow it to be passed, but given that RSA has an optional integer parameter, if nothing is passed or an integer is passed, I'd run RSA key pair generation.
  • why is luaopen_lcrypto there if it doesn't seem to be defined/used?

Copy link
Collaborator

@pkulchenko pkulchenko left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

@jart, does the API look good to you?

mterron added 6 commits June 3, 2025 20:38
Cleanup language on the test file
Add definitions
Align function name
The options are now passed in a table instead of positional parameters. This is not LuaCrypto compatible but it is a nicer interface.
Copy link
Collaborator

@pkulchenko pkulchenko left a comment

Choose a reason for hiding this comment

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

Had a chance to review added methods and updated API. Looks good to me. Thanks!

Add correct jwltopem and pemtojwk function
Expand tests
@mterron mterron changed the title Add LuaCrypto compatible functions (plus some auxiliary functions) as… redbean: Add LuaCrypto compatible functions (plus some auxiliary functions) Jun 7, 2025
@solisoft
Copy link
Contributor

Awesome !

@pkulchenko pkulchenko marked this pull request as draft June 24, 2025 00:39
Improve error messages
Improve parameter validation
Correct base64url encoding for JWK
Add support for optional claims to convertPemToJwk
Expand test coverage
Add basic definitions
@pkulchenko
Copy link
Collaborator

@mterron, thank you for the updates! I liked added error checking and new functions! Noticed a couple of things to check on (couldn't leave comments on individual lines for some reason, so summarizing it here):

  • line 502: Previously this would return NULL for an unsupported hash algorithm, but now it will store -1, which will then gets passed to mbedtls_md_info_from_type and other functions. Is that expected?
  • similarly line 612, 723, 886; I'd consider implementing something similar to what you did on line 932.
  • line 694: should hash_name be checked for validity?
  • 808 and 810 have DEBUGF for the hash, but not other places with similar processing.
  • I'm curious, why 989, 994 and 999 lines were replaced? should be the same result, no?
  • if checks on lines 1369 and 1371 are the same, which makes the content of else if on line 1378 unreachable. Is there an issue with the logic?
  • Indentation on line 1386 seems to be off. Also on line 1918.

mterron added 2 commits June 24, 2025 20:30
Improve JWK functions
Enable PKCS1 v2.1 support in MBEDTLS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants