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

feat(cc): implement inclusion using Security S2 #3170

Merged
merged 4 commits into from
Aug 10, 2021

Conversation

AlCalzone
Copy link
Member

@AlCalzone AlCalzone commented Aug 10, 2021

This PR implements inclusion of new nodes using Security S2. To realize this, new inclusion strategies have been added, marking the old S0-by-default strategy as deprecated:

Default:
Prefer Security S2 if supported, use Security S0 if absolutely necessary (e.g. for legacy locks), don't use encryption otherwise.
Issues a warning if Security S2/S0 is attempted to be used but secure bootstrapping fails.
👍🏻 This is the recommended strategy and should be used unless there is a good reason not to.


SmartStart:
Include using SmartStart (requires Security S2).
Can't include a device that does not support Smart Start.
Issues a warning if secure bootstrapping fails.
👍🏻👍🏻 Should be preferred over Default if supported.


Insecure:
Don't use encryption, even if supported.
👎🏻 Not recommended, because S2 should be used where possible.


Security_S0:
Use Security S0, even if a higher security mode is supported.
Issues a warning if Security S0 is not supported or the secure bootstrapping fails.
👎🏻👎🏻 Not recommended because S0 should be used sparingly and S2 preferred whereever possible.


Security_S2:
Use Security S2 and issue a warning if it is not supported or the secure bootstrapping fails.
🤷🏻‍♂️ Not really an advantage over Default. Less versatile, more complicated for the user.


The method signatures for beginInclusion and replaceFailedNode have been changed as follows

public async beginInclusion(options: InclusionOptions): Promise<boolean>;
public async replaceFailedNode(nodeId: number, options: ReplaceNodeOptions): Promise<boolean>;

to accomodate for S2 inclusion (the old variants will keep working until v9.x).

The shape of these options depend on the inclusion strategy:

export type InclusionOptions =
	| {
			strategy: InclusionStrategy.Default;
			userCallbacks: InclusionUserCallbacks;
			/**
			 * Force secure communication (S0) even when S2 is not supported and S0 is supported but not necessary.
			 * This is not recommended due to the overhead caused by S0.
			 */
			forceSecurity?: boolean;
	  }
	| {
			strategy: InclusionStrategy.Security_S2;
			userCallbacks: InclusionUserCallbacks;
	  }
	| {
			strategy: InclusionStrategy.SmartStart;
			provisioningList: unknown;
	  }
	| {
			strategy: InclusionStrategy.Insecure | InclusionStrategy.Security_S0;
	  };

Default, and Security_S2 will accept user callbacks that allow interactively validating the DSK and choosing security classes. Although it is not guaranteed that they will be called, the application MUST provide them:

export interface InclusionUserCallbacks {
	/**
	 * Instruct the application to display the user which security classes the device has requested and whether client-side authentication (CSA) is desired.
	 * The returned promise MUST resolve to the user selection - which of the requested security classes have been granted and whether CSA was allowed.
	 * If the user did not accept the requested security classes, the promise MUST resolve to `true`.
	 */
	grantSecurityClasses(
		requested: InclusionGrant,
	): Promise<InclusionGrant | false>;

	/**
	 * Instruct the application to display the received DSK for the user to verify if it matches the one belonging to the device and
	 * additionally enter the PIN that's found on the device.
	 * The returned promise MUST resolve to the 5-digit PIN (as a string) when the user has confirmed the DSK and entered the PIN and `false` otherwise.
	 *
	 * @param dsk The partial DSK in the form `-bbbbb-ccccc-ddddd-eeeee-fffff-11111-22222`. The first 5 characters are left out because they are the unknown PIN.
	 */
	validateDSKAndEnterPIN(dsk: string): Promise<string | false>;

	/** Called by the driver when the user validation has timed out and needs to be aborted */
	abort(): void;
}

export interface InclusionGrant {
	/**
	 * An array of security classes that are requested or to be granted.
	 * The granted security classes MUST be a subset of the requested ones.
	 */
	securityClasses: SecurityClass[];
	/** Whether client side authentication is requested or to be granted */
	clientSideAuth: boolean;
}

For the Default strategy, the application can also decide to prefer S0 over no encryption (forceSecurity: true), even if it is not necessary.

SmartStart is just a stub at this point and likely to change. Insecure and Security_S0 won't need any arguments.


replaceFailedNode only supports a subset of the inclusion strategies. Because that command does not provide the node info of the new node, reacting to the supported CCs or device classes is not possible. For that reason, the application MUST tell zwave-js before the inclusion whether S2, S0 or no encryption should be used:

export type ReplaceNodeOptions =
	| {
			strategy: InclusionStrategy.Security_S2;
			userCallbacks: InclusionUserCallbacks;
	  }
	| {
			strategy: InclusionStrategy.Insecure | InclusionStrategy.Security_S0;
	  };

Last but not least this PR adds a second parameter to the "node added" event to indicate whether a node was included with lower than intended security:

// node added event handler:
(node: ZWaveNode, result: InclusionResult) => void;

export interface InclusionResult {
	/** This flag warns that a node was included with a lower than intended security, e.g. unencrypted instead of Security S2 */
	lowSecurity?: boolean;
}

@AlCalzone AlCalzone merged commit 3a82344 into cc/security-s2 Aug 10, 2021
@AlCalzone AlCalzone deleted the cc/s2-inclusion branch August 10, 2021 13:19
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.

1 participant