One-shot AES CBC and ECB #2406
Comments
|
Do we want to make it easier to use ECB? :) |
|
@scalablecory I even pre-apologized for the suggestion at the very beginning 😅. If it were up to me ECB of any kind would be in an out of band package and removed from shared framework. That said, I know there is a very real need for it by some folks, so I threw it out there as an idea. When / if this comes to an API proposal the thought can be put further to the test. |
|
ECB is important as the building block for things that we don't have intrinsically, like AES-KW (IIRC that uses ECB)--if nothing else, pretty much every other mode can be written in terms of ECB. I think the one shots are good, but I don't know that we want distinct types for classic modes. I had pictured these as just And I don't think it makes sense to drop support for the alternate padding schemes. AesCcm and AesGcm got new types because they're fundamentally different concepts than "just AES", and we wanted to not have any mode where the IV (nonce) got reused because of a property existing where it shouldn't have. And the notion of "what tag sizes are supported" is different per tag algorithm (could have been EncryptGcm, DecryptGcm, SupportedGcmTagSizes, etc, I guess... Really the problem I want to avoid here is that when AES2 comes around we don't end up with type explosions from { AES, AES2, 3DES } x { CBC, ECB, CTR, ... }. The places that it makes sense to be a bit polymorphic we should be, and CBC/ECB/CTR/etc feel more like different methods to me (verbs/adverbs) than different classes (nouns). |
Yes, I'm not a fan of the IV property since that makes the developer think the IV has the same use / lifetime as the key.
The usage of AES is rarely polymorphic in my own uses - the usage of AES is almost always specific to what mode it is in. AES ECB can only be used as a building block for key-wrap or rare occurrences of "I have only one block to encrypt with one time use keys". CBC needs a CSPRNG IV and does not tolerate predictable IVs. CTR tolerates predictable IVs and can be a counter, or it can be zero if the key is used once. Etc, etc. So while the API shape may be polymorphic I don't really think it can ever be used as such. My thought was to encode that in the type.
Fair enough. I don't have strong arguments to remove it other than I will personally avoid them. I was hoping to push the padding handling down to the implementation. |
Well, I mean more like "RC2, DES, 3DES, and AES all support ECB, CBC, ...", so something like partial class SymmetricAlgorithm
{
// default(PaddingMode) => use the property?
public int EncryptEcb(ReadOnlySpan<byte> source, Span<byte> destination, PaddingMode paddingMode = default);
public int DecryptEcb(ReadOnlySpan<byte> source, Span<byte> destination, PaddingMode paddingMode = default);
protected virtual bool TryTransformCbc(ReadOnlySpan<byte> source, ReadOnlySpan<byte> iv, Span<byte> destination, PaddingMode paddingMode, bool encrypt);
// Maybe change iv to Span and add an optional parameter for "generate it for me",
// but perhaps either "generate it for me" or "I already have one" is a named alternative,
// such as EncryptCbcWithIv, EncryptCbcWithRandomIv
public int EncryptCbc(ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte> destination, PaddingMode paddingMode = default);
public int DecryptCbc(ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, Span<byte> destination, PaddingMode paddingMode = default);
// Split nonce and counter? Unify them?
public int EncryptCtr(ReadOnlySpan<byte> plaintext, ..., Span<byte> destination);
public int DecryptCtr(ReadOnlySpan<byte> ciphertext, ..., Span<byte> destination);
// I suppose for consistency, feedbackSize == 0 means "use the property".
// Same IV questions as CBC
public int EncryptCfb(..., int feedbackSize = 0);
public int DecryptCfb(..., int feedbackSize = 0);
// OFB, CTS?, etc.
}All of the public methods (span-accepting, byte[] returning, Span-Try, etc) can do argument validation (and, when appropriate, random-value filling) then defer the single (per mode) No IV/Nonce value would ever be read from the IV property, but Padding and FeedbackSize seem like they can (as you rarely want to change them). |
Okay, if per-type modes is not looking like a good idea, then I would propose: namespace System.Security.Cryptography {
public abstract class SymmetricAlgorithm {
public byte[] EncryptEcb(ReadOnlySpan<byte> plaintext, PaddingMode paddingMode = default);
public bool TryEncyptEcb(ReadOnlySpan<byte> plaintext, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode = default);
public byte[] EncryptCbc(ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, PaddingMode paddingMode = default);
public bool TryEncyptCbc(ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode = default);
public byte[] DecryptEcb(ReadOnlySpan<byte> ciphertext, PaddingMode paddingMode = default);
public bool TryDecryptEcb(ReadOnlySpan<byte> ciphertext, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode = default);
public byte[] DecryptCbc(ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, PaddingMode paddingMode = default);
public bool TryDecryptCbc(ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode = default);
}
}Thoughts.
|
Yeah, they wouldn't need to be done yet, but having a pattern worked out ahead of time seems virtuous. (CFB, at least, is on the 5.0 plan). |
Hm. That one is new to me. I struggle to find this documentation in the repository. There's this but doesn't mention that. Anyway, seems like a reasonable thing, updated. |
|
That one is at https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/virtual-members. As part of updating the guidelines for a 3rd edition of Framework Design Guidelines we're also looking at how to better (more modernly?) share the guidelines. (Since there's a published book involved there's a bit of copyright/contract complication that slows things down a bit) |
|
Just throwing this out there for discussion... If we were to reinvent the In my ideal world, the API would look something like below. (Ignore for now that it violates the FDG.) public abstract class SymmetricAlgorithm2 : IDisposable
{
public abstract int Decrypt(ROS<byte> src, Span<byte> dest);
public abstract int Encrypt(ROS<byte> src, Span<byte> dest);
public abstract void Dispose();
}
public sealed class AesAlgorithm2 : SymmetricAlgorithm2
{
public AesAlgorithm(ReadOnlySpan<byte> key, ChainingMode chainingMode, PaddingMode paddingMode);
}Basically, everything about how the algorithm behaves is fixed at construction time, and there's just Edit: I should clarify that this sketch API is only useful for one-shot. It's useless for streaming. |
Probably? Keys are usually tied to some lifetime of "thing which can do encryption with the key" like a
I seem to have very different tastes from y'all on this. In my "ideal" world the mode is part of the type (hence my original suggestion) or at least something that is "compile time", not some attribute on a thing. The mode simply dictates too much of the API shape for me, and like CNG / BCrypt, you end up with some invariant structure of "stuff that only applies to the mode" like I covered a lot of that above, so I won't reiterate it, but between feedback size, IV, nonces, etc, and some other modes of AES that are even more bizarre like IGE which has two IVs, trying to cram the mode into a single API surface doesn't make a lot of sense to me. If the mode dictates what information is necessary for it to function, then a "base" encrypt / decrypt doesn't work. The sketched API above doesn't have an IV in it anywhere, for example. Where does it go? It can't go on Encrypt / Decrypt since You can have a method per-mode, or a type per-mode, and now I'm back to where I started. |
No, I wouldn't bother with letting you get it back 😄. Maybe an export method, definitely not a property.
I agree with @vcsjones, the mode inputs don't lend themselves to a general "encrypt/decrypt" virtual. Looking at the current model (ignoring CreateEncryptor(key,iv)), the strawman SymmetricAlgorithm2, and what we did for GCM/CCM, the instance is "the key", and then we need to model how things work.
Perhaps, but we can't (shouldn't/won't) take a PVOID for "uh, extra stuff needed for the mode...", like the IV/Nonce/TagSize/etc.
Yeah, and both are fine-ish. Type-per-mode makes things like PBES less pleasant since there's no unifying representation of (e.g.) CBC, which is why I'm favoring method(-pair)-per-mode. Type-per-algorithm-and-mode also makes for a lot more namespace-level noise for IntelliSense. (Type-per-mode DOES have the benefit of suggesting that you shouldn't mix modes with the same key (or, at least, not semi-suggesting mixing modes is OK), but I don't feel it does that "strongly" enough to offset the value lost to things like the PBES implementations for processing PKCS8/PKCS12). |
Sure I agree with that. I took the question to mean "is the key an attribute of the object instance or method call". I meant to say it should not be a method parameter. |
Well, I suppose you can fix that with moar types... public interface ICbcMode {
bool TryEncyptCbc(ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode = default);
bool TryDecyptCbc(ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode = default);
}
public class AesCbc : ICbcMode {
}
public class TripleDesCbc : ICbcMode {
}I'm not sure if I am serious or not, but I see your point.
Could stick it in a new
I think part of what I don't like about mode methods is that it forever implies that any new mode might apply to all future algorithms. Perhaps AES2 won't define any of these modes. Perhaps it'll only define GCM-SIV. |
There is a bit of a point-in-time "things that work everywhere" (not CCM/GCM) for deciding things can go on SymmetricAlgorithm, but that was really already done by the Mode property. The ultimate recourse is "NotSupportedException" if some future algorithm is not defined for ECB/CBC/etc modes. But since we use SymmetricAlgorithm for block ciphers, and the block modes apply to all block ciphers, I don't really see a place where it would fall apart. (If we added a stream cipher, it'd fall apart, but so would the current model. If we ever added one it'd probably need a new base class already, or just be its own, standalone, thing.) |
|
Okay. In that case then I will stick with my API suggestion here. I might also suggest an API for getting the ciphertext size based on padding? // TODO: validation, no overflowing, etc.
public int GetCipherTextLength(PaddingMode paddingMode, int plaintextLength) {
switch (paddingMode) {
case PaddingMode.None:
return plaintextLength; //TODO: throw if % BlockSize != 0?
case PaddingMode.Zeros:
case PaddingMode.PKCS7:
case PaddingMode.ANSIX923:
case PaddingMode.ISO10126:
return (plaintextLength | BlockSize - 1) + 1;
default:
throw new CryptographicException();
}
} |
|
My suggestion was a per-mode method like |
Doh.
I separated them, only because I agree with the straight-forward remark and it is something I would expect very few developers to actually do. namespace System.Security.Cryptography {
public abstract class SymmetricAlgorithm {
public byte[] EncryptEcb(ReadOnlySpan<byte> plaintext, PaddingMode paddingMode = default);
public bool TryEncyptEcb(ReadOnlySpan<byte> plaintext, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode = default);
public byte[] EncryptCbc(ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, PaddingMode paddingMode = default);
public bool TryEncyptCbc(ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode = default);
public byte[] DecryptEcb(ReadOnlySpan<byte> ciphertext, PaddingMode paddingMode = default);
public bool TryDecryptEcb(ReadOnlySpan<byte> ciphertext, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode = default);
public byte[] DecryptCbc(ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, PaddingMode paddingMode = default);
public bool TryDecryptCbc(ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode = default);
protected virtual bool TryEncryptCbcCore(ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode);
protected virtual bool TryDecryptCbcCore(ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode);
protected virtual bool TryEncryptEcbCore(ReadOnlySpan<byte> plaintext, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode);
protected virtual bool TryDecryptEcbCore(ReadOnlySpan<byte> ciphertext, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode);
public int GetCiphertextLength(PaddingMode paddingMode, int plaintextLength);
}
}Both you and @GrabYourPitchforks's |
It's something that can be done in addition to the Try. If I'm a caller with a presized destination (because I used GetCiphertextLength or it's decrypt and I provided a big enough buffer) I don't ever expect false. Rather than redundantly checking, or worse... ignoring the return, I can call the int-returning non-Try.
FWIW, here's a sample of almost every possible variation of a Span-writing thing. Which ones are appropriate depends a lot on context: public partial class ByteOperations {
public static byte[] FromHexadecimal(ReadOnlySpan<char> source) { ... }
public static byte[] FromHexadecimal(char[] source) { ... }
public static byte[] FromHexadecimal(char[] source, int sourceIndex, int count) { ... }
public static byte[] FromHexadecimal(ArraySegment<char> source) { ... }
public static byte[] FromHexadecimal(string source) { ... }
public static int FromHexadecimal(ReadOnlySpan<char> source, Span<byte> destination) { ... }
public static int FromHexadecimal(char[] source, byte[] destination) { ... }
public static int FromHexadecimal(byte[] source, int sourceIndex, char[] destination, int destinationIndex, int count) { ... }
public static int FromHexadecimal(ArraySegment<char> source, ArraySegment<byte> destination) { ... }
public static bool TryFromHexadecimal(ReadOnlySpan<char> source, Span<byte> destination, out int bytesWritten) { ... }
public static bool TryFromHexadecimal(char[] source, byte destination, out int bytesWritten) { ... }
public static bool TryFromHexadecimal(char[] source, int sourceIndex, byte destination, int destinationIndex, int count, out int bytesWritten) { ... }
public static bool TryFromHexadecimal(ArraySegment<char> source, ArraySegment<byte> destination, out int bytesWritten) { ... }
public static OperationStatus FromHexadecimal(ReadOnlySpan<char> source, Span<byte> destination, out int charsConsumed, out int bytesWritten) { ... }
public static OperationStatus FromHexadecimal(char[] source, byte[] destination, out int charsConsumed, out int bytesWritten) { ... }
public static OperationStatus FromHexadecimal(char[] source, int sourceIndex, byte[] destination, int destinationIndex, int count, out int charsConsumed, out int bytesWritten) { ... }
public static OperationStatus FromHexadecimal(ArraySegment<char> source, ArraySegment<byte> destination, out int charsConsumed, out int bytesWritten) { ... }
}Since this one isn't char-based the string overloads drop out. OperationStatus isn't needed because there's no invalid input data to report. We don't really need the array-with-offset versions or ArraySegment. So a full version of EncryptCbc might be public byte[] EncryptCbc(byte[] plaintext, byte[] iv, PaddingMode paddingMode = default);
public byte[] EncryptCbc(ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, PaddingMode paddingMode = default);
public int EncyptCbc(ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte> destination, PaddingMode paddingMode = default);
public bool TryEncyptCbc(ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte>
destination, out int bytesWritten, PaddingMode paddingMode = default);
// All of the above can be written in terms of this.
protected virtual bool TryEncryptCbcCore(ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode); |
What's the advantage of taking a |
|
CLR languages that don't support implicit conversions; or developers who don't understand that there's an implicit conversion. When we first started making (ReadOnly)Span-only things we found out that none of us paid attention to the fact that F# won't use implicit conversions, and we made the calling syntax really ugly. F# now has a special rule to permit |
|
As always, thank you for the detailed explanations. Given that, the proposal now looks something like this: namespace System.Security.Cryptography {
public abstract class SymmetricAlgorithm {
public byte[] EncryptEcb(byte[] plaintext, PaddingMode paddingMode = default);
public byte[] EncryptEcb(ReadOnlySpan<byte> plaintext, PaddingMode paddingMode = default);
public int EncyptEcb(ReadOnlySpan<byte> plaintext, Span<byte> destination, PaddingMode paddingMode = default);
public bool TryEncyptEcb(ReadOnlySpan<byte> plaintext, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode = default);
public byte[] EncryptCbc(byte[] plaintext, byte[] iv, PaddingMode paddingMode = default);
public byte[] EncryptCbc(ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, PaddingMode paddingMode = default);
public int EncyptCbc(ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte> destination, PaddingMode paddingMode = default);
public bool TryEncyptCbc(ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode = default);
public byte[] DecryptEcb(byte[] ciphertext, PaddingMode paddingMode = default);
public byte[] DecryptEcb(ReadOnlySpan<byte> ciphertext, PaddingMode paddingMode = default);
public int DecryptEcb(ReadOnlySpan<byte> ciphertext, Span<byte> destination, PaddingMode paddingMode = default);
public bool TryDecryptEcb(ReadOnlySpan<byte> ciphertext, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode = default);
public byte[] DecryptCbc(byte[] ciphertext, byte[] iv, PaddingMode paddingMode = default);
public byte[] DecryptCbc(ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, PaddingMode paddingMode = default);
public int DecryptCbc(ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, Span<byte> destination, PaddingMode paddingMode = default);
public bool TryDecryptCbc(ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode = default);
protected virtual bool TryEncryptCbcCore(ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode);
protected virtual bool TryDecryptCbcCore(ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode);
protected virtual bool TryEncryptEcbCore(ReadOnlySpan<byte> plaintext, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode);
protected virtual bool TryDecryptEcbCore(ReadOnlySpan<byte> ciphertext, Span<byte> destination, out int bytesWritten, PaddingMode paddingMode);
public int GetCiphertextLength(PaddingMode paddingMode, int plaintextLength);
}
} |
|
API review: CBC padding mode should default to PKCS7, ECB padding modes should be non-default (require explicit value at call site). For all methods, move non-out parameters before out parameters. @bartonjs - please document whether the newly introduced virtual members would ever mutate the underlying instance (changing block mode or padding mode) or throw. |
|
Thanks. Will take this if someone wants to assign to me. |
|
@vcsjones Done. FWIW, during the meeting we landed on the Core methods NOT changing the mode, so they'll throw in the wrong mode. It'll only matter for custom types that directly extend SymmetricAlgorithm, but @GrabYourPitchforks was concerned with people "knowing" that they can call CreateEncryptor/CreateDecryptor across threads without problems; so we'll stay defensive by default. |
|
@bartonjs Hm. It sounds like y'all had an idea on implementation that either I am misunderstanding or I am misunderstanding your comment / concerns.
Do you mean the instance property
What is a "wrong" mode? One that does not match the instance's value? |
Optional parameters need to be last. Since ECB would no longer have an optional parameter, the public bool TryEncyptEcb(
ReadOnlySpan<byte> plaintext,
Span<byte> destination,
PaddingMode paddingMode,
out int bytesWritten);
public bool TryEncyptCbc(
ReadOnlySpan<byte> plaintext,
ReadOnlySpan<byte> iv,
Span<byte> destination,
out int bytesWritten,
PaddingMode paddingMode = PaddingMode.PKCS7);Or would you expect the order to be consistent across modes: public bool TryEncyptEcb(
ReadOnlySpan<byte> plaintext,
Span<byte> destination,
out int bytesWritten,
PaddingMode paddingMode);
public bool TryEncyptCbc(
ReadOnlySpan<byte> plaintext,
ReadOnlySpan<byte> iv,
Span<byte> destination,
out int bytesWritten,
PaddingMode paddingMode = PaddingMode.PKCS7); |
public bool TryEncyptEcb(
ReadOnlySpan<byte> plaintext,
Span<byte> destination,
PaddingMode paddingMode,
out int bytesWritten);
public bool TryEncyptCbc(
ReadOnlySpan<byte> plaintext,
ReadOnlySpan<byte> iv,
Span<byte> destination,
out int bytesWritten,
PaddingMode paddingMode = PaddingMode.PKCS7);is correct for what the group decided. The two calls won't look the same. But that's fine, it further helps to make a brain engage when switching modes 😄. As for the implementation, while all of the built-in types are going to override the virtual *Core methods, the base class has to do one of:
The suggestion is to do (3). |
bartonjs
added
the
api-needs-work
|
@bartonjs Looks good to me; I made one small change to fix the "Encypt" -> "Encrypt" typo. |
bartonjs
added
api-ready-for-review
|
Looks good as proposed: namespace System.Security.Cryptography
{
public abstract class SymmetricAlgorithm
{
// ECB (Block Aligned, no default padding, no IV)
public byte[] DecryptEcb(byte[] ciphertext, PaddingMode paddingMode);
public byte[] DecryptEcb(ReadOnlySpan<byte> ciphertext, PaddingMode paddingMode);
public int DecryptEcb(ReadOnlySpan<byte> ciphertext, Span<byte> destination, PaddingMode paddingMode);
public byte[] EncryptEcb(byte[] plaintext, PaddingMode paddingMode);
public byte[] EncryptEcb(ReadOnlySpan<byte> plaintext, PaddingMode paddingMode);
public int EncryptEcb(ReadOnlySpan<byte> plaintext, Span<byte> destination, PaddingMode paddingMode);
public bool TryDecryptEcb(
ReadOnlySpan<byte> ciphertext, Span<byte> destination, PaddingMode paddingMode, out int bytesWritten);
protected virtual bool TryDecryptEcbCore(
ReadOnlySpan<byte> ciphertext, Span<byte> destination, PaddingMode paddingMode, out int bytesWritten);
public bool TryEncryptEcb(
ReadOnlySpan<byte> plaintext, Span<byte> destination, PaddingMode paddingMode, out int bytesWritten);
protected virtual bool TryEncryptEcbCore(
ReadOnlySpan<byte> plaintext, Span<byte> destination, PaddingMode paddingMode, out int bytesWritten);
// CBC (Block Aligned, default to PKCS#7 padding, IV required)
public byte[] DecryptCbc(byte[] ciphertext, byte[] iv, PaddingMode paddingMode = PaddingMode.PKCS7);
public byte[] DecryptCbc(
ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, PaddingMode paddingMode = PaddingMode.PKCS7);
public int DecryptCbc(
ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, Span<byte> destination,
PaddingMode paddingMode = PaddingMode.PKCS7);
public byte[] EncryptCbc(byte[] plaintext, byte[] iv, PaddingMode paddingMode = PaddingMode.PKCS7);
public byte[] EncryptCbc(
ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, PaddingMode paddingMode = PaddingMode.PKCS7);
public int EncryptCbc(
ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte> destination,
PaddingMode paddingMode = PaddingMode.PKCS7);
public bool TryDecryptCbc(
ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten,
PaddingMode paddingMode = PaddingMode.PKCS7);
protected virtual bool TryDecryptCbcCore(
ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, Span<byte> destination, PaddingMode paddingMode,
out int bytesWritten);
public bool TryEncryptCbc(
ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten,
PaddingMode paddingMode = PaddingMode.PKCS7);
protected virtual bool TryEncryptCbcCore(
ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte> destination, PaddingMode paddingMode,
out int bytesWritten);
}
public abstract class SymmetricAlgorithm
{
// CFB (Feedback-size aligned, default to No padding, IV required, adds feedback size (default 8))
public byte[] DecryptCfb(
byte[] ciphertext, byte[] iv, PaddingMode paddingMode = PaddingMode.None, int feedbackSizeBits = 8);
public byte[] DecryptCfb(
ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, PaddingMode paddingMode = PaddingMode.None,
int feedbackSizeBits = 8);
public int DecryptCfb(
ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, Span<byte> destination,
PaddingMode paddingMode = PaddingMode.None, int feedbackSizeBits = 8);
public byte[] EncryptCfb(
byte[] plaintext, byte[] iv, PaddingMode paddingMode = PaddingMode.None, int feedbackSizeBits = 8);
public byte[] EncryptCfb(
ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, PaddingMode paddingMode = PaddingMode.None,
int feedbackSizeBits = 8);
public int EncryptCfb(
ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte> destination,
PaddingMode paddingMode = PaddingMode.None, int feedbackSizeBits = 8);
public bool TryDecryptCfb(
ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten,
PaddingMode paddingMode = PaddingMode.None, int feedbackSizeBits = 8);
public bool TryEncryptCfb(
ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte> destination, out int bytesWritten,
PaddingMode paddingMode = PaddingMode.None, int feedbackSizeBits = 8);
protected virtual bool TryDecryptCfbCore(
ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> iv, Span<byte> destination, PaddingMode paddingMode,
int feedbackSizeBits, out int bytesWritten);
protected virtual bool TryEncryptCfbCore(
ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> iv, Span<byte> destination, PaddingMode paddingMode,
int feedbackSizeBits, out int bytesWritten);
}
public abstract class SymmetricAlgorithm
{
public int GetCiphertextLengthEcb(int plaintextLength, PaddingMode paddingMode);
public int GetCiphertextLengthCbc(int plaintextLength, PaddingMode paddingMode = PaddingMode.PKCS7);
public int GetCiphertextLengthCfb(
int plaintextLength, PaddingMode paddingMode = PaddingMode.None, int feedbackSizeBits = 8);
}
} |
bartonjs
added
api-approved
Of course you'd say that 😝 |
|
@bartonjs what is the possibility that this could come in as smaller PRs? Perhaps something like:
I could understand being iffy on splitting 2 and 3. Maybe you have a better idea how to slice, or maybe you want it all in one go. That's cool too, just floating the idea. |
|
I'm cool with whatever staging makes sense, provided that tests and code comes together. By mode, Span-writing things first then coming back and adding the ease-of-use variants, Span-writing things first+by mode. Doing the ease-of-use non-virtuals first wouldn't be great, since they'd be hard to test ;); other than that, I don't see any obviously bad plans. |
ghost
added
in-pr
ghost
added
the
in-pr
ghost
removed
the
in-pr
|
🥳 |
.NET Core 3.0 has
AesCcmandAesGcmtype that are simple, one-shot APIs. I really like the API shape of these and would suggest that it might be worth addingAesCbcandAesEcb(sorry) types.The one-shot (no streaming, no update / finalize) would give a few benefits.
I like that the mode is part of the type. The modes aren't really interchangeable, and having them as part of the type helps suggest that. That is, as far as using a primitive goes, I can't really "upgrade" from one mode to another like it's some kind of configuration. It requires some thought and consideration as to how each mode behaves.
Type-per-mode would allow the API shape to fit the mode better (no IV for ECB).
One-shot usage would be hard to "you're holding it wrong". The API surface would be simple
DecryptandEncryptmethods.One-shot API would mean there is no state to carry between calls (like CNG updating the IV).
That might afford a few more opportunities to use CryptoPool or stack buffers instead of
new byte[].Do not support some less common types of block padding (PKCS#7 and no padding only). This can help developers make better choices security-wise and push handling down to the underlying implementation.
/cc @GrabYourPitchforks @bartonjs
Updated API Proposal
Since the API review we added CFB support, so this adds the CFB members, and adds a mild reshaping to account for CFB being different than ECB and CBC:
Current approved additions, with no updates required:
CFB additions
Modifications of existing approved members:
namespace System.Security.Cryptography { public abstract class SymmetricAlgorithm { - public int GetCiphertextLength(PaddingMode paddingMode, int plaintextLength); + public int GetCiphertextLengthEcb(int plaintextLength, PaddingMode paddingMode); + public int GetCiphertextLengthCbc(int plaintextLength, PaddingMode paddingMode = PaddingMode.PKCS7); + public int GetCiphertextLengthCfb( + int plaintextLength, PaddingMode paddingMode = PaddingMode.None, int feedbackSizeBits = 8); } }Previously discussed, outdated, API Proposal
The text was updated successfully, but these errors were encountered: