-
Notifications
You must be signed in to change notification settings - Fork 839
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
Security Module Plugin API #713
Merged
Merged
Changes from all commits
Commits
Show all changes
104 commits
Select commit
Hold shift + click to select a range
8dff540
Node Key Security Provider BouncyCastle backed internal plugin
usmansaleem 33482d5
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem bf62f2c
wip
usmansaleem 76f01f1
wip - renaming plugin name
usmansaleem 3db6b44
refactoring package name
usmansaleem 72b4dc9
wip
usmansaleem 3a30a73
wip - refactor package name
usmansaleem d5392e7
wip
usmansaleem 160d9df
Merge upstream/master
usmansaleem b934f3a
wip - clean compile
usmansaleem 2505c3d
spotless fix
usmansaleem afd1124
test compilation fix
usmansaleem 87e063f
spotless fix
usmansaleem 977b3f0
license header
usmansaleem 5c3e76a
spotless fix
usmansaleem 8a69d3a
spotless fix
usmansaleem 4d14a37
mock fix in CommandTestAbstract
usmansaleem e044e49
supplier memoize
usmansaleem 4b2209a
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem 94f2996
supplier memoize moved into BouncyCastleSecurityModule
usmansaleem 295caef
Fixing PublicKeySubCommand and its unit tests
usmansaleem 7bd3790
updating plugin cli option and removing unit test cases
usmansaleem 4f08b33
Register SecurityModuleService in dsl ThreadBesuRunner
usmansaleem 3b18e3e
refactoring threadbesunoderunner
usmansaleem a2187a3
variable names cleanup
usmansaleem c133689
cleanup
usmansaleem f7e796c
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem ee607f8
remove memoize from bcsecuritymodule
usmansaleem 77b604d
remove nodeKey instance variable from BesuCommand
usmansaleem 1196a41
Fixing CommandTestAbstract mocks and PublicKeySubCommandTest
usmansaleem cb93117
cleaning up security module name constant
usmansaleem aa6ead8
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem 0774c4e
Using extended Functional interface
usmansaleem a7d0dfe
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem 3630ab8
plugin api changes hash
usmansaleem 92412c4
Ditching BouncyCastle from plugin and package name. Moving to localfile
usmansaleem 2db11f7
exception msg
usmansaleem d3b1cac
unit test for Plugin
usmansaleem e1aea38
header
usmansaleem 26962c6
final
usmansaleem bc45536
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem 7a8fb1d
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem 690618d
using final keyword in nodePrivateKeyFile()
usmansaleem 570271a
Making PublicKey interface;
usmansaleem de52ac4
Plugin API hash
usmansaleem 0575240
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem 8850fe8
tidying up LocalFileSecurityModulePlugin
usmansaleem cce1bcb
review - converting Signature to interface
usmansaleem d83e47f
refactoring Signature and PublicKey in data subpackage
usmansaleem c5f1738
review - refactoring inner class to be top level class. Adding javadoc
usmansaleem 3088be3
refactoring method out of SecurityModuleProvider
usmansaleem c28cc76
spotless fix
usmansaleem 7341cfc
adding unstable annotation in plugin interfaces
usmansaleem eaf709f
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem 1627081
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem 6a27c66
expose buildNodeKey from BesuCommand so that PublicKeySubcommand can …
usmansaleem 7c19a1a
Merge upstream master
usmansaleem 25af953
use functional reference for nodekey in public key subcommand
usmansaleem 2487f0f
unextend SecurityModuleProvider from Functional.
usmansaleem 007b87a
SecurityModuleException
usmansaleem 47ab014
plugin checksum
usmansaleem a6ae3a9
Use Runnable function reference to initialize besu configuration serv…
usmansaleem 4943186
LocalFileSecurityModulePlugin - Use isDocker directly
usmansaleem fc0be7f
review - rename variable to nodeKey
usmansaleem 0a9525a
review - javadoc
usmansaleem e2ff900
fix compilation issues
usmansaleem c643c23
use orElseGet
usmansaleem 30c52d1
settings.gradle ordering
usmansaleem bb7bc22
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem 1ad1c45
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem a9a42f4
Expose ECPoint from PublicKey interface'
usmansaleem 860a411
ECPoint byte[] to BigInteger conversion padding
usmansaleem 91c90c9
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem 2aac744
javadoc
usmansaleem aa53c23
checksum
usmansaleem 715450d
spotless fix
usmansaleem 79b4f31
plugin checksum and final keyword
usmansaleem ee26e5c
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem 4bad3f5
ECPointUtil unsigned x y
usmansaleem 4897a0a
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem 8b6589f
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem 3437e7f
merge upstream master
usmansaleem 24cccad
spotless fix
usmansaleem 39deda8
rename method
usmansaleem c25d921
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem d9fec18
Removing internal plugin and directly instantiating KeyPairSecurityMo…
usmansaleem afbc588
removing internal plugin from acceptance dsl
usmansaleem 6283c0c
javadoc
usmansaleem 26aebd3
final keywords
usmansaleem eb6712a
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem 317bc51
ECPointUtil toUnsignedByteArray cleanup and unit tests
usmansaleem 6f675cb
cleaning up KeyPairSecurityModule
usmansaleem 5dee1ab
typo
usmansaleem 73df123
review suggestion - exception message in keypairsecuritymodule
usmansaleem 5f83f01
ECPointUtil cleanup - Use BC BigIntegers.asUnsignedByteArray
usmansaleem 3f62973
Merge upstream master
usmansaleem 6a36b4e
review suggestions
usmansaleem 75c5803
review suggestion - Register default SecurityModuleProvider with Secu…
usmansaleem b18f39b
spotless fix
usmansaleem b5dba6d
plugin api checksum
usmansaleem 6322e08
Using Supplier<SecurityModule> instead of SecurityModuleSupplier
usmansaleem 3405b17
plugin api checksum
usmansaleem 519370c
--security-module cli option instead of --security-module-provider
usmansaleem a47c113
Merge remote-tracking branch 'upstream/master' into nodekey_plugin
usmansaleem File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,7 @@ | |
import org.hyperledger.besu.cli.DefaultCommandValues; | ||
import org.hyperledger.besu.cli.subcommands.PublicKeySubCommand.AddressSubCommand; | ||
import org.hyperledger.besu.cli.subcommands.PublicKeySubCommand.ExportSubCommand; | ||
import org.hyperledger.besu.crypto.SECP256K1.KeyPair; | ||
import org.hyperledger.besu.crypto.NodeKey; | ||
import org.hyperledger.besu.ethereum.core.Address; | ||
import org.hyperledger.besu.ethereum.core.Util; | ||
|
||
|
@@ -33,6 +33,7 @@ | |
import java.nio.file.Files; | ||
import java.nio.file.Path; | ||
import java.util.Optional; | ||
import java.util.function.Supplier; | ||
|
||
import org.apache.logging.log4j.LogManager; | ||
import org.apache.logging.log4j.Logger; | ||
|
@@ -62,25 +63,29 @@ public class PublicKeySubCommand implements Runnable { | |
private CommandSpec spec; // Picocli injects reference to command spec | ||
|
||
private final PrintStream out; | ||
private final KeyLoader keyLoader; | ||
private final Runnable besuConfigurationService; | ||
private final Supplier<NodeKey> nodeKey; | ||
|
||
public PublicKeySubCommand(final PrintStream out, final KeyLoader keyLoader) { | ||
public PublicKeySubCommand( | ||
final PrintStream out, | ||
final Runnable besuConfigurationService, | ||
final Supplier<NodeKey> nodeKey) { | ||
this.out = out; | ||
this.keyLoader = keyLoader; | ||
this.besuConfigurationService = besuConfigurationService; | ||
this.nodeKey = nodeKey; | ||
} | ||
|
||
@Override | ||
public void run() { | ||
spec.commandLine().usage(out); | ||
} | ||
|
||
private Optional<KeyPair> getKeyPair() { | ||
try { | ||
return Optional.of(keyLoader.load(parentCommand.nodePrivateKeyFile())); | ||
} catch (IOException e) { | ||
LOG.error("An error occurred while trying to read the private key", e); | ||
return Optional.empty(); | ||
} | ||
private void initBesuConfigurationService() { | ||
besuConfigurationService.run(); | ||
} | ||
|
||
private NodeKey getNodeKey() { | ||
return nodeKey.get(); | ||
} | ||
|
||
/** | ||
|
@@ -113,22 +118,24 @@ public void run() { | |
checkNotNull(parentCommand); | ||
checkNotNull(parentCommand.parentCommand); | ||
|
||
parentCommand.getKeyPair().ifPresent(this::outputPublicKey); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was this change because NPE were being encountered? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you mean not using getKeyPair anymore? or the usage of Optional.ofNullable? |
||
parentCommand.initBesuConfigurationService(); | ||
final NodeKey nodeKey = parentCommand.getNodeKey(); | ||
Optional.ofNullable(nodeKey).ifPresent(this::outputPublicKey); | ||
} | ||
|
||
private void outputPublicKey(final KeyPair keyPair) { | ||
private void outputPublicKey(final NodeKey nodeKey) { | ||
// if we have an output file defined, print to it | ||
// otherwise print to standard output. | ||
if (publicKeyExportFile != null) { | ||
final Path path = publicKeyExportFile.toPath(); | ||
|
||
try (final BufferedWriter fileWriter = Files.newBufferedWriter(path, UTF_8)) { | ||
fileWriter.write(keyPair.getPublicKey().toString()); | ||
fileWriter.write(nodeKey.getPublicKey().toString()); | ||
} catch (final IOException e) { | ||
LOG.error("An error occurred while trying to write the public key", e); | ||
} | ||
} else { | ||
parentCommand.out.println(keyPair.getPublicKey().toString()); | ||
parentCommand.out.println(nodeKey.getPublicKey().toString()); | ||
} | ||
} | ||
} | ||
|
@@ -165,11 +172,13 @@ public void run() { | |
checkNotNull(parentCommand); | ||
checkNotNull(parentCommand.parentCommand); | ||
|
||
parentCommand.getKeyPair().ifPresent(this::outputAddress); | ||
parentCommand.initBesuConfigurationService(); | ||
final NodeKey nodeKey = parentCommand.getNodeKey(); | ||
Optional.ofNullable(nodeKey).ifPresent(this::outputAddress); | ||
} | ||
|
||
private void outputAddress(final KeyPair keyPair) { | ||
final Address address = Util.publicKeyToAddress(keyPair.getPublicKey()); | ||
private void outputAddress(final NodeKey nodeKey) { | ||
final Address address = Util.publicKeyToAddress(nodeKey.getPublicKey()); | ||
|
||
// if we have an output file defined, print to it | ||
// otherwise print to standard output. | ||
|
@@ -186,10 +195,4 @@ private void outputAddress(final KeyPair keyPair) { | |
} | ||
} | ||
} | ||
|
||
@FunctionalInterface | ||
public interface KeyLoader { | ||
|
||
KeyPair load(final File keyFile) throws IOException; | ||
} | ||
} |
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should no be any docker specific behaviour
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole file is using docker specific behavior using org/hyperledger/besu/cli/BesuCommand.java:2103
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's garbage code to be deleted. We don't provide the parameter in the docker image since 29c3c14#diff-ebacf6f6ae4ee68078bb16454b23247dL19 so this part of the code is dead since July 2019. It was discussed about removing it but it was not a priority. Now, if it's confusing, it should be a priority IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I have created #785 and will clean it out in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Its a shame the docker code wasn't cleaned out before now - but the removal shouldn't be part of this work.