Re: [PATCH 2/5] PM / hibernate: Generate and verify signature for snapshot image

[Stephan Mueller <smueller@chronox.de>]

Am Donnerstag, 3. Januar 2019, 15:32:24 CET schrieb Lee, Chun-Yi:

Hi Chun,

> +int snapshot_image_verify_decrypt(void)
> +{
> +	int ret, i;
> +
> +	if (!h_buf) {
> +		ret = -ENOMEM;
> +		goto error;
> +	}
> +
> +	ret = snapshot_key_init();
> +	if (ret)
> +		goto error_prep;
> +
> +	ret = snapshot_prepare_hash(true);
> +	if (ret || !s4_verify_desc)
> +		goto error_prep;
> +
> +	for (i = 0; i < nr_copy_pages; i++) {
> +		ret = crypto_shash_update(s4_verify_desc, *(h_buf + i), PAGE_SIZE);
> +		if (ret)
> +			goto error_shash;
> +	}
> +
> +	ret = crypto_shash_final(s4_verify_desc, s4_verify_digest);
> +	if (ret)
> +		goto error_shash;
> +
> +	pr_debug("Signature %*phN\n", SNAPSHOT_DIGEST_SIZE, signature);
> +	pr_debug("Digest    %*phN\n", SNAPSHOT_DIGEST_SIZE, s4_verify_digest);
> +	if (memcmp(signature, s4_verify_digest, SNAPSHOT_DIGEST_SIZE))
> +		ret = -EKEYREJECTED;
> +
> + error_shash:
> +	snapshot_finish_hash();
> +
> + error_prep:
> +	vfree(h_buf);
> +	if (ret)
> +		pr_warn("Signature verification failed: %d\n", ret);
> + error:
> +	sig_verify_ret = ret;
> +	return ret;
> +}

May I ask why the authentication part is done manually here? Why not using an 
AEAD cipher like the authenc() ciphers, or CCM (I would not recommend GCM 
though)? In this case, the encryption/decryption operation would automatically 
perform the creation of the hash and the verification of the hash. I.e. 
decryption can return EBADMSG which indicates an authentication failure.

> +
> +static int
> +__copy_data_pages(struct memory_bitmap *copy_bm, struct memory_bitmap
> *orig_bm) +{
> +	unsigned long pfn, dst_pfn;
> +	struct page *d_page;
> +	void *crypto_buffer = NULL;
> +	int ret = 0;
> +
> +	memory_bm_position_reset(orig_bm);
> +	memory_bm_position_reset(copy_bm);
> +	for (;;) {
> +		pfn = memory_bm_next_pfn(orig_bm);
> +		if (unlikely(pfn == BM_END_OF_MAP))
> +			break;
> +		dst_pfn = memory_bm_next_pfn(copy_bm);
> +		copy_data_page(dst_pfn, pfn);
> +
> +		/* Setup buffer */
> +		d_page = pfn_to_page(dst_pfn);
> +		if (PageHighMem(d_page)) {
> +			void *kaddr = kmap_atomic(d_page);
> +
> +			copy_page(buffer, kaddr);
> +			kunmap_atomic(kaddr);
> +			crypto_buffer = buffer;
> +		} else {
> +			crypto_buffer = page_address(d_page);
> +		}
> +
> +		/* Generate digest */
> +		if (!s4_verify_desc)
> +			continue;
> +		ret = crypto_shash_update(s4_verify_desc, crypto_buffer,
> +					  PAGE_SIZE);

Same here, the creation of the hash would be implicit during the encryption.

Ciao
Stephan