-
Notifications
You must be signed in to change notification settings - Fork 1
Integration of rust crypto-layer for secure element support #636
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
base: main
Are you sure you want to change the base?
Conversation
| public async init(): Promise<Transport> { | ||
| log.trace("Initializing Libsodium..."); | ||
| await SodiumWrapper.ready(); | ||
| const sodium = SodiumWrapper.ready(); |
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.
Why did you split the await of SodiumWrapper?
| await this.identity.init(deviceSharedSecret.identity); | ||
| await this.identityDeletionProcess.init(); | ||
| await this.activeDevice.init(privBaseDevice, device); | ||
| await this.activeDevice.init(privBaseDevice as DeviceBoundKeyHandle, device); |
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.
Careful here, it is DeviceBoundKeyHandle | CryptoSecretKey. I guess you could add this to the top at the definition of let privBaseDevice
| this.info.set("device", device.toJSON()), | ||
| this.info.set("identity", deviceSharedSecret.identity.toJSON()), | ||
| this.info.set("baseKey", privBaseDevice.toJSON()), | ||
| this.info.set("isHardwareBased", isHardwareBased), |
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 would rather go with setting baseKey for legacy and baseKeyHandle with the CAL. Or we should rename isHardwareBased to isBaseKeyStoredOnHardware.
| | CryptoSignatureKeypair | ||
| | CryptoSignaturePrivateKey | ||
| | CryptoSecretKey | ||
| | DeviceBoundKeyHandle, |
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.
Does it make sense to store the DeviceBoundKeyHandle to the DeviceSecretController? I mean the KeyHandle is not a secret.
| import path from "path"; | ||
| import * as tmp from "tmp"; | ||
| import { AccountController, DeviceSharedSecret, Transport } from "../../src"; | ||
| import { ALL_CRYPTO_PROVIDERS } from "../../src/core/CryptoProviderMapping"; |
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.
Tests shouldn't import from "../../src", you might need to export CryptoProviderMapping to import from "@nmshd/transport"?
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.
import from src is ok, but there might be an export missing (so that CryptoProviderMapping is available at ../../src)
| Software: "Software", | ||
| Hardware: "Hardware", | ||
| Network: "Network", | ||
| LEGACY: "LEGACY" |
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.
Why is LEGACY Uppercase?
| const FALLBACK_PREFERENCE: SecurityLevel = CryptoProviderTypes.Software; | ||
|
|
||
| export function getPreferredProviderLevel(cryptoObject: CryptoObject, cryptoOperation: CryptoKeyType, purpose?: Exclude<CryptoPurpose, undefined>): SecurityLevel { | ||
| const allowedOps = CRYPTO_OPERATION_OBJECT_MAP[cryptoObject]; |
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.
Why do you need this autorization on code level base?
|
|
||
| export const ALL_CRYPTO_PROVIDERS = ["SoftwareProvider", "AndroidProvider"]; | ||
|
|
||
| const CRYPTO_OPERATION_OBJECT_MAP: Partial<Record<CryptoObject, CryptoKeyType[]>> = { |
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.
For me, this is a duplicate of OBJECT_OPERATION_PREFERENCES with less details :)
| } as const; | ||
| type CryptoObject = (typeof CryptoObject)[keyof typeof CryptoObject]; | ||
|
|
||
| export const ALL_CRYPTO_PROVIDERS = ["SoftwareProvider", "AndroidProvider"]; |
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 don't get the AndroidProvider here. Shouldn't this information come either from CAL or the app?
…are provider for crypto
.gitignore
Outdated
| tsconfig.tsbuildinfo | ||
| node_modules | ||
| packages/*/coverage | ||
| nmshd-runtime.code-workspace |
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.
undo that please.
Readiness checklist
Integrates the rust crypto-layer (CAL) into the runtime to support secure elements for key storage and crypto operations.