Skip to content

Commit

Permalink
[FABN-1174] - KeyValue store and cryptosuite changes
Browse files Browse the repository at this point in the history 
This CR contains breaking changes to the client APIs
- CryptoSuite no longer has ephemeral true option, now forces use of existing generateEphmeralKey method
- Cryptosuite now has new createKeyFromRaw option to match generateEphmeralKey method when passing PEM string
- Stores no longer return Promise in constructor, new async initialize method used to create the store

Additional changes include
- Convert worked methods to async/await
- Update API descriptions
- Convert tape unit test to mocha
- Update of all tests to account for breaking API changes (async/await)
- Addition of release noets for v2.0.0

Change-Id: Ib7ee5fc4e5a1e2cc3830ffb1c0c934f602e2bb5a
Signed-off-by: [email protected] <[email protected]>
  • Loading branch information
@nklincoln
nklincoln committed Mar 11, 2019
1 parent f67f186 commit 5869b9a
Show file tree
Hide file tree
Showing 54 changed files with 1,463 additions and 1,323 deletions.
10 changes: 3 additions & 7 deletions build/tasks/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ gulp.task('clean-up', () => {

gulp.task('docker-clean', shell.task([
// stop and remove chaincode docker instances
'docker kill $(docker ps | grep "dev-" | awk \'{print $1}\')',
'docker rm $(docker ps -a | grep "dev-" | awk \'{print $1}\')',
'docker kill $(docker ps -aq)',
'docker rm $(docker ps -aq) -f',

// remove chaincode images so that they get rebuilt during test
'docker rmi $(docker images | grep "^dev-" | awk \'{print $3}\')',
Expand All @@ -103,7 +103,7 @@ gulp.task('docker-clean', shell.task([
'docker-compose -f test/fixtures/docker-compose/docker-compose-tls.yaml -p node down'
], {
verbose: true, // so we can see the docker command output
ignoreErrors: true // kill and rm may fail because the containers may have been cleaned up
ignoreErrors: true // kill, rm, and rmi may fail because the containers may have been cleaned up or not exist
}));

gulp.task('docker-ready', ['docker-clean'], shell.task([
Expand Down Expand Up @@ -294,10 +294,6 @@ gulp.task('run-tape-e2e', ['docker-ready'],

// Typescript
'test/typescript/test.js',

// Perf
'test/integration/perf/orderer.js',
'test/integration/perf/peer.js'
]))
.pipe(tape({
reporter: tapColorize()
Expand Down
13 changes: 6 additions & 7 deletions fabric-ca-client/lib/FabricCAServices.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,12 +198,7 @@ const FabricCAServices = class extends BaseClient {
}
}

let opts;
if (this.getCryptoSuite()._cryptoKeyStore) {
opts = {ephemeral: false};
} else {
opts = {ephemeral: true};
}
const storeKey = this.getCryptoSuite()._cryptoKeyStore ? true : false;

try {
let csr;
Expand All @@ -213,7 +208,11 @@ const FabricCAServices = class extends BaseClient {
csr = req.csr;
} else {
try {
privateKey = await this.getCryptoSuite().generateKey(opts);
if (storeKey) {
privateKey = await this.getCryptoSuite().generateKey();
} else {
privateKey = this.getCryptoSuite().generateEphemeralKey();
}
logger.debug('successfully generated key pairs');
} catch (err) {
throw new Error(util.format('Failed to generate key for enrollment due to error [%s]', err));
Expand Down
18 changes: 10 additions & 8 deletions fabric-ca-client/test/FabricCAServices.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ describe('FabricCAServices', () => {
service = new FabricCAServicesRewire('http://penguin.com', null, 'ca_name', cryptoPrimitives);
clientMock = sinon.createStubInstance(FabricCAClient);
service._fabricCAClient = clientMock;
cryptoPrimitives._cryptoKeyStore = false;
});

it('should throw if missing required argument "request"', async () => {
Expand Down Expand Up @@ -252,7 +253,7 @@ describe('FabricCAServices', () => {

const keyStub = sinon.createStubInstance(ECDSAKey);
keyStub.generateCSR.returns('CN=penguin');
cryptoPrimitives.generateKey.resolves(keyStub);
cryptoPrimitives.generateEphemeralKey.returns(keyStub);

// Take control of the enroll
clientMock.enroll.rejects(new Error('enroll error'));
Expand All @@ -271,6 +272,7 @@ describe('FabricCAServices', () => {
const keyStub = sinon.createStubInstance(ECDSAKey);
keyStub.generateCSR.throws(new Error('CSR error'));
cryptoPrimitives.generateKey.resolves(keyStub);
cryptoPrimitives.generateEphemeralKey.returns(keyStub);

const atts = [{name: 'penguin'}, {name: 'power'}];
const req = {enrollmentID: 'enrollmentID', enrollmentSecret: 'enrollmentSecret', profile: 'profile', attr_reqs: atts};
Expand All @@ -283,7 +285,7 @@ describe('FabricCAServices', () => {
getSubjectCommonNameStub.returns('mr_penguin');
normalizeX509Stub.returns('normal');

cryptoPrimitives.generateKey.rejects(new Error('Key error'));
cryptoPrimitives.generateEphemeralKey.throws(new Error('Key error'));

const atts = [{name: 'penguin'}, {name: 'power'}];
const req = {enrollmentID: 'enrollmentID', enrollmentSecret: 'enrollmentSecret', profile: 'profile', attr_reqs: atts};
Expand Down Expand Up @@ -319,9 +321,8 @@ describe('FabricCAServices', () => {
const req = {enrollmentID: 'enrollmentID', enrollmentSecret: 'enrollmentSecret', profile: 'profile', attr_reqs: atts};
await service.enroll(req);

// Opts should contain false
const callArgs = newSuite.generateKey.getCall(0);
callArgs.args[0].should.deep.equal({ephemeral: false});
// should call generateKey, not generateEphemeral
sinon.assert.called(newSuite.generateKey);

});

Expand All @@ -333,6 +334,7 @@ describe('FabricCAServices', () => {
const keyStub = sinon.createStubInstance(ECDSAKey);
keyStub.generateCSR.returns('CN=penguin');
cryptoPrimitives.generateKey.resolves(keyStub);
cryptoPrimitives._cryptoKeyStore = true;

// Take control of the enroll
clientMock.enroll.resolves({
Expand All @@ -343,9 +345,8 @@ describe('FabricCAServices', () => {
const req = {enrollmentID: 'enrollmentID', enrollmentSecret: 'enrollmentSecret', profile: 'profile', attr_reqs: atts};
await service.enroll(req);

// generateKey should be called with ephmereal set to true
const genKeyArgs = cryptoPrimitives.generateKey.getCall(0);
genKeyArgs.args[0].should.deep.equal({ephemeral: true});
// generateKey should be called
sinon.assert.calledOnce(cryptoPrimitives.generateKey);

// Enrol should be called with test values
sinon.assert.calledOnce(clientMock.enroll);
Expand All @@ -367,6 +368,7 @@ describe('FabricCAServices', () => {
const keyStub = sinon.createStubInstance(ECDSAKey);
keyStub.generateCSR.returns('CN=penguin');
cryptoPrimitives.generateKey.resolves(keyStub);
cryptoPrimitives._cryptoKeyStore = true;

// Take control of the enroll
clientMock.enroll.resolves({
Expand Down
5 changes: 3 additions & 2 deletions fabric-client/lib/BaseClient.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,13 @@ const BaseClient = class {
* This can be overriden with a configuration setting <code>key-value-store</code>, the value of which is the
* full path of a CommonJS module for the alternative implementation.
*
* @async
* @param {Object} options Specific to the implementation, for initializing the instance. For the built-in
* file-based implementation, this requires a single property <code>path</code> to the top-level folder for the store
* @returns {Promise} A Promise for a {@link module:api.KeyValueStore} instance of the KeyValueStore implementation
*/
static newDefaultKeyValueStore(options) {
return sdkUtils.newKeyValueStore(options);
static async newDefaultKeyValueStore(options) {
return await sdkUtils.newKeyValueStore(options);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions fabric-client/lib/Channel.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3704,7 +3704,7 @@ const Channel = class {
* @returns {boolean} A boolean value of true when both the identity and
* the signature are valid, false otherwise.
*/
verifyProposalResponse(proposal_response) {
async verifyProposalResponse(proposal_response) {
logger.debug('verifyProposalResponse - start');
if (!proposal_response) {
throw new Error('Missing proposal response');
Expand All @@ -3730,7 +3730,7 @@ const Channel = class {
logger.debug('verifyProposalResponse - found endorser\'s MSP');

try {
identity = msp.deserializeIdentity(endorsement.endorser, false);
identity = await msp.deserializeIdentity(endorsement.endorser, false);
if (!identity) {
throw new Error('Unable to find the endorser identity');
}
Expand Down
45 changes: 29 additions & 16 deletions fabric-client/lib/Client.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,30 +106,32 @@ const Client = class extends BaseClient {
/**
* Load a common connection profile object or load a JSON file and return a Client object.
*
* @async
* @param {object | string} loadConfig - This may be the config object or a path to the configuration file
* @return {Client} An instance of this class initialized with the network end points.
*/
static loadFromConfig(loadConfig) {
static async loadFromConfig(loadConfig) {
const client = new Client();
client.loadFromConfig(loadConfig);
await client.loadFromConfig(loadConfig);
return client;
}

/**
* Load a common connection profile object or load a JSON file and update this client with
* any values in the config.
*
* @async
* @param {object | string} config - This may be the config object or a path to the configuration file
*/
loadFromConfig(loadConfig) {
async loadFromConfig(loadConfig) {
const additional_network_config = _getNetworkConfig(loadConfig, this);
if (!this._network_config) {
this._network_config = additional_network_config;
} else {
this._network_config.mergeSettings(additional_network_config);
}
if (this._network_config.hasClient()) {
this._setAdminFromConfig();
await this._setAdminFromConfig();
this._setMspidFromConfig();
this._addConnectionOptionsFromConfig();
}
Expand Down Expand Up @@ -889,6 +891,7 @@ const Client = class extends BaseClient {
* Queries the target peer for a list of {@link Peer} objects of all peers
* known by the target peer.
*
* @async
* @param {PeerQueryRequest} request - The request parameters.
* @returns {PeerQueryResponse} The list of peer information
*/
Expand Down Expand Up @@ -941,6 +944,7 @@ const Client = class extends BaseClient {
* Queries the target peer for the names of all the channels that a
* peer has joined.
*
* @async
* @param {Peer} peer - The target peer to send the query
* @param {boolean} useAdmin - Optional. Indicates that the admin credentials
* should be used in making this call to the peer. An administrative
Expand Down Expand Up @@ -1015,6 +1019,7 @@ const Client = class extends BaseClient {
/**
* Queries the installed chaincodes on a peer.
*
* @async
* @param {Peer} peer - The target peer
* @param {boolean} useAdmin - Optional. Indicates that the admin credentials
* should be used in making this call to the peer. An administrative
Expand Down Expand Up @@ -1124,6 +1129,7 @@ const Client = class extends BaseClient {
* performed on a peer-by-peer basis. Only the peer organization's ADMIN
* identities are allowed to perform this operation.
*
* @async
* @deprecated
* @param {Deprecated_ChaincodeInstallRequest} request - The request object
* @param {Number} timeout - A number indicating milliseconds to wait on the
Expand Down Expand Up @@ -1228,8 +1234,6 @@ const Client = class extends BaseClient {
* from the common connection profile along with the system configuration to build
* instances of the stores and assign them to this client and the crypto suites
* if needed.
*
* @returns {Promise} - A promise to build a key value store and crypto store.
*/
async initCredentialStores() {
if (!this._network_config) {
Expand All @@ -1241,9 +1245,9 @@ const Client = class extends BaseClient {
this.setStateStore(key_value_store);
const crypto_suite = BaseClient.newCryptoSuite();
// all crypto suites should extends api.CryptoSuite
crypto_suite.setCryptoKeyStore(BaseClient.newCryptoKeyStore(client_config.credentialStore.cryptoStore));
const cryptoStore = BaseClient.newCryptoKeyStore(client_config.credentialStore.cryptoStore);
crypto_suite.setCryptoKeyStore(cryptoStore);
this.setCryptoSuite(crypto_suite);
return true;
} else {
throw new Error('No credentialStore settings found');
}
Expand Down Expand Up @@ -1305,11 +1309,13 @@ const Client = class extends BaseClient {
* Set the admin signing identity object. This method will only assign a
* signing identity for use by this client instance and will not persist
* the identity.
*
* @async
* @param {string} private_key - the private key PEM string
* @param {string} certificate the PEM-encoded string of certificate
* @param {string} mspid The Member Service Provider id for the local signing identity
*/
setAdminSigningIdentity(private_key, certificate, mspid) {
async setAdminSigningIdentity(private_key, certificate, mspid) {
logger.debug('setAdminSigningIdentity - start mspid:%s', mspid);
if (typeof private_key === 'undefined' || private_key === null || private_key === '') {
throw new Error('Invalid parameter. Must have a valid private key.');
Expand All @@ -1324,8 +1330,8 @@ const Client = class extends BaseClient {
if (!crypto_suite) {
crypto_suite = BaseClient.newCryptoSuite();
}
const key = crypto_suite.importKey(private_key, {ephemeral: true});
const public_key = crypto_suite.importKey(certificate, {ephemeral: true});
const key = await crypto_suite.createKeyFromRaw(private_key);
const public_key = await crypto_suite.createKeyFromRaw(certificate);

this._adminSigningIdentity = new SigningIdentity(certificate, public_key, mspid, crypto_suite, new Signer(crypto_suite, key));
}
Expand All @@ -1336,7 +1342,7 @@ const Client = class extends BaseClient {
* be must loaded that defines an organization for this client and have an
* admin credentials defined.
*/
_setAdminFromConfig() {
async _setAdminFromConfig() {
let admin_key, admin_cert, mspid = null;
if (!this._network_config) {
throw new Error('No common connection profile has been loaded');
Expand All @@ -1353,7 +1359,7 @@ const Client = class extends BaseClient {
}
// if we found all we need then set the admin
if (admin_key && admin_cert && mspid) {
this.setAdminSigningIdentity(admin_key, admin_cert, mspid);
await this.setAdminSigningIdentity(admin_key, admin_cert, mspid);
}
}

Expand Down Expand Up @@ -1397,6 +1403,7 @@ const Client = class extends BaseClient {
* and the organization in the client section of the common connection profile
* settings.
*
* @async
* @param {Object} opts - contains
* - username [required] - username of the user
* - password [optional] - password of the user
Expand Down Expand Up @@ -1463,6 +1470,7 @@ const Client = class extends BaseClient {
/**
* Persist the current <code>userContext</code> to the key value store.
*
* @async
* @returns {Promise} A Promise for the userContext object upon successful persistence
*/
async saveUserToStateStore() {
Expand Down Expand Up @@ -1508,6 +1516,7 @@ const Client = class extends BaseClient {
* has been set on the Client instance. If no state store has been set, this cache will not be established
* and the application is responsible for setting the user context again if the application crashes and is recovered.
*
* @async
* @param {User | UserNamePasswordObject} user - An instance of the User class encapsulating the authenticated
* user’s signing materials (private key and enrollment certificate).
* The parameter may also be a {@link UserNamePasswordObject} that contains the username
Expand Down Expand Up @@ -1553,6 +1562,7 @@ const Client = class extends BaseClient {
* (via the KeyValueStore interface). The loaded user object must represent an enrolled user with a valid
* enrollment certificate signed by a trusted CA (such as the CA server).
*
* @async
* @param {string} name - Optional. If not specified, will only return the current in-memory user context object, or null
* if none has been set. If "name" is specified, will also attempt to load it from the state store
* if search in memory failed.
Expand Down Expand Up @@ -1609,6 +1619,7 @@ const Client = class extends BaseClient {
/**
* Restore the state of the {@link User} by the given name from the key value store (if found). If not found, return null.
*
* @async
* @param {string} name - Name of the user
* @returns {Promise} A Promise for a {User} object upon successful restore, or if the user by the name
* does not exist in the state store, returns null without rejecting the promise
Expand Down Expand Up @@ -1671,6 +1682,7 @@ const Client = class extends BaseClient {
* Note that upon successful creation of the new user object, it is set to
* the client instance as the current <code>userContext</code>.
*
* @async
* @param {UserOpts} opts - Essential information about the user
* @returns {Promise} Promise for the user object.
*/
Expand Down Expand Up @@ -1699,7 +1711,8 @@ const Client = class extends BaseClient {
if (this.getCryptoSuite() === null) {
logger.debug('cryptoSuite is null, creating default cryptoSuite and cryptoKeyStore');
this.setCryptoSuite(sdkUtils.newCryptoSuite());
this.getCryptoSuite().setCryptoKeyStore(Client.newCryptoKeyStore()); // This is impossible
const cryptoStore = Client.newCryptoKeyStore();
this.getCryptoSuite().setCryptoKeyStore(cryptoStore);
} else {
if (this.getCryptoSuite()._cryptoKeyStore) {
logger.debug('cryptoSuite has a cryptoKeyStore');
Expand Down Expand Up @@ -1727,9 +1740,9 @@ const Client = class extends BaseClient {
if (privateKeyPEM) {
logger.debug('then privateKeyPEM data');
if (opts.skipPersistence) {
importedKey = await this.getCryptoSuite().importKey(privateKeyPEM.toString(), {ephemeral: true});
importedKey = await this.getCryptoSuite().createKeyFromRaw(privateKeyPEM.toString());
} else {
importedKey = await this.getCryptoSuite().importKey(privateKeyPEM.toString(), {ephemeral: !this.getCryptoSuite()._cryptoKeyStore});
importedKey = await this.getCryptoSuite().importKey(privateKeyPEM.toString());
}
} else {
importedKey = opts.cryptoContent.privateKeyObj;
Expand Down
Loading

0 comments on commit 5869b9a

Please sign in to comment.