Skip to content
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

Crypto to SubtleCrypo incompatible [RSA_OAEP Browser Public to Server encrypt] #25756

Closed
MasterJames opened this issue Jan 28, 2019 · 16 comments
Closed
Assignees
Labels
crypto Issues and PRs related to the crypto subsystem.

Comments

@MasterJames
Copy link

MasterJames commented Jan 28, 2019

NodeJS v11.8.0
Linux tstserver 4.15.0-43-generic 46-Ubuntu SMP Thu Dec 6 14:45:28 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
It's Ubuntu in a VirtualBox VM on Windows 8.1

So I am getting pretty sure there is an incompatibility with compatibility between the latest Node Crypto and the Browser's SubtleCrypto.

Here is my logic (it's pretty hard to give code samples with both sides).
I was able to build keys in Node send the public key to the browser encrypt and return for a successful decrypt, so server to browser direction I did do successfully.
I also want to send something from NodeJS to the Browser securely even though it might be over SSL.
I use SubtleCrypto on the browser to generate a temporary key pair for the session (Public and Private).
This way there's no way to intercept the private key for decryption (which is were it fails with DomException).
I can receive the SubtleCrypto Public key from the browser, and use it to encrypt a message, encode it to base64 and on the browser side it receives the message in base64 decodes it to the exact same buffer array the server shows. so none of the base64 to ByteArray stuff is wrong via a visual confirmation.

Still at the nearly final decrypt stage on the Browser side it simply says DomException (which I suppose is poor error messaging on their side) and fails with a crash error.

I believe somehow what you think is needed for decryption with SubtleCrypto is wrong. Browser side can encrypt and decrypt so that is working. The other direction it's working for all stages until the final one goes poof (DomException?).

I suggested there be an example for people to help them as it is untypically difficult in general, but that idea got initially shied away from. It's really why I suggest that as it is important to demonstrate have tests and know it's not breaking etc.
That is here #25589 I might have modified that slightly but those examples are only going the other direction (node to browser). I will try to document this code here now for Browser keys to server encrypt and back to browser decrypt.

First to Generate the keys via SubtleCrypto as crypto on the browsers.

let genKeys = async function ( ciphers ) {
	if( ciphers === undefined ) {
		ciphers = {
			name: "RSA-OAEP",
			modulusLength: 2048,
			publicExponent: new Uint8Array([1, 0, 1]),
			hash: "SHA-256"
		};
	}
	let keys = await crypto.generateKey( ciphers , true, [ "encrypt", "decrypt" ] );
	return {
		ciphers : ciphers,
		prvKey: keys.privateKey,
		pubKey: keys.publicKey
	};
};

Then we need to send the server the Public Key for encryption. This can be one function but it's in two here. I've done all possible variations in regards to no line breaks etc.

let exportKeyAsPEM = async function( key ) {
	let kPrt = await crypto.exportKey( 'spki', key );
	let kStr = String.fromCharCode(...new Uint8Array( kPrt ));
	let kHex = btoa( kStr );
	let kPem = formatKeyAsPEM( kHex );
	console.log( "exportKeyAsPem kPEM:", kPem );
	return kPem;
};

let formatKeyAsPEM = function( key ) {
	let res = '-----BEGIN PUBLIC KEY-----\n';
	while( key.length > 0 ) {
		res += key.substring( 0, 64 ) + '\n';
		key = key.substring( 64 );
	}
	res = res + "-----END PUBLIC KEY-----";
	return res;
};

It's all a little spread out for debugging but since node crypto likes standard PEM file format as key argument (this is great but expected +thanks) and works without error when importing it via nothing special just my socket message object string text.

So on the server side all you really can do is use publicEncrypt with the browsers generate public key like this.
I wrap this stuff up in an object on the node side but I'll modify this here to be similar to how I posted the code from the browser which is also not the traditional 'function name()' I seem to avoid. Also note my utl.log function you can switch to console.log
[was encrypt: function( key, txt ]

let encrypt = function( key, txt ) {
	if( key !== undefined && txt !== undefined ) {
		// debugger;
		// let te = new TextEncoder();
		// let enc = te.encode( buf );
		let buf = Buffer.from( txt );
		let crypt = lib.crypto.publicEncrypt( key, buf );
		let b64 = crypt.toString( 'base64' );

		utl.log( 1, "encrypt: txt:", txt, " buf:", buf, " crypt:", crypt, " b64:", b64 );

		return b64;
	}
};

It seems Buffer.from is exactly the same result as one gets from many other methods like TextEncoder's encode, and many other bit's an bobbins found out in the wild.

Anyway one gets a DomException during decrypt on the Browser via the Private Key like so.

let decrypt = async function ( key, txt ) {
	let rx = /^([0-9a-zA-Z+/]{4})*(([0-9a-zA-Z+/]{2}==)|([0-9a-zA-Z+/]{3}=))?$/;
	let dec, regChk = rx.exec( txt );
	if( regChk == null || regChk[0] !== regChk.input ) {
		let te = new TextEncoder();
		dec = te.encode( txt );
		console.log( "decrypt txt not base64 converted:", txt, "to:", dec );
	}
	else {
		dec = new Uint8Array( atob( txt ).split( '' ).map( function( cca ) { return cca.charCodeAt( 0 ); } ) );
	}
	let cipher = { name: 'RSA-OAEP' };
	let buf = await crypto.decrypt( cipher, key, dec );
	let td = new TextDecoder();
	let res = td.decode( buf );
	console.log( 'decrypt: txt:', txt, 'dec:', dec, 'buf:', buf, 'res:', res );
	return res;
};

The Regex Check is just to make sure there is a valid base64 string to avoid any dysfunctional Exception there. Also I had tried converting if not base64 but that is silly and temporary as I am unable to getting working and clean it up yet.
It's in the SubtleCrypto.decrypt that fires the unhelpful DomException error, but it really seems to me that what you're sending is not right.

If any one can spot the error on my side please do.
The string is maybe 100 let's say 50 characters in length so it fits the 256 Uint8Array size but you'd see a different length exception etc. if that was the issue.

Really by the process of elimination I'm left to believe the error is in how you've encrypted it.
I really don't see how to assess the problem further or prove it's incompatible.

The whole point of Node is to communicate with the Browser (encryption is a must even if SSL is enabled), you need to establish a working example for how this works with Chrome / V8 on the browser and the best suggested Standards there, which I believe is the SubtleCrypto and I think this is sufficient for an expert with Node's Crypto to try this and help explain what's failing.

I strongly feel there really needs to be a way to validate and demonstrate these things are working with tests that alert if something has changed.

I think maybe if the fault is on the browser (easy way out pass the buck) it would be nice to have a flag that one sets if they are encrypting for Chrome/Firefox(? meh) Browser to repair this incompatibility.

As the stuff is randomly encrypted there really no way to analyze the format is flawed. I've checked the public key with OpenSSL and encrypted something and base64 via command line and still that DomException fires.

I feel like the Browser side may not be as responsible for making it stuff work with node, so it seems important to report it here.

I've seen a thread from a few months back that got Locked due to "heated debate" so this is still a serious issue for people. I think I've provided a way to do the right thing and demonstrate more clearly to people how it works and that it works at all. Well at least in this way RSA-OAEP from the Browser Public key to node encrypt and back again. Even one that shows another method but the same ideal pubkey from browser encryption direction.

Any suggestion for a better posting via a fiddle and a nodefiddle I'd be willing to try and do it but really one needs to do this more directly serving the browser etc.

The problem is similar on Firefox.

@Trott Trott added the crypto Issues and PRs related to the crypto subsystem. label Jan 28, 2019
@Trott
Copy link
Member

Trott commented Jan 28, 2019

@nodejs/crypto

@MasterJames MasterJames changed the title Crypto to SubtleCrypo incompatible Crypto to SubtleCrypo incompatible [RSA_OAEP Browser Public to Server encrypt] Jan 28, 2019
@bnoordhuis
Copy link
Member

It would help if you phrased this more as a traditional bug report: I tried action X, I expected outcome Y but instead I got Z. Bonus points if you include a minimal, ready-to-run test case. Right now, it's kind of hard to know where to start.

@MasterJames
Copy link
Author

Yup I tried using node crypto to encrypt with a public key sent from a chrome browser using SubtleCrypto.
I expected it to work.
It failed with DomException to decrypt on the browser side.
Included are all the steps to replicate this complex to reproduce problem.

@bnoordhuis
Copy link
Member

Sorry, but that's not a useful bug report to me. I don't have a couple of hours to go off noodling until I can reproduce your problem. I'm happy to look into it but I'll need minimal and easy steps to reproduce.

@MasterJames
Copy link
Author

I understand thanks for comment.
I still feel some proof or educational documentation to demonstrate how to connect nodejs to chrome browser is worth providing.
At this point i have found other means that suffice.
I am not sure how either would provide a working example of it being broken. Some kind of PR in an area of example server-to-browser source code? Or would that new section be needed first?

@sam-github
Copy link
Contributor

I'm having some trouble understanding the bug flow as well. From what I can understand, the following might be an actionable bug report:

  1. Some js code that can be pasted into a chrome debug console that outputs a public key in PEM
  2. standalone node.js code that reads that public key, encrypts a message with it, and outputs the encrypted data in a copy-pastable form, perhaps a hex encoded string.
  3. Some js code that can be pasted into a chrome debug console that decrypts the pasted data

Or some variation of this, if that is not in fact the problem. Btw, I suspect step 1 is unnecessary, its likely both the public key generation and encryption could occur node.js side. Also, its possibly easier to debug the other way, if its a problem: browser generates key pair and encrypted data, node.js reads private key and data and decrypts it. In this case, you could paste into this bug report complete standalone node.js code that has the key and encrypted data as a string, and then decrypts the data (and fails to get the right data).

You shouldn't need a full-blown app + networking + UI to do any of this.

@MasterJames
Copy link
Author

MasterJames commented Feb 5, 2019

In the example a browser privately generates the key pair sending the public key to nodejs. Nodejs encrypts in the way provided RSA-OAEP a message back to the browser. That message fails to decrypt on the browser side for reason that after sometime led me to believe it's in node that there is a problem.
Anyway it is a complex situation that's why, prior to the bug report I had asked for a place in nodejs document that demonstrates not only it works but how to do it. I provided my best attempts at those fidgety bits (that in my opinion probably should me more encapsulated but are not) in the initial post.
So I guess it would require a PR that is an example of how to use nodejs and the chrome browser to do various forms of encryption.
Without it there is no way to prove it works.
I also pointed to an example that does work for getting a key pair in nodejs and sending it to the browser where the browser uses that server send public key to encrypt a message and node does decrypt it fine going that opposite direction to this issues route.
I think at this point we are asking where should this demo go and then maybe that example folder in the documentation branch a PR with a demonstration of it not working would go for review repair or patching the problem deeper on the nodejs side crypto if that be the case.
It just seems like the better place to put it at this point to me.
Personally I would have thought what I posted would enable someone familiar with crypto on nodejs side to easily replicate and or even spot the issue.
Maybe OAEP is no longer considered safe. Maybe the browser doesn't like to decrypt something that's not signed or generated with a valid Certificate Authority.
I have a work around for now just thought I'd point out the difficulties validating browser to server is working and provide a test set so it doesn't break in the future.
I noted lots of guesses that don't help out in the wild and even threads closed due to heated debate I thought I would try my best to find a way to once and for all put and end to the confusion but it seems we have reached a limit of average developer intelligence or the ability of the community to find a place to demonstrate a sanctioned official way of how to use the various methods from both sides of the javascript encrypted communication.
NodeJS stomped out the previous crypto with a superior (non-compatible) solution and really no help other then how it works internally. I reached an impasse with a certain method that seemed a bug in node but I am not sure other then something between them seems to fail.

@pedra

This comment has been minimized.

@MasterJames

This comment has been minimized.

@pedra

This comment has been minimized.

@MasterJames

This comment has been minimized.

@tniessen

This comment has been minimized.

@tniessen
Copy link
Member

tniessen commented May 5, 2019

Admittedly, this bug report is not easy to follow, but I took the time anyway. RSA-OAEP is an algorithm, but not a key format. When you export the key as PEM, you lose the algorithm information (the stored algorithm is 1.2.840.113549.1.1.1 which simply means RSA, not RSA-OAEP). The first problem is that you are not passing in padding: crypto.constants.RSA_PKCS1_OAEP_PADDING to publicEncrypt, meaning that publicEncrypt uses a different padding than your WebCrypto app.

The other problem is that publicEncrypt etc. are very old functions with few options and as such, they do not currently support OAEP with SHA-256. SHA-1 should work. I'll try to add support for other hash algorithms shortly.

Please let me know if this helps: Add the correct padding option and change the message digest algorithm to SHA-1.

@tniessen tniessen self-assigned this May 5, 2019
@panva
Copy link
Member

panva commented May 5, 2019

I'll try to add support for other hash algorithms shortly.

Amazing, this would add one of the missing defined JWAs - RSA-OAEP-256

Afterwards, the only remaining would be ECDH with X25519 and X448. All native with no crypto dependencies (key format conversion asn1 js implementation aside)

I can also confirm RSA-OAEP JWA alg works fine. Given you pass the right padding to use and understand it’s using SHA1, that’s obviously part of the JWA definition.

@MasterJames
Copy link
Author

Okay thanks for the time and reply. I apologize for my confusing post. These are complicated problems with the interconnection of disconnected things. I will get to the problem again in some time and report back. I have become distracted by Ubuntu 19.04 resolv with bind issues and also NodeJS after 11.8 are all broken for me likely related to a certain turtle websocket library.
I do need these all working and look forward to understanding and resolving all the issues. It maybe with the new openssl 1.1.1b in Ubuntu 19 and likely some other solution replacement of websocket in node, that all this becomes obsolete with your adding these extensions and other fixes along the data paths involved. Thanks again for helping make the web and therefore the world a better place.

pull bot pushed a commit to SimenB/node that referenced this issue Aug 7, 2019
PR-URL: nodejs#28335
Fixes: nodejs#25756
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@MasterJames
Copy link
Author

Cool. Thanks will check later.

targos pushed a commit that referenced this issue Aug 19, 2019
This adds an oaepHash option to asymmetric encryption which allows
users to specify a hash function when using OAEP padding. This
feature is required for interoperability with WebCrypto applications.

PR-URL: #28335
Fixes: #25756
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
targos pushed a commit that referenced this issue Aug 19, 2019
PR-URL: #28335
Fixes: #25756
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants