Add an RsaEncrypt filter that takes a public key - #137
Draft
NickJosevski wants to merge 1 commit into
Draft
Conversation
Follows droyad's suggestion on #39 that encoding just the public key would be more reusable than passing a whole X509 certificate. The filter takes a base64 SubjectPublicKeyInfo, encrypts with OAEP-SHA256 and returns base64. Failures are reported in the output as [RsaEncrypt error: ...] rather than returned as null. A null return leaves the raw #{...} in place, which reads as though no encryption was asked for, and silently not encrypting is the wrong default for this. Direct RSA can only encrypt a couple of hundred bytes, so the size ceiling is checked up front and reported with the actual numbers rather than surfacing as an opaque CryptographicException. Refs #39, #38 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #39, #38. Draft — the .NET Framework question below needs a decision before this is worth reviewing properly.
Follows droyad's suggestion on #39 that encoding just the public key would be more reusable than passing a whole X509 certificate. That also sidesteps the certificate store and
X509KeyStorageFlagsproblem that stalled #38 in 2020.Takes a base64
SubjectPublicKeyInfo, encrypts with OAEP-SHA256, returns base64.What is different from #38
Failures are loud. #38 caught every exception and returned
null, which leaves the raw#{...}in the output — indistinguishable from nobody having asked for encryption. This returns[RsaEncrypt error: ...], following the existingUriPartprecedent:[RsaEncrypt error: the public key is not valid base64][RsaEncrypt error: the public key could not be read as a SubjectPublicKeyInfo structure][RsaEncrypt error: the input is 200 bytes, which is more than the 190 bytes a 2048 bit key can encrypt directly]The size ceiling is checked up front. Direct RSA-OAEP-SHA256 with a 2048 bit key tops out at 190 bytes. #38 would have thrown, swallowed it and silently not encrypted.
OAEP-SHA256 rather than OAEP-SHA1.
The tests actually decrypt. #38 only asserted the result was non-empty, and its cert helper returned
nulloutsideNETCOREAPPso the whole test body was skipped on .NET Framework. These round-trip through the private key and assert the plaintext comes back.Open question: .NET Framework
RSA.ImportSubjectPublicKeyInfodoes not exist on net462, which this library still targets. Right now that path returns[RsaEncrypt error: not supported when Octostache is running on .NET Framework].The alternatives are hand-rolling SPKI DER parsing for net462, which I would rather not do in a crypto path, or accepting an X509 certificate on that target instead. Worth deciding before this goes further.
Still true regardless
OAEP is randomised, so the same value encrypts differently every evaluation and a substituted config file will show a diff on every deployment. That is inherent, not a bug, but it should be documented if this lands.
656 tests pass on net10.0. Not verified on net48 — that only builds on Windows.