Skip to content

Fix RSA-OAEP encryption for empty strings #57558

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

Closed

Conversation

AmitPrajapati-1
Copy link

No description provided.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Mar 20, 2025
oaepHash: 'sha1',
padding: crypto.constants.RSA_PKCS1_OAEP_PADDING,
key: keys.privateKey
}, Buffer.from(ciphertext, 'base64')).toString('utf8').trim();

Choose a reason for hiding this comment

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

Using the last trim() function maynot a good idea. In my opinion, the fix should be in the internal C level.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed encryption and decryption logic for improved accuracy and consistency.

@TheNorthMemory
Copy link

Ops... The trouble looks like still there on v23.7.0/v23.8.0/v23.9.0/v23.10.0. On another way, the ciphertext should be in base64 string instead of the Buffer type. It's most for the external applications data exchange.

@AmitPrajapati-1
Copy link
Author

Ohh.. thanks for the clarification. I have added error handling for empty strings in the C-level code. Please check this PR: #57572.

@panva
Copy link
Member

panva commented Mar 21, 2025

Superseded by #57572

@panva panva closed this Mar 21, 2025
panva added a commit to panva/node that referenced this pull request Mar 21, 2025
@AmitPrajapati-1 AmitPrajapati-1 deleted the fix-crypto-private-decrypt branch March 22, 2025 04:40
nodejs-github-bot pushed a commit that referenced this pull request Apr 1, 2025
closes #57553
closes #57572
closes #57558

PR-URL: #57575
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
aduh95 pushed a commit that referenced this pull request Apr 1, 2025
closes #57553
closes #57572
closes #57558

PR-URL: #57575
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
JonasBa pushed a commit to JonasBa/node that referenced this pull request Apr 11, 2025
closes nodejs#57553
closes nodejs#57572
closes nodejs#57558

PR-URL: nodejs#57575
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
RafaelGSS pushed a commit that referenced this pull request May 1, 2025
closes #57553
closes #57572
closes #57558

PR-URL: #57575
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
closes #57553
closes #57572
closes #57558

PR-URL: #57575
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants