[Rpm-maint] [rpm-software-management/rpm] rpmsign: enable signing files with PKCS11 tokens (PR #4125)

[Jeremy Cline]

jeremycline left a comment (rpm-software-management/rpm#4125)

> > Hey folks, just a gentle nudge here. Are there any changes or additional tests people want before this is acceptable?
> 
> Heh, I've been wondering about more or less the same - is there something more coming?
> 
> The patch as such looks fine and acceptable to me. What's missing is updating the rpmsign man page to cover this functionality. As for the `--fskpath="pkcs11:token=...` stuff, I haven't got the faintest idea, so relying on @simo5 and @neverpanic for an "ack" on that part.

Sounds good, I added a bit to the fskpath argument noting it supports pkcs11 (with caveats)

> 
> I did just spot this comment from the above though:
> 
> > Typically I would say we should create a separate OpenSSL library context when loading new providers, but since this is a simple one-shot tool, I don't think that's a concern here.
> 
> rpmsign itself is a simple one-shot tool, but technically this bit is in the librpmsign library, which could be used in some other kind of context too, such as a daemon.

I started looking at this and realized that I was foolish to have only looked at the `ima_evm_signhash` function signature. It is, of course, undocumented so who knows what they actually intended, but it doesn't work like you would expect it to work, based on the types.

It never uses `access_info.u.provider`. Instead, it does [this little dance](https://github.com/linux-integrity/ima-evm-utils/blob/a2f0182c2e6fd84672ef5218d4626c81a9f963d4/src/libimaevm.c#L1074) (which seems close to what @simo5 described except they're caring about the URI):
```C
	store = OSSL_STORE_open_ex(keyfile, NULL, "provider=pkcs11", ui_method,
				   (void *)keypass, NULL, NULL, NULL);
	if (!store) {
		log_err("Failed to open store for provider=pkcs11\n");
		goto err_provider;
	}
	for (info = OSSL_STORE_load(store);
	     info != NULL && pkey == NULL;
	     info = OSSL_STORE_load(store)) {
		typ = OSSL_STORE_INFO_get_type(info);

		switch (typ) {
		case OSSL_STORE_INFO_PKEY:
			pkey = OSSL_STORE_INFO_get1_PKEY(info);
			break;
		}
		OSSL_STORE_INFO_free(info);
	}
	OSSL_STORE_close(store);
```

The second argument to `OSSL_STORE_open_ex()` is the library context (NULL being the default/global one). So, there's no reason to create our own context. Nor is there any reason to actually load the provider ourselves (because `access_info.u.provider` is never used).

Furthermore, passing `type = IMAEVM_OSSL_ACCESS_TYPE_PROVIDER` is exactly the same as passing `type = IMAEVM_OSSL_ACCESS_TYPE_NONE`, except if the [key starts with pkcs11:](https://github.com/linux-integrity/ima-evm-utils/blob/a2f0182c2e6fd84672ef5218d4626c81a9f963d4/src/libimaevm.c#L1139). Since the engine API is dead, there's no reason for rpmsign to ever provide a different type.

So, I've gotten rid of everything except unconditionally setting `IMAEVM_OSSL_ACCESS_TYPE_PROVIDER`. Users need to provide an openssl configuration that loads the provider (see the test for an example).

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/4125#issuecomment-4119148766
You are receiving this because you are subscribed to this thread.

Message ID: <rpm-software-management/rpm/pull/4125/c4119148766 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20260324/4a9920fc/attachment-0001.htm>