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

Dormant: Implement PGP encrypt and PGP decrypt operations #89

Closed
wants to merge 36 commits into from

Conversation

tlwr
Copy link
Contributor

@tlwr tlwr commented Mar 7, 2017

This is for #6, implemented using Async and OpenPGP.js.

It adds two operations:

  • PGP encrypt [Public key]
  • PGP decrypt [Private key, [Private key password]]
    And 6 tests

The test file includes 4 key pairs which were generated for the sole purpose of testing CyberChef's PGP operations.

  • 1024 with password and without password
  • 4096 with password and without password (These keys and associated tests are commented out to make tests run faster)

(A project for the future would be test skipping so we don't have to comment out long running tests).

Screenshots (4096 RSA):
image
image

I added a rather hacky setTimeout before app.chef.bake (in HTMLApp.js) so the UI has time to update the baking status. I think it is sufficiently hacky that a less bodged solution should be found in the future but for now it seems to suffice.

Copy link
Member

@n1474335 n1474335 left a comment

Choose a reason for hiding this comment

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

This looks really useful. One general comment:

The openpgp.js library is quite large (321KB even once minified), so if we're going to include it and make everyone download it, we need to make sure it's pulling its weight. Can we create more operations using its capabilities, for example signing, verifying, creating new keys and anything else it supports?

@@ -92,6 +92,8 @@ var Categories = [
"Substitute",
"Derive PBKDF2 key",
"Derive EVP key",
"PGP Encrypt",
"PGP Decrypt",
Copy link
Member

Choose a reason for hiding this comment

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

Add these to the 'Public Key' category as well.

},
{
name: "Private key password",
type: "text",
Copy link
Member

Choose a reason for hiding this comment

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

I think this would suit the string type better. It will then display a single-line input box instead of a full textarea.

* Encrypts the input using PGP.
*
* @param {string} input - plaintext to encrypt
* @param {function} args
Copy link
Member

Choose a reason for hiding this comment

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

The type here should be Object [].

// Timeout is so that UI can update before openpgp blocks thread
openpgp.encrypt(options)
.then(function(ciphertext) {
console.log(ciphertext);
Copy link
Member

Choose a reason for hiding this comment

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

Extraneous console.log should be removed.

reject("Failed to read public key", error);
}

// Timeout is so that UI can update before openpgp blocks thread
Copy link
Member

Choose a reason for hiding this comment

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

This comment feels out of place. Either include more context or put it where the timeout actually takes place.

* Decrypts the input using PGP.
*
* @param {string} input - ciphertext to decrypt
* @param {function} args
Copy link
Member

Choose a reason for hiding this comment

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

Type should be Object [].

});
// This timeout is so the UI has time to update the baking status
// before any blocking operations delay it until after the operation.
setTimeout(function() {
Copy link
Member

Choose a reason for hiding this comment

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

I understand why you've done this, but I really don't like the idea of adding 10ms of latency to every single bake. It's possible we could just reduce the timeout to 0ms and it would still allow the UI to update due to timeouts allowing the execution of anything left in the processing queue first (see this SO comment). However this is still not ideal. I suspect the answer is web workers, which we've discussed before. If the 0ms trick works, I can accept this for now and we'll engineer a more robust solution in the future. If the 0ms trick does not work, we'll have to think of a different solution before this is merged into master.

@tlwr tlwr changed the title Implement PGP encrypt and PGP decrypt operations WIP: Implement PGP encrypt and PGP decrypt operations Mar 8, 2017
@tlwr
Copy link
Contributor Author

tlwr commented Mar 9, 2017

Thanks for catching all the careless errors above, I will be more scrupulous from now on.

I completely agree about the setTimeout... and will endeavour to find a better (if temporary) solution (i.e. without resorting to web workers, I think we have enough going on at present).

I hadn't noticed the size of OpenPGP.js but I agree it would be a waste to not include all possible functionality if we are "paying" for it anyway. I had some questions about the new functionality before I pushed my changes:

1. Output format of "PGP Verify".

I was going to go with the following (when verification succeeds)

Verified: true

Key ID: a9510d8fd7e352f5

message-goes-here

and (when it fails)

Verified: false

Key ID:

message-goes-here

This seems consistent with other text producing operations: e.g. Extract email addresses (displaying a total as the first line and then the results afterwards).

2. Exclusion of methods for looking up and uploading to key server.

I am under the impression that we do not want (at present) to include any operations involving network connectivity (for security, portability, reproducibility, etc).

I think if what you are alluding to in #79 gets implemented then perhaps a module system or similar could be built and there could be a "networked operations" module which would include such things. Right now I think it is best left for another day.

3. Input and output format of "PGP Generate".

For input I am currently using the arguments:

Password: string,
Key size: [1024, 2048, 4096],
Name: string,
Email: string.

For output I have currently implemented $publicKey\n\n$privateKey:

-----BEGIN PGP PUBLIC KEY BLOCK-----
Version: OpenPGP.js v2.3.6
Comment: http://openpgpjs.org

etc
-----END PGP PUBLIC KEY BLOCK-----

-----BEGIN PGP PRIVATE KEY BLOCK-----
Version: OpenPGP.js v2.3.6
Comment: http://openpgpjs.org

etc
-----END PGP PRIVATE KEY BLOCK-----

This operation is quite odd because it completely ignores any input and seems to just be providing a convenience (which is fine). I wondered if you had any insight in if it should take arguments. E.g. Password is input.

Further at-length discussion of this topic may lead into the "pipe output as argument" rabbit-hole which this PR is probably not the place for.

4. Non-Armored operations.

Currently all operations deal with Armored-strings instead of bytes, which (in my limited experience), seems to be the preferred format for all things PGP.

Instead of having multiple operations (for armored output and raw output) I was going to attempt to create two operations:

  • PGP Remove ASCII Armor (string -> byteArray)
  • PGP Add ASCII Armor (byteArray -> string)
    So that operations can all have armored input/output and if the raw bytes are necessary then an operation can be chained to add/remove the armor:
[PGP Encrypt]
[PGP Remove ASCII Armor]

And

[PGP Add ASCII Armor]
[PGP Decrypt]

Although I think the jury is out on whether people actually do use non-Armored, you and your colleagues probably know more than I do.

There is probably an additional debate whether to use "Armored" or "Armoured" too :)

@n1474335
Copy link
Member

n1474335 commented Mar 9, 2017

I do think Web Workers are a good idea, but like you say, there are a number of big changes going through at the moment so let's wait until we're a bit more stable before we introduce another major architectural redesign.

In answer to your questions:

1. Output format for "PGP Verify".

Are you suggesting that this operation will also decrypt the message? I would personally leave that to the "PGP Decrypt" op and just use this one to list something along these lines:

Verified: [true or false]
Signed on [date]
Signed by [signer]
Signed with [algorithm]

If it's not valid, you could display the hash that actually gets produced as well as the above information.

2. Exclusion of methods for looking up and uploading to key server.

Yes, you're absolutely correct that we don't want anything like this at the moment. Perhaps at some point in the future, but for now we can leave it out.

3. Input and output format of "PGP Generate".

The arguments and output you have suggested look ideal. There is currently one other operation that also completely ignores the input: Generate UUID. I'm fine with this model if it makes sense, which I think it does in this case. It would be more confusing to the user to expect them to know to enter the password in the input or have to read the description to find out how to use the operation.

Piping output into arguments is another thing that we'll tackle at some point. It's definitely a really good idea, but we'll need to think carefully about the best way to design it. There is an issue for it here: #26.

4. Non-Armored operations.

Armoured strings should be the default but, as you've suggested, it might be nice to offer the ability to change the input and output formats to also support raw byte arrays and hex. I would prefer that this be an option argument for any relevant operations as well as a pair of separate operations. There is precedent for this. The Parse X.509 certificate operation includes the option to specify the output format, but there are also separate operations to convert PEM to Hex and Hex to PEM. I think this model works well as it offers the freedom to play around with the formats, but keeps things simple if someone wants to just modify the settings on a single operation.

Regarding the spelling, it doesn't bother me a huge amount, but considering that the rest of CyberChef uses British English spelling (see "Favourites" and "Parse colour code" for example), it makes sense to stick to that convention and use "Armour". Perhaps at some point we can offer language packs ;)

Added:
+ PGP Sign cleartext
+ PGP Verify cleartext
+ PGP Add ASCII Armor
+ PGP Remove ASCII Armor
+ Many tests for all operations

Updated:
+ PGP Encrypt (formatting of error messages)
+ PGP Decrypt (^^)
+ PGP Sign (this operation is now exclusively for non-clearsigned)
+ PGP Verify (^^)
Rarely, if ever, used. It is now trivial to generate and test in the
actual web app.
@tlwr
Copy link
Contributor Author

tlwr commented Mar 11, 2017

Just updating this with my progress after the most recent commit (still a WIP).

I separated out Sign / Sign Cleartext and Verify / Verify Cleartext because I think they are sufficiently different both in terms of the function they provide and from a usability perspective.

I've operations are currently as follows:

PGP Encrypt

Arguments:

  • Plaintext input
  • Recipient's public key

Outputs: ----- BEGIN PGP MESSAGE -----...

PGP Decrypt

Arguments:

  • Encrypted input (----- BEGIN PGP MESSAGE -----...)
  • Recipient's private key
  • [Recipient's private key password]

Outputs: plaintext

PGP Sign

Arguments:

  • Plaintext input
  • Recipient's public key
  • Author's private key
  • [Author's private key password]

Outputs: ----- BEGIN PGP MESSAGE -----...

PGP Verify

Arguments:

  • Encrypted input (----- BEGIN PGP MESSAGE -----)
  • Author's public key
  • Recipient's private key
  • [Recipient's private key password]
  • Display message in output (checkbox)

Outputs:

Verified: true
Key ID: a9510d8fd7e352f5
Encrypted for: CyberChef nopw 1024 <toby@toby.codes>
Signed on: // currently unimplemented
Signed by: CyberChef nopw 1024 <toby@toby.codes>
Signed with: // currently unimplemented


$message-goes-here-if-displayed

PGP Sign Cleartext

Arguments:

  • Plaintext input
  • Author's private key
  • [Author's pirvate key password]

Outputs:


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

$message-goes-here
-----BEGIN PGP SIGNATURE-----
Version: OpenPGP.js v2.3.6
Comment: http://openpgpjs.org

$pgp-bytes
-----END PGP SIGNATURE-----

PGP Verify Cleartext

Arguments:

  • Clearsigned input (----- BEGIN PGP SIGNED MESSAGE -----...----- BEGIN PGP SIGNATURE -----...)
  • Author's public key
  • Display message in output (checkbox)

Outputs:

Verified: true
Key ID: a9510d8fd7e352f5
Signed on: // currently unimplemented
Signed by: CyberChef nopw 1024 <toby@toby.codes>
Signed with: // currently unimplemented


$message-goes-here-if-displayed

PGP Generate Key Pair

Unchanged from above

PGP Add ASCII Armor

Arguments:

  • byteString input (PGP bytes)
  • Armor type ("Message", "Public key", "Private key")

Outputs:
The armored message, so one of the following:

  • ----- BEGIN PGP PUBLIC KEY -----
  • ----- BEGIN PGP PRIVATE KEY -----
  • ----- BEGIN PGP MESSAGE -----

Currently clearsigned messages don't work (see PGP Remove ASCII Armor).

PGP Remove ASCII Armor

Arguments:

  • PGP Armored input

The PGP Armored input can currently be anything in PGP Armor:

  • Public key
  • Private key
  • Message
  • Clearsigned message (this will only output the PGP signature bytes [essentially a detached signature] and will not contain the message text).

Outputs: byteString of the raw PGP bytes.

Detached signatures

Currently detached signatures are not supported. I'm not sure how they would fit in with the current operations:

  1. Optional arguments for "PGP Verify Cleartext" (an input box for the detached signature to verify) and an additional boolean argument for different formatting for "PGP Sign Cleartext", for example:
Output:
$message

Detached signature: 
-----BEGIN PGP SIGNATURE-----...
  1. New operation "PGP Sign Detached" with HTML files output: "message", "message.asc" (ASCII Armor), "message.sig" (raw bytes), and another operation "PGP Verify Detached".

Example HTML output:
image

I'm leaning towards the latter even though we have so many operations already. It is a better representation of the "detached nature" of detached signatures. If I was to implement this I would add a download link for each of the files (which I think should be done anyway).

This method has the advantage operation having an "opposite number" as it were:

  • PGP Encrypt <=> PGP Decrypt
  • PGP Sign <=> PGP Decrypt
  • PGP Sign Cleartext <=> PGP Verify Cleartext
  • PGP Sign Detached <=> PGP Verify Detached

And all the operations above would have no optional arguments that may be confusing.

Summary of todo list

  • Ensure ASCII Armor operations for cleartext signatures
  • Tests for above
  • Renaming of "Armor" -> "Armour"
  • Possibly enable output of raw bytes (from operations) depending on how well ASCII Armor operations go
  • Gauge if implementation of "Signed on" and "Signed with" in "PGP Verify" and "PGP Verify Cleartext" is doable (i.e. is it implemented in OpenPGP.js), if so, implement it.
  • Improve operation descriptions
  • Detached signatures implementation

Copy link
Member

@n1474335 n1474335 left a comment

Choose a reason for hiding this comment

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

Progress is looking really good. My previous comments about not displaying the decrypted message when you run Verify can probably be ignored. It's clearer now that Sign and Verify are just authenticated versions of Encrypt and Decrypt respectively. I was thinking of them more as helper functions that you could use on top of Encrypt and Decrypt.

Nearly there!

"PGP Verify Cleartext",
"PGP Generate Key Pair",
"PGP Add ASCII Armor",
"PGP Remove ASCII Armor",
Copy link
Member

Choose a reason for hiding this comment

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

Change these to 'Sign PGP Cleartext', 'Verify PGP Cleartext', 'Generate PGP Key Pair', 'Add PGP ASCII Armour' and 'Remove PGP ASCII Armour'.

*
* @param {string} input - signed input to verify
* @param {Object[]} args
* @returns {string} - "true" or "false" depending on the validity of the signature
Copy link
Member

Choose a reason for hiding this comment

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

This now returns more than just "true" or "false"

*
* @param {string} input - signed input to verify
* @param {Object[]} args
* @returns {string} - "true" or "false" depending on the validity of the signature
Copy link
Member

Choose a reason for hiding this comment

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

This also returns more than "true" or "false"

@tlwr
Copy link
Contributor Author

tlwr commented Mar 20, 2017

Progress update

image

image

Because PGP clearsigs are so different from other ASCII operations I have created a separate operation for detaching clear signatures: "Detach PGP Cleartext".

Still on the to-do list

  • Operation descriptions
  • JSDoc
  • setTimeout hackiness in HTMLApp

@n1474335
Copy link
Member

I think it's time to merge this into feature-async-ops. The only outstanding thing now is the setTimeout hack and the correct way to handle that is by using web workers which I'd like to work on myself.

Next steps:

  1. Update this PR to work with the new ES6 imports/exports
  2. Merge it into feature-async-ops
  3. @n1474335 to work on web workers on feature-async-ops branch
  4. Merge feature-async-ops into master
  5. Increment major version

@tlwr
Copy link
Contributor Author

tlwr commented Mar 31, 2017

Okay, sounds good. When I fetch and merge I'll remove the setTimeout. Most likely I will get this done on 04/01 or 04/02.

@tlwr
Copy link
Contributor Author

tlwr commented Apr 8, 2017

Just an update on where I am right now. I started working on merging upstream changes (node/webpack/babel) a while ago although I haven't had much time.

I have run into a couple of issues:

  • One of the flow control tests seems to be causing me some trouble (can someone else have a look, does it terminate or give you any OOM errors?)
  • Using import "openpgp" or import * as openpgp from "openpgp"; (Or var openpgp = require("openpgp");) causes the following code to be generated in the build:
/***/ }),
/* 15 */
/***/ (function(module, exports, __webpack_require__) {

var __WEBPACK_AMD_DEFINE_RESULT__;!(__WEBPACK_AMD_DEFINE_RESULT__ = function() {
    "use strict";

    return window.document;
}.call(exports, __webpack_require__, exports, module),
                __WEBPACK_AMD_DEFINE_RESULT__ !== undefined && (module.exports = __WEBPACK_AMD_DEFINE_RESULT__));

And the window.document then causes problems (running the tests under node which doesn't have a window global).

If anyone has encountered this before (or similar situations) I'd love to know how you resolved it.

Additionally thanks to @graingert I have realised that a lot of the code involving promises that I have written is subpar/redundant. I will rewrite that code after merging the upstream node changes, then we can get this merged (finally).

@n1474335 sorry if this is holding you up!

@n1474335
Copy link
Member

n1474335 commented Apr 9, 2017

Hey @tlwr, cheers for the update. Don't worry about holding anything up, I have a long list of other things to work on in the meantime!

If you commit what you've got so far, I'll see if I can fix your tests. The import problems can potentially be fixed using the imports-loader, although this is clearly a workaround rather than a long-term solution. The OpenPGP.js library does claim that it supports Node, so there must be a way of doing this correctly.

@tlwr
Copy link
Contributor Author

tlwr commented Apr 12, 2017

Committed where I got up to with integrating my work with your work on node/babel/webpack.

I've also taken the liberty of rewriting my old code using promises to use best practices. More suboptimal stuff remains but as it is not related to PGP it is probably outside the purview of this PR.

I did see the imports-loader solution but I am surprised if it is necessary. I'm quite sure it is a mistake on my part.

return ciphertext.data;
})
.catch(function(err) {
throw "Could not encrypt text: " + err;
Copy link
Contributor

Choose a reason for hiding this comment

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

It's best to keep the original exception so that we don't lose the stacktrace

.catch(function(err) {
console.error("Chef's promise was rejected, this should never occur");
});
}, 10);
Copy link
Contributor

Choose a reason for hiding this comment

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

This set timeout will throw away any exceptions, which should be caught at 155

Surely it would be better to explicitly wait for the ui to update?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe @n1474335 doesn't plan on keeping this in.

There is some discussion above about this problem. A possible solution is web-workers which @n1474335 wants to work on at a later date, I think your input could be quite valuable here (there is almost certainly something that we have missed).

The catch statement referenced here should never be called. The throwing away of the stack trace occurs in Recipe.js. Recipe.js definitely needs some TLC (on my todo) (both on my bad promises code front, and operationally (manging of stack traces into CyberChef UI focused error messages)).

It is probably prudent, now that the UI is being decoupled from the Chef/Recipe/Operation logic, that we change how we handle errors and stack traces (to work better with node).

I.e. we preserve a stack trace (and operation metadata) and handle the error message formatting (from the actual trace) in App.js

@@ -54,6 +54,11 @@ import Chef from "../src/core/Chef.js";
output: null,
};

// Remove whitespace helper
var noWS = function(string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Best to avoid the var keyword.

Instead of a comment use:

function removeWhitespace(string) {
 ... 
} 


var CYBERCHEF_GENERATED_KEY_PAIRS = [
{
name: "CyberChef 1",
Copy link
Contributor

Choose a reason for hiding this comment

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

You can put all these in a .json file and import it.

Also it's that var again, use const

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test runner (as-is) is very hacky. It was originally built when we could only run operations in the browser. There was an entire process involving node calling phantomJS running the actual CyberChef operations, relying upon stdout and stderr piped from phantomJS to node and the test runner reporting results correctly. As expected this was a bodge and very fragile at best. (It has moved on a bit since then but is still quite bad)

It will be a good day when we remove all of our home-grown test running infrastructure and use proper test runner. This is now feasible because of @n1474335's work with node. Now we can do something like:

import * as CyberChef from "CyberChef";

CyberChef.bake("input", recipeConfig)
.then(x => console.log(x));

Instead of the TestRegister that is inflicted upon us currently.

The future of this file (once the test runner is refactored) is not storing key pairs in a JSON file. Generating key pairs is an operation that needs to be tested anyway. We should just generate key pairs and use those for other tests.

Again (as above) this is another area where your JS knowledge could prove useful: picking and implementing a test runner from one of the many available in the ecosystem.

[PGP_TEST_KEY_PAIRS[0], PGP_TEST_KEY_PAIRS[1]],
[PGP_TEST_KEY_PAIRS[1], PGP_TEST_KEY_PAIRS[0]],
].forEach(function(pairOfKeyPairs) {
var alice = pairOfKeyPairs[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

Use const


return openpgp.encrypt(options)
.then(function(ciphertext) {
return ciphertext.data;
Copy link
Contributor

Choose a reason for hiding this comment

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

Use arrows: .then(c => c.data)

@graingert
Copy link
Contributor

You should rebase and clean up your commits

@tlwr
Copy link
Contributor Author

tlwr commented Apr 13, 2017

I've cleaned up Recipe.js by using async/await. I'd love a second look though.

I also had to increment the ecmaVersion config param in eslint so we can use async function.

Still banging my head against openpgp + webpack. It definitely feels like something trivial that I am overlooking.

@n1474335
Copy link
Member

@tlwr

I've just spent about 5 hours trying to sort out this bug. I've got to the stage where it successfully compiles, but the test suite then hangs, even if I comment out all the PGP tests. It could be something to do with the changes you've made to Recipe.js, although it seems to work fine in the browser... Either way, I haven't investigated any further. I don't really want to push my "fix" as it isn't actually complete yet. I'll explain the steps I've gone through in the hopes it will help someone else fix it properly.

  1. The cause of the return window.document; bug is jQuery. When I set up webpack in ES6, webpack, Babel, libraries imported through npm, and NodeJS version #95 I included a resolve alias for jquery (jquery: "jquery/src/jquery") so that it would import the uncompiled version. This is recommended as it means webpack can more easily deduplicate common modules elsewhere in the project. However, if you remove this resolve alias, it no longer loads the offending code (node_modules/jquery/src/var/document.js) as its own module, so it doesn't get executed. I still haven't worked out quite why it's only causing a problem now when it has been there ever since I pushed ES6, webpack, Babel, libraries imported through npm, and NodeJS version #95.

  2. Once this bug is fixed, you'll hit another one complaining that it can't load the crypto module. This is caused by webpack overriding the require keyword using variable injection when it inlines the openpgp module. When OpenPGP detects that it's running in a node environment rather than a browser, it tries to load native modules like crypto using require and fails because require is now undefined. This is what really stumps me. I can't work out why webpack is overriding require and I can't find a way to stop it. When the webpack target is set to node, it should allow modules to use require. The only solution I have is to import the source version of OpenPGP rather than the compiled distribution version (ironically the exact opposite of the solution/workaround for the previous bug). The source version is all written using ES6 modules and doesn't include the stub at the top which tries to detect which module system you're using (CommonJS, AMD, ES6 etc.) and then load your modules for you. With this stub missing, it loads successfully. You will need to npm install --save asmcrypto-lite rusha es6-promise for the uncompiled version to work.

So in short:

Gruntfile.js
                 resolve: {
                     alias: {
-                        jquery: "jquery/src/jquery"
+                        openpgp: "openpgp/src/index"
                     }
                 },

package.json
   "dependencies": {
+    "asmcrypto-lite": "^1.1.0",
+    "es6-promise": "^4.1.0",
+    "rusha": "^0.8.5",

As I said, once this is done the tests will still hang, so there's clearly something else wrong as well but I think I'll call it a night.

@graingert
Copy link
Contributor

graingert commented Apr 16, 2017 via email

@n1474335
Copy link
Member

n1474335 commented Apr 16, 2017

@graingert Unfortunately that doesn't seem to work, webpack still overrides require for the openpgp module.

/* 604 */
/***/ (function(module, exports, __webpack_require__) {

/* WEBPACK VAR INJECTION */(function($) {var require;var require;(function(f)...

Where (function(f)... is the beginning of openpgp.js.

@graingert
Copy link
Contributor

graingert commented Apr 16, 2017 via email

@graingert
Copy link
Contributor

graingert commented Apr 16, 2017 via email

@tlwr
Copy link
Contributor Author

tlwr commented Apr 16, 2017

@n1474335 your effort sounds pretty heroic. I've implemented your suggested changes and got the same results as you: progress!

I'm curious what node --version you are running. As I understand it, webpack resolve aliases aren't touched by plugins (babel) which causes syntax errors due to new ES syntax (in OpenPGP.js), which does defeat the point of transpiling. I am on v4.2.6. Looking into how to solve this now.

Also if you comment out the FlowControl tests do the tests still hang? (I believe Jump: skips negatively is the specific test).

@tlwr tlwr changed the title WIP: Implement PGP encrypt and PGP decrypt operations Dormant: Implement PGP encrypt and PGP decrypt operations Apr 21, 2017
@tlwr
Copy link
Contributor Author

tlwr commented Apr 21, 2017

Closing, will reopen with a cleaned up and separate PR (just for the PGP operations) once #117 is merged.

@tlwr tlwr closed this Apr 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants