From 0cd2eaae1a82659334ff02e80ae76ce0e4d39c36 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Mon, 4 Oct 2021 14:02:26 -0700 Subject: [PATCH 1/2] Remove ICryptoUtils and CryptoUtils --- src/client/common/crypto.ts | 43 ------ src/client/common/platform/fileSystem.ts | 2 - src/client/common/serviceRegistry.ts | 3 - src/client/common/types.ts | 19 --- src/test/common/crypto.unit.test.ts | 143 ------------------- src/test/common/installer.test.ts | 3 - src/test/common/moduleInstaller.test.ts | 3 - src/test/common/serviceRegistry.unit.test.ts | 3 - 8 files changed, 219 deletions(-) delete mode 100644 src/client/common/crypto.ts delete mode 100644 src/test/common/crypto.unit.test.ts diff --git a/src/client/common/crypto.ts b/src/client/common/crypto.ts deleted file mode 100644 index e592566f006b..000000000000 --- a/src/client/common/crypto.ts +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { createHash } from 'crypto'; -import { injectable } from 'inversify'; -import { traceError } from './logger'; -import { ICryptoUtils, IHashFormat } from './types'; - -/** - * Implements tools related to cryptography - */ -@injectable() -export class CryptoUtils implements ICryptoUtils { - // eslint-disable-next-line class-methods-use-this - public createHash( - data: string, - hashFormat: E, - algorithm: 'SHA512' | 'SHA256' | 'FNV' = 'FNV', - ): IHashFormat[E] { - let hash: string; - if (algorithm === 'FNV') { - // eslint-disable-next-line global-require - const fnv = require('@enonic/fnv-plus'); - hash = fnv.fast1a32hex(data) as string; - } else if (algorithm === 'SHA256') { - hash = createHash('sha256').update(data).digest('hex'); // NOSONAR - } else { - hash = createHash('sha512').update(data).digest('hex'); - } - if (hashFormat === 'number') { - const result = parseInt(hash, 16); - if (Number.isNaN(result)) { - traceError(`Number hash for data '${data}' is NaN`); - } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return result as any; - } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return hash as any; - } -} diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index f06260e2f723..64d6fffcf3f0 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -471,8 +471,6 @@ export class FileSystemUtils implements IFileSystemUtils { } } -// We *could* use ICryptoUtils, but it's a bit overkill, issue tracked -// in https://github.com/microsoft/vscode-python/issues/8438. export function getHashString(data: string): string { const hash = createHash('sha512'); hash.update(data); diff --git a/src/client/common/serviceRegistry.ts b/src/client/common/serviceRegistry.ts index ec5fbdebfde8..f85eab8afa68 100644 --- a/src/client/common/serviceRegistry.ts +++ b/src/client/common/serviceRegistry.ts @@ -5,7 +5,6 @@ import { IAsyncDisposableRegistry, IBrowserService, IConfigurationService, - ICryptoUtils, ICurrentProcess, IEditorUtils, IExperimentService, @@ -59,7 +58,6 @@ import { WorkspaceService } from './application/workspace'; import { AsyncDisposableRegistry } from './asyncDisposableRegistry'; import { ConfigurationService } from './configuration/service'; import { PipEnvExecutionPath } from './configuration/executionSettings/pipEnvExecution'; -import { CryptoUtils } from './crypto'; import { EditorUtils } from './editor'; import { ExperimentService } from './experiments/service'; import { @@ -161,7 +159,6 @@ export function registerTypes(serviceManager: IServiceManager): void { ITerminalActivationHandler, PowershellTerminalActivationFailedHandler, ); - serviceManager.addSingleton(ICryptoUtils, CryptoUtils); serviceManager.addSingleton(IExperimentService, ExperimentService); serviceManager.addSingleton(ITerminalHelper, TerminalHelper); diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 0e7523317cb1..1ba908c91505 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -471,25 +471,6 @@ export interface IHashFormat { string: string; // If hash format is a string } -/** - * Interface used to implement cryptography tools - */ -export const ICryptoUtils = Symbol('ICryptoUtils'); -export interface ICryptoUtils { - /** - * Creates hash using the data and encoding specified - * @returns hash as number, or string - * @param data The string to hash - * @param hashFormat Return format of the hash, number or string - * @param [algorithm] - */ - createHash( - data: string, - hashFormat: E, - algorithm?: 'SHA512' | 'SHA256' | 'FNV', - ): IHashFormat[E]; -} - export const IAsyncDisposableRegistry = Symbol('IAsyncDisposableRegistry'); export interface IAsyncDisposableRegistry extends IAsyncDisposable { push(disposable: IDisposable | IAsyncDisposable): void; diff --git a/src/test/common/crypto.unit.test.ts b/src/test/common/crypto.unit.test.ts deleted file mode 100644 index 5883ccf5aaf3..000000000000 --- a/src/test/common/crypto.unit.test.ts +++ /dev/null @@ -1,143 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { assert, expect } from 'chai'; -import * as fs from 'fs-extra'; -import * as path from 'path'; -import { CryptoUtils } from '../../client/common/crypto'; -import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../constants'; - -const RANDOM_WORDS = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'test', 'common', 'randomWords.txt'); - -suite('Crypto Utils', async () => { - let crypto: CryptoUtils; - let wordsText: string; - suiteSetup(async () => { - wordsText = await fs.readFile(RANDOM_WORDS, 'utf8'); - }); - setup(() => { - crypto = new CryptoUtils(); - }); - async function getRandomWords(): Promise { - return wordsText.split('\n'); - } - - test('If hashFormat equals `number`, method createHash() returns a number', async () => { - const hash = crypto.createHash('blabla', 'number'); - assert.typeOf(hash, 'number', 'Type should be a number'); - }); - test('If hashFormat equals `string`, method createHash() returns a string', async () => { - const hash = crypto.createHash('blabla', 'string'); - assert.typeOf(hash, 'string', 'Type should be a string'); - }); - test('Hashes must be same for same strings (sha256)', async () => { - const hash1 = crypto.createHash('blabla', 'string', 'SHA256'); - const hash2 = crypto.createHash('blabla', 'string', 'SHA256'); - assert.equal(hash1, hash2); - }); - test('Hashes must be different for different strings (sha256)', async () => { - const hash1 = crypto.createHash('Hello', 'string', 'SHA256'); - const hash2 = crypto.createHash('World', 'string', 'SHA256'); - assert.notEqual(hash1, hash2); - }); - test('If hashFormat equals `number`, the hash should not be NaN', async () => { - let hash = crypto.createHash('test', 'number'); - assert.isNotNaN(hash, 'Number hash should not be NaN'); - hash = crypto.createHash('hash', 'number'); - assert.isNotNaN(hash, 'Number hash should not be NaN'); - hash = crypto.createHash('HASH1', 'number'); - assert.isNotNaN(hash, 'Number hash should not be NaN'); - }); - test('If hashFormat equals `string`, the hash should not be undefined', async () => { - let hash = crypto.createHash('test', 'string'); - assert.isDefined(hash, 'String hash should not be undefined'); - hash = crypto.createHash('hash', 'string'); - assert.isDefined(hash, 'String hash should not be undefined'); - hash = crypto.createHash('HASH1', 'string'); - assert.isDefined(hash, 'String hash should not be undefined'); - }); - test('If hashFormat equals `number`, hashes with different data should return different number hashes', async () => { - const hash1 = crypto.createHash('hash1', 'number'); - const hash2 = crypto.createHash('hash2', 'number'); - assert.notEqual(hash1, hash2, 'Hashes should be different numbers'); - }); - test('If hashFormat equals `string`, hashes with different data should return different string hashes', async () => { - const hash1 = crypto.createHash('hash1', 'string'); - const hash2 = crypto.createHash('hash2', 'string'); - assert.notEqual(hash1, hash2, 'Hashes should be different strings'); - }); - test('If hashFormat equals `number`, ensure numbers are uniformly distributed on scale from 0 to 100', async () => { - const wordList = await getRandomWords(); - const buckets: number[] = Array(100).fill(0); - const hashes = Array(10).fill(0); - for (const w of wordList) { - for (let i = 0; i < 10; i += 1) { - const word = `${w}${i}`; - const hash = crypto.createHash(word, 'number'); - buckets[hash % 100] += 1; - hashes[i] = hash % 100; - } - } - // Total number of words = wordList.length * 10, because we added ten variants of each word above. - const expectedHitsPerBucket = (wordList.length * 10) / 100; - for (const hit of buckets) { - expect(hit).to.be.lessThan(1.25 * expectedHitsPerBucket); - expect(hit).to.be.greaterThan(0.75 * expectedHitsPerBucket); - } - }); - test('If hashFormat equals `number`, on a scale of 0 to 100, small difference in the input on average produce large differences (about 33) in the output ', async () => { - const wordList = await getRandomWords(); - const buckets: number[] = Array(100).fill(0); - let hashes: number[] = []; - let totalDifference = 0; - // We are only iterating over the first 10 words for purposes of this test - for (const w of wordList.slice(0, 10)) { - hashes = []; - totalDifference = 0; - if (w.length === 0) { - continue; - } - for (let i = 0; i < 10; i += 1) { - const word = `${w}${i}`; - const hash = crypto.createHash(word, 'number'); - buckets[hash % 100] += 1; - hashes.push(hash % 100); - } - for (let i = 0; i < 10; i += 1) { - const word = `${i}${w}`; - const hash = crypto.createHash(word, 'number'); - buckets[hash % 100] += 1; - hashes.push(hash % 100); - } - // Iterating over ASCII alphabets 'a' to 'z' and appending to the word - for (let i = 0; i < 26; i += 1) { - const word = `${String.fromCharCode(97 + i)}${w}`; - const hash = crypto.createHash(word, 'number'); - buckets[hash % 100] += 1; - hashes.push(hash % 100); - } - // Iterating over ASCII alphabets 'a' to 'z' and prepending to the word - for (let i = 0; i < 26; i += 1) { - const word = `${w}${String.fromCharCode(97 + i)}`; - const hash = crypto.createHash(word, 'number'); - buckets[hash % 100] += 1; - hashes.push(hash % 100); - } - - for (let i = 0; i < hashes.length; i += 1) { - for (let j = 0; j < hashes.length; j += 1) { - if (hashes[i] > hashes[j]) { - totalDifference += hashes[i] - hashes[j]; - } else { - totalDifference += hashes[j] - hashes[i]; - } - } - } - const averageDifference = totalDifference / hashes.length / hashes.length; - expect(averageDifference).to.be.lessThan(1.25 * 33); - expect(averageDifference).to.be.greaterThan(0.75 * 33); - } - }); -}); diff --git a/src/test/common/installer.test.ts b/src/test/common/installer.test.ts index 49c6238e390f..a5de8634b341 100644 --- a/src/test/common/installer.test.ts +++ b/src/test/common/installer.test.ts @@ -25,7 +25,6 @@ import { import { WorkspaceService } from '../../client/common/application/workspace'; import { AsyncDisposableRegistry } from '../../client/common/asyncDisposableRegistry'; import { ConfigurationService } from '../../client/common/configuration/service'; -import { CryptoUtils } from '../../client/common/crypto'; import { EditorUtils } from '../../client/common/editor'; import { ExperimentService } from '../../client/common/experiments/service'; import { @@ -93,7 +92,6 @@ import { IAsyncDisposableRegistry, IBrowserService, IConfigurationService, - ICryptoUtils, ICurrentProcess, IEditorUtils, IExperimentService, @@ -214,7 +212,6 @@ suite('Installer', () => { ITerminalActivationHandler, PowershellTerminalActivationFailedHandler, ); - ioc.serviceManager.addSingleton(ICryptoUtils, CryptoUtils); ioc.serviceManager.addSingleton(IExperimentService, ExperimentService); ioc.serviceManager.addSingleton(ITerminalHelper, TerminalHelper); diff --git a/src/test/common/moduleInstaller.test.ts b/src/test/common/moduleInstaller.test.ts index 91c0afd32997..7290b9eeae73 100644 --- a/src/test/common/moduleInstaller.test.ts +++ b/src/test/common/moduleInstaller.test.ts @@ -31,7 +31,6 @@ import { import { WorkspaceService } from '../../client/common/application/workspace'; import { AsyncDisposableRegistry } from '../../client/common/asyncDisposableRegistry'; import { ConfigurationService } from '../../client/common/configuration/service'; -import { CryptoUtils } from '../../client/common/crypto'; import { EditorUtils } from '../../client/common/editor'; import { DiscoveryVariants } from '../../client/common/experiments/groups'; import { ExperimentService } from '../../client/common/experiments/service'; @@ -94,7 +93,6 @@ import { IAsyncDisposableRegistry, IBrowserService, IConfigurationService, - ICryptoUtils, ICurrentProcess, IEditorUtils, IExperimentService, @@ -256,7 +254,6 @@ suite('Module Installer', () => { ITerminalActivationHandler, PowershellTerminalActivationFailedHandler, ); - ioc.serviceManager.addSingleton(ICryptoUtils, CryptoUtils); ioc.serviceManager.addSingleton(IExperimentService, ExperimentService); ioc.serviceManager.addSingleton( diff --git a/src/test/common/serviceRegistry.unit.test.ts b/src/test/common/serviceRegistry.unit.test.ts index 72ba200bddcb..78dbf75dd8d2 100644 --- a/src/test/common/serviceRegistry.unit.test.ts +++ b/src/test/common/serviceRegistry.unit.test.ts @@ -30,7 +30,6 @@ import { WorkspaceService } from '../../client/common/application/workspace'; import { AsyncDisposableRegistry } from '../../client/common/asyncDisposableRegistry'; import { ConfigurationService } from '../../client/common/configuration/service'; import { PipEnvExecutionPath } from '../../client/common/configuration/executionSettings/pipEnvExecution'; -import { CryptoUtils } from '../../client/common/crypto'; import { EditorUtils } from '../../client/common/editor'; import { ExtensionInsidersDailyChannelRule, @@ -82,7 +81,6 @@ import { IAsyncDisposableRegistry, IBrowserService, IConfigurationService, - ICryptoUtils, ICurrentProcess, IEditorUtils, IExtensions, @@ -130,7 +128,6 @@ suite('Common - Service Registry', () => { [INugetService, NugetService], [ITerminalActivator, TerminalActivator], [ITerminalActivationHandler, PowershellTerminalActivationFailedHandler], - [ICryptoUtils, CryptoUtils], [ITerminalHelper, TerminalHelper], [ITerminalActivationCommandProvider, PyEnvActivationCommandProvider, TerminalActivationProviders.pyenv], [ITerminalActivationCommandProvider, Bash, TerminalActivationProviders.bashCShellFish], From 31697eb35d1005777f28f59e29f3d77c33ce7f12 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Mon, 4 Oct 2021 14:03:16 -0700 Subject: [PATCH 2/2] News entry --- news/3 Code Health/7333.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/3 Code Health/7333.md diff --git a/news/3 Code Health/7333.md b/news/3 Code Health/7333.md new file mode 100644 index 000000000000..b94f05143661 --- /dev/null +++ b/news/3 Code Health/7333.md @@ -0,0 +1 @@ +Remove unused SHA512 hashing code.