entronad / crypto-es

A cryptography algorithms library
Other
275 stars 31 forks source link

[index.d.ts] CipherParams should have optional fields #4

Closed AlttiRi closed 4 years ago

AlttiRi commented 4 years ago

CipherParams should have optional fields, not required, like it is for CipherParamsData in index.d.ts for original CryptoJS 3.1.2.

I want to directly pass a WordArray, but I get the warning from IDE for a code like this:

    CryptoES.AES.decrypt(
        {ciphertext: wordArray},
        key,
        {iv}
    );

warn

decrypt method uses only the ciphertext field from the passed CipherParamsData (CryptoJS) / CipherParams (CryptoES) object.

entronad commented 4 years ago

I don't think the first param of decrypt() can be a wordArray.

As is sayed in the "The Cipher Output" in README: The result of encryp() is a CipherParams, it is the full information of the encryption, not mearly the wordArray. CipherParams has a toString() method to present this object as a string. So the union type (CipherParams | string) of the first param in decrypt() refers to this object and it's string form, only a wordArray or {ciphertext: wordArray} is incomplete.

So the proper usage is:

const encrypted = C.AES.encrypt("Message", "Secret").toString();
// send or store the encryped string.
const decrypted = C.AES.decrypt(encrypted, "Secret").toString(C.enc.Utf8);

While:

const encrypted = C.AES.encrypt("Message", "Secret").wordArray;
const decrypted = C.AES.decrypt({ciphertext: encrypted}, "Secret").toString(C.enc.Utf8);

will get a wrong answer.

AlttiRi commented 4 years ago

The reason of the wrong answer is a salt that CryptoJS creates only when you pass the key as String. The correct version:

const cipherParams = C.AES.encrypt("Message", "Secret");
const decrypted = C.AES.decrypt({
    ciphertext: cipherParams.ciphertext,
    salt: cipherParams.salt
}, "Secret");
console.log(decrypted2.toString(C.enc.Utf8)); // "Message"

The generation of the salt for the String key is not the related to AES work.

I want to use only AES functional, without "bonus functional". For that is enough: data WordArray, key WordArray, iv WordArray, padding (optionally) and mode (optionally).

const input = C.lib.WordArray.create([-61338304, 1191554385, -138865087, -290257254], 16);
const key = C.lib.WordArray.create([2045645054, -2029791820, -1952917600, -199318521], 16);
const iv = C.lib.WordArray.create([0, 0, 0, 0], 16);

const ciphertext = C.AES.encrypt(input, key, {
    iv,
    padding: C.pad.NoPadding
}).ciphertext;

const decrypted = C.AES.decrypt({ciphertext}, key, {
    iv,
    padding: C.pad.NoPadding
});
console.log(decrypted); // === input
entronad commented 4 years ago

I got your point now. I thought you want to change the type of first param of decode() before. But in fact you want to say that for the CipherParams class, only ciphertext is requiered and other fields are optional. Right?

AlttiRi commented 4 years ago

I think so. At least for decrypt function only ciphertext is required, and salt as an optional param (when you pass the key as String), other things will be ignored (key, iv).

One more example:

const input = _arrayBufferToWordArray(new TextEncoder().encode("Message"));
const key = C.lib.WordArray.create([2045645054, -2029791820, -1952917600, -199318521], 16);
const iv = C.lib.WordArray.create([1, 2, 3, 4], 16);

const ciphertext = C.AES.encrypt(input, key, {iv}).ciphertext;
const decrypted = C.AES.decrypt({ciphertext}, key, {iv});

console.log(new TextDecoder().decode(_wordArrayToArrayBuffer(decrypted))); // Message

This code does not work, although cipherParams contains both key and iv:

const cipherParams = C.AES.encrypt(input, key, {iv});
const decrypted = C.AES.decrypt(cipherParams);
entronad commented 4 years ago

I have looked into this now. In fact, CipherParams class has not only fields but some methods: https://github.com/entronad/crypto-es/blob/6f6bd2235f987050a687c0b3e27279ff7d1c6ee4/lib/index.d.ts#L500 That's the main reason of the typescript error. So plain JS objects like {ciphertext} will not work even if the fields are set optional. I suggest the best way is to use a creator or constructor. the params of the creator are quite loose as you need:

C.AES.decrypt(C.lib.CipherParams.create({ciphertext}), key, {iv});
or
C.AES.decrypt(new C.lib.CipherParams({ciphertext}), key, {iv});

If I change the param of decode() to simply String | object, only C.AES.decrypt({ciphertext}, key, {iv}) will be enough. But I don't think it's a good idea. The reason to use TypeScript is for type safety, we'd better not not to fallback to the plain JS objects, a little bit more codes is nessesary.

AlttiRi commented 4 years ago

index.d.ts of the original CryptoJS 3.1.2 has 2 types CipherParamsData (all fields are optional):

interface CipherParamsData {
    ciphertext?: lib.WordArray
    key?: lib.WordArray
    iv?: lib.WordArray
    salt?: lib.WordArray
    algorithm?: Cipher
    mode?: mode.IBlockCipherModeImpl
    padding?: pad.IPaddingImpl
    blockSize?: number
    formatter?: format.IFormatter
}

and CipherParams:

interface CipherParams extends Base, CipherParamsData {
    init(cipherParams?: CipherParamsData): void
    create(cipherParams?: CipherParamsData): CipherParams
    toString(formatter?: format.IFormatter): string
}

decrypt method accepts especially CipherParamsData.

You can do it like this. Currently you use CipherParams everywhere .

In fact I just want to hide IDE warning.

AlttiRi commented 4 years ago

By the way,

https://github.com/entronad/crypto-es/blob/6f6bd2235f987050a687c0b3e27279ff7d1c6ee4/lib/index.d.ts#L679

the 3rd parameter should be not just Object, but IBlockCipherCfg.

interface IBlockCipherCfg {
    iv?: WordArray;
    mode?: mode.IBlockCipherModeImpl //default CBC
    padding?: pad.IPaddingImpl //default Pkcs7
}

For AES, DES, TripleDES.

entronad commented 4 years ago

Well, let me have a thorough think and give this "flexible" param types a good solution.

entronad commented 4 years ago

I have added some cfg types: https://github.com/entronad/crypto-es/commit/57f5c48ced90264052fd0c3b201f300b14c57e28 Please upgrade to 1.2.5 and have a try.

AlttiRi commented 4 years ago

BTW, when you use Markdown to put code snippet, it's better to specify the language (js, ts) for color highlighting:

```js 

With:

const hash = CryptoES.MD5("Message");

Without:

const hash = CryptoES.MD5("Message");

It's related to readme file.


this "flexible" param types

It's generics.