From: Michal Suchanek <hidden> Date: 2021-11-25 18:05:38
Hello,
This is resend of the KEXEC_SIG patchset.
The first patch is new because it'a a cleanup that does not require any
change to the module verification code.
The second patch is the only one that is intended to change any
functionality.
The rest only deduplicates code but I did not receive any review on that
part so I don't know if it's desirable as implemented.
The first two patches can be applied separately without the rest.
Thanks
Michal
Michal Suchanek (6):
s390/kexec_file: Don't opencode appended signature check.
powerpc/kexec_file: Add KEXEC_SIG support.
kexec_file: Don't opencode appended signature verification.
module: strip the signature marker in the verification function.
module: Use key_being_used_for for log messages in
verify_appended_signature
module: Move duplicate mod_check_sig users code to mod_parse_sig
arch/powerpc/Kconfig | 11 +++++
arch/powerpc/kexec/elf_64.c | 14 ++++++
arch/s390/kernel/machine_kexec_file.c | 42 ++----------------
crypto/asymmetric_keys/asymmetric_type.c | 1 +
include/linux/module_signature.h | 1 +
include/linux/verification.h | 4 ++
kernel/module-internal.h | 2 -
kernel/module.c | 12 +++--
kernel/module_signature.c | 56 +++++++++++++++++++++++-
kernel/module_signing.c | 33 +++++++-------
security/integrity/ima/ima_modsig.c | 22 ++--------
11 files changed, 113 insertions(+), 85 deletions(-)
--
2.31.1
@@ -29,6 +29,7 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)constunsignedlongmarker_len=sizeof(MODULE_SIG_STRING)-1;structmodule_signature*ms;unsignedlongsig_len;+intret;/* Skip signature verification when not secure IPLed. */if(!ipl_secure_flag)
@@ -43,25 +44,12 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)kernel_len-=marker_len;ms=(void*)kernel+kernel_len-sizeof(*ms);-kernel_len-=sizeof(*ms);+ret=mod_check_sig(ms,kernel_len,"kexec");+if(ret)+returnret;sig_len=be32_to_cpu(ms->sig_len);-if(sig_len>=kernel_len)-return-EKEYREJECTED;-kernel_len-=sig_len;--if(ms->id_type!=PKEY_ID_PKCS7)-return-EKEYREJECTED;--if(ms->algo!=0||-ms->hash!=0||-ms->signer_len!=0||-ms->key_id_len!=0||-ms->__pad[0]!=0||-ms->__pad[1]!=0||-ms->__pad[2]!=0){-return-EBADMSG;-}+kernel_len-=sizeof(*ms)+sig_len;returnverify_pkcs7_signature(kernel,kernel_len,kernel+kernel_len,sig_len,
From: Michal Suchanek <hidden> Date: 2021-11-25 18:05:46
Multiple users of mod_check_sig check for the marker, then call
mod_check_sig, extract signature length, and remove the signature.
Put this code in one place together with mod_check_sig.
Signed-off-by: Michal Suchanek <redacted>
---
include/linux/module_signature.h | 1 +
kernel/module_signature.c | 56 ++++++++++++++++++++++++++++-
kernel/module_signing.c | 26 +++-----------
security/integrity/ima/ima_modsig.c | 22 ++----------
4 files changed, 63 insertions(+), 42 deletions(-)
@@ -37,33 +37,17 @@ struct modsig {**Return:0onsuccess,errorcodeotherwise.*/-intima_read_modsig(enumima_hooksfunc,constvoid*buf,loff_tbuf_len,+intima_read_modsig(enumima_hooksfunc,constvoid*buf,loff_tlen,structmodsig**modsig){-constsize_tmarker_len=strlen(MODULE_SIG_STRING);-conststructmodule_signature*sig;structmodsig*hdr;-size_tsig_len;-constvoid*p;+size_tsig_len,buf_len=len;intrc;-if(buf_len<=marker_len+sizeof(*sig))-return-ENOENT;--p=buf+buf_len-marker_len;-if(memcmp(p,MODULE_SIG_STRING,marker_len))-return-ENOENT;--buf_len-=marker_len;-sig=(conststructmodule_signature*)(p-sizeof(*sig));--rc=mod_check_sig(sig,buf_len,func_tokens[func]);+rc=mod_parse_sig(buf,&buf_len,&sig_len,func_tokens[func]);if(rc)returnrc;-sig_len=be32_to_cpu(sig->sig_len);-buf_len-=sig_len+sizeof(*sig);-/* Allocate sig_len additional bytes to hold the raw PKCS#7 data. */hdr=kzalloc(sizeof(*hdr)+sig_len,GFP_KERNEL);if(!hdr)
From: Michal Suchanek <hidden> Date: 2021-11-25 18:05:51
Add value for kexec appended signature and pass in key_being_used_for
enum rather than a string to verify_appended_signature to produce log
messages about the signature.
Signed-off-by: Michal Suchanek <redacted>
---
arch/powerpc/kexec/elf_64.c | 2 +-
arch/s390/kernel/machine_kexec_file.c | 2 +-
crypto/asymmetric_keys/asymmetric_type.c | 1 +
include/linux/verification.h | 3 ++-
kernel/module.c | 3 ++-
kernel/module_signing.c | 11 ++++++-----
6 files changed, 13 insertions(+), 9 deletions(-)
@@ -2890,11 +2889,7 @@ static int module_sig_check(struct load_info *info, int flags)*Requireflags==0,asamodulewithversioninformation*removedisnolongerthemodulethatwassigned*/-if(flags==0&&-info->len>markerlen&&-memcmp(mod+info->len-markerlen,MODULE_SIG_STRING,markerlen)==0){-/* We truncate the module to discard the signature */-info->len-=markerlen;+if(flags==0){err=verify_appended_signature(mod,&info->len,VERIFY_USE_SECONDARY_KEYRING,"module");if(!err){
@@ -2894,7 +2895,8 @@ static int module_sig_check(struct load_info *info, int flags)memcmp(mod+info->len-markerlen,MODULE_SIG_STRING,markerlen)==0){/* We truncate the module to discard the signature */info->len-=markerlen;-err=mod_verify_sig(mod,info);+err=verify_appended_signature(mod,&info->len,+VERIFY_USE_SECONDARY_KEYRING,"module");if(!err){info->sig_ok=true;return0;
On Thu, Nov 25, 2021 at 07:02:38PM +0100, Michal Suchanek wrote:
Hello,
This is resend of the KEXEC_SIG patchset.
The first patch is new because it'a a cleanup that does not require any
change to the module verification code.
The second patch is the only one that is intended to change any
functionality.
The rest only deduplicates code but I did not receive any review on that
part so I don't know if it's desirable as implemented.
The first two patches can be applied separately without the rest.
Thanks
Michal
Michal Suchanek (6):
s390/kexec_file: Don't opencode appended signature check.
powerpc/kexec_file: Add KEXEC_SIG support.
kexec_file: Don't opencode appended signature verification.
module: strip the signature marker in the verification function.
module: Use key_being_used_for for log messages in
verify_appended_signature
module: Move duplicate mod_check_sig users code to mod_parse_sig
arch/powerpc/Kconfig | 11 +++++
arch/powerpc/kexec/elf_64.c | 14 ++++++
arch/s390/kernel/machine_kexec_file.c | 42 ++----------------
crypto/asymmetric_keys/asymmetric_type.c | 1 +
include/linux/module_signature.h | 1 +
include/linux/verification.h | 4 ++
kernel/module-internal.h | 2 -
kernel/module.c | 12 +++--
kernel/module_signature.c | 56 +++++++++++++++++++++++-
kernel/module_signing.c | 33 +++++++-------
security/integrity/ima/ima_modsig.c | 22 ++--------
11 files changed, 113 insertions(+), 85 deletions(-)
For all patches which touch s390:
Acked-by: Heiko Carstens <hca@linux.ibm.com>
From: Baoquan He <hidden> Date: 2021-12-01 02:38:13
Hi,
On 11/25/21 at 07:02pm, Michal Suchanek wrote:
Hello,
This is resend of the KEXEC_SIG patchset.
The first patch is new because it'a a cleanup that does not require any
change to the module verification code.
The second patch is the only one that is intended to change any
functionality.
The rest only deduplicates code but I did not receive any review on that
part so I don't know if it's desirable as implemented.
Do you have the link of your 1st version?
And after going through the whole series, it doesn't tell what this
patch series intends to do in cover-letter or patch log.
Thanks
Baoquan
The first two patches can be applied separately without the rest.
Thanks
Michal
Michal Suchanek (6):
s390/kexec_file: Don't opencode appended signature check.
powerpc/kexec_file: Add KEXEC_SIG support.
kexec_file: Don't opencode appended signature verification.
module: strip the signature marker in the verification function.
module: Use key_being_used_for for log messages in
verify_appended_signature
module: Move duplicate mod_check_sig users code to mod_parse_sig
arch/powerpc/Kconfig | 11 +++++
arch/powerpc/kexec/elf_64.c | 14 ++++++
arch/s390/kernel/machine_kexec_file.c | 42 ++----------------
crypto/asymmetric_keys/asymmetric_type.c | 1 +
include/linux/module_signature.h | 1 +
include/linux/verification.h | 4 ++
kernel/module-internal.h | 2 -
kernel/module.c | 12 +++--
kernel/module_signature.c | 56 +++++++++++++++++++++++-
kernel/module_signing.c | 33 +++++++-------
security/integrity/ima/ima_modsig.c | 22 ++--------
11 files changed, 113 insertions(+), 85 deletions(-)
--
2.31.1
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
From: Michal Suchánek <hidden> Date: 2021-12-01 11:48:48
Hello,
On Wed, Dec 01, 2021 at 10:37:47AM +0800, Baoquan He wrote:
Hi,
On 11/25/21 at 07:02pm, Michal Suchanek wrote:
quoted
Hello,
This is resend of the KEXEC_SIG patchset.
The first patch is new because it'a a cleanup that does not require any
change to the module verification code.
The second patch is the only one that is intended to change any
functionality.
The rest only deduplicates code but I did not receive any review on that
part so I don't know if it's desirable as implemented.
From: Philipp Rudo <hidden> Date: 2021-12-07 16:10:56
Hi Michal,
i finally had the time to take a closer look at the series. Except for
the nit in patch 4 and my personal preference in patch 6 the code looks
good to me.
What I don't like are the commit messages on the first commits. In my
opinion they are so short that they are almost useless. For example in
patch 2 there is absolutely no explanation why you can simply copy the
s390 over to ppc. Or in patch 3 you are silently changing the error
code in kexec from EKEYREJECT to ENODATA. So I would appreciate it if
you could improve them a little.
Thanks
Philipp
On Thu, 25 Nov 2021 19:02:38 +0100
Michal Suchanek [off-list ref] wrote:
Hello,
This is resend of the KEXEC_SIG patchset.
The first patch is new because it'a a cleanup that does not require any
change to the module verification code.
The second patch is the only one that is intended to change any
functionality.
The rest only deduplicates code but I did not receive any review on that
part so I don't know if it's desirable as implemented.
The first two patches can be applied separately without the rest.
Thanks
Michal
Michal Suchanek (6):
s390/kexec_file: Don't opencode appended signature check.
powerpc/kexec_file: Add KEXEC_SIG support.
kexec_file: Don't opencode appended signature verification.
module: strip the signature marker in the verification function.
module: Use key_being_used_for for log messages in
verify_appended_signature
module: Move duplicate mod_check_sig users code to mod_parse_sig
arch/powerpc/Kconfig | 11 +++++
arch/powerpc/kexec/elf_64.c | 14 ++++++
arch/s390/kernel/machine_kexec_file.c | 42 ++----------------
crypto/asymmetric_keys/asymmetric_type.c | 1 +
include/linux/module_signature.h | 1 +
include/linux/verification.h | 4 ++
kernel/module-internal.h | 2 -
kernel/module.c | 12 +++--
kernel/module_signature.c | 56 +++++++++++++++++++++++-
kernel/module_signing.c | 33 +++++++-------
security/integrity/ima/ima_modsig.c | 22 ++--------
11 files changed, 113 insertions(+), 85 deletions(-)
From: Philipp Rudo <hidden> Date: 2021-12-07 16:11:14
Hi Michal,
On Thu, 25 Nov 2021 19:02:44 +0100
Michal Suchanek [off-list ref] wrote:
quoted hunk
Multiple users of mod_check_sig check for the marker, then call
mod_check_sig, extract signature length, and remove the signature.
Put this code in one place together with mod_check_sig.
Signed-off-by: Michal Suchanek <redacted>
---
include/linux/module_signature.h | 1 +
kernel/module_signature.c | 56 ++++++++++++++++++++++++++++-
kernel/module_signing.c | 26 +++-----------
security/integrity/ima/ima_modsig.c | 22 ++----------
4 files changed, 63 insertions(+), 42 deletions(-)
I personally don't like it when a function has a "check" in it's name
as it doesn't describe what the function is checking for. For me
mod_has_sig_marker is much more precise. I would use that instead.
Thanks
Philipp
quoted hunk
+{
+ const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+
+ if (markerlen > *len)
+ return -ENODATA;
+
+ if (memcmp(data + *len - markerlen, MODULE_SIG_STRING,
+ markerlen))
+ return -ENODATA;
+
+ *len -= markerlen;
+ return 0;
+}
+
/**
* mod_check_sig - check that the given signature is sane
*
* @ms: Signature to check.
- * @file_len: Size of the file to which @ms is appended.
+ * @file_len: Size of the file to which @ms is appended (without the marker).
* @name: What is being checked. Used for error messages.
*/
int mod_check_sig(const struct module_signature *ms, size_t file_len,
@@ -44,3 +66,35 @@ int mod_check_sig(const struct module_signature *ms, size_t file_len, return 0; }++/**+ * mod_parse_sig - check that the given signature is sane and determine signature length+ *+ * @data: Data with appended signature.+ * @len: Length of data. Signature and marker length is subtracted on success.+ * @sig_len: Length of signature. Filled on success.+ * @name: What is being checked. Used for error messages.+ */+int mod_parse_sig(const void *data, size_t *len, size_t *sig_len, const char *name)+{+ const struct module_signature *sig;+ int rc;++ rc = mod_check_sig_marker(data, len);+ if (rc)+ return rc;++ if (*len < sizeof(*sig))+ return -ENODATA;++ sig = (const struct module_signature *)(data + (*len - sizeof(*sig)));++ rc = mod_check_sig(sig, *len, name);+ if (rc)+ return rc;++ *sig_len = be32_to_cpu(sig->sig_len);+ *len -= *sig_len + sizeof(*sig);++ return 0;+}
@@ -37,33 +37,17 @@ struct modsig {**Return:0onsuccess,errorcodeotherwise.*/-intima_read_modsig(enumima_hooksfunc,constvoid*buf,loff_tbuf_len,+intima_read_modsig(enumima_hooksfunc,constvoid*buf,loff_tlen,structmodsig**modsig){-constsize_tmarker_len=strlen(MODULE_SIG_STRING);-conststructmodule_signature*sig;structmodsig*hdr;-size_tsig_len;-constvoid*p;+size_tsig_len,buf_len=len;intrc;-if(buf_len<=marker_len+sizeof(*sig))-return-ENOENT;--p=buf+buf_len-marker_len;-if(memcmp(p,MODULE_SIG_STRING,marker_len))-return-ENOENT;--buf_len-=marker_len;-sig=(conststructmodule_signature*)(p-sizeof(*sig));--rc=mod_check_sig(sig,buf_len,func_tokens[func]);+rc=mod_parse_sig(buf,&buf_len,&sig_len,func_tokens[func]);if(rc)returnrc;-sig_len=be32_to_cpu(sig->sig_len);-buf_len-=sig_len+sizeof(*sig);-/* Allocate sig_len additional bytes to hold the raw PKCS#7 data. */hdr=kzalloc(sizeof(*hdr)+sig_len,GFP_KERNEL);if(!hdr)
From: Philipp Rudo <hidden> Date: 2021-12-07 16:11:32
Hi Michal,
On Thu, 25 Nov 2021 19:02:42 +0100
Michal Suchanek [off-list ref] wrote:
It is stripped by each caller separately.
Signed-off-by: Michal Suchanek <redacted>
---
arch/powerpc/kexec/elf_64.c | 9 ---------
arch/s390/kernel/machine_kexec_file.c | 9 ---------
kernel/module.c | 7 +------
kernel/module_signing.c | 12 ++++++++++--
kernel/module_signing.c is only compiled with MODULE_SIG enabled but
KEXEC_SIG only selects MODULE_SIG_FORMAT. In the unlikely case that
KEXEC_SIG is enabled but MODULE_SIG isn't this causes a build breakage.
So you need to update KEXEC_SIG to select MODULE_SIG instead of
MODULE_SIG_FORMAT for s390 and ppc.
Thanks
Philipp
@@ -2890,11 +2889,7 @@ static int module_sig_check(struct load_info *info, int flags)*Requireflags==0,asamodulewithversioninformation*removedisnolongerthemodulethatwassigned*/-if(flags==0&&-info->len>markerlen&&-memcmp(mod+info->len-markerlen,MODULE_SIG_STRING,markerlen)==0){-/* We truncate the module to discard the signature */-info->len-=markerlen;+if(flags==0){err=verify_appended_signature(mod,&info->len,VERIFY_USE_SECONDARY_KEYRING,"module");if(!err){
From: Michal Suchánek <hidden> Date: 2021-12-07 17:32:31
On Tue, Dec 07, 2021 at 05:10:14PM +0100, Philipp Rudo wrote:
Hi Michal,
i finally had the time to take a closer look at the series. Except for
the nit in patch 4 and my personal preference in patch 6 the code looks
good to me.
What I don't like are the commit messages on the first commits. In my
opinion they are so short that they are almost useless. For example in
patch 2 there is absolutely no explanation why you can simply copy the
s390 over to ppc.
They use the same signature format. I suppose I can add a note saying
that.
Or in patch 3 you are silently changing the error
code in kexec from EKEYREJECT to ENODATA. So I would appreciate it if
Not sure what I should do about this. The different implementations use
different random error codes, and when they are unified the error code
clearly changes for one or the other.
Does anything depend on a particular error code returned?
Thanks
Michal
you could improve them a little.
Thanks
Philipp
On Thu, 25 Nov 2021 19:02:38 +0100
Michal Suchanek [off-list ref] wrote:
quoted
Hello,
This is resend of the KEXEC_SIG patchset.
The first patch is new because it'a a cleanup that does not require any
change to the module verification code.
The second patch is the only one that is intended to change any
functionality.
The rest only deduplicates code but I did not receive any review on that
part so I don't know if it's desirable as implemented.
The first two patches can be applied separately without the rest.
Thanks
Michal
Michal Suchanek (6):
s390/kexec_file: Don't opencode appended signature check.
powerpc/kexec_file: Add KEXEC_SIG support.
kexec_file: Don't opencode appended signature verification.
module: strip the signature marker in the verification function.
module: Use key_being_used_for for log messages in
verify_appended_signature
module: Move duplicate mod_check_sig users code to mod_parse_sig
arch/powerpc/Kconfig | 11 +++++
arch/powerpc/kexec/elf_64.c | 14 ++++++
arch/s390/kernel/machine_kexec_file.c | 42 ++----------------
crypto/asymmetric_keys/asymmetric_type.c | 1 +
include/linux/module_signature.h | 1 +
include/linux/verification.h | 4 ++
kernel/module-internal.h | 2 -
kernel/module.c | 12 +++--
kernel/module_signature.c | 56 +++++++++++++++++++++++-
kernel/module_signing.c | 33 +++++++-------
security/integrity/ima/ima_modsig.c | 22 ++--------
11 files changed, 113 insertions(+), 85 deletions(-)
From: Philipp Rudo <hidden> Date: 2021-12-08 09:55:20
Hi Michal,
On Tue, 7 Dec 2021 18:32:21 +0100
Michal Suchánek [off-list ref] wrote:
On Tue, Dec 07, 2021 at 05:10:14PM +0100, Philipp Rudo wrote:
quoted
Hi Michal,
i finally had the time to take a closer look at the series. Except for
the nit in patch 4 and my personal preference in patch 6 the code looks
good to me.
What I don't like are the commit messages on the first commits. In my
opinion they are so short that they are almost useless. For example in
patch 2 there is absolutely no explanation why you can simply copy the
s390 over to ppc.
They use the same signature format. I suppose I can add a note saying
that.
The note is what I was asking for. For me the commit message is an
important piece of documentation for other developers (or yourself in a
year). That's why in my opinion it's important to describe _why_ you do
something in it as you cannot get the _why_ by reading the code.
quoted
Or in patch 3 you are silently changing the error
code in kexec from EKEYREJECT to ENODATA. So I would appreciate it if
Not sure what I should do about this. The different implementations use
different random error codes, and when they are unified the error code
clearly changes for one or the other.
My complaint wasn't that you change the return code. There's no way to
avoid choosing one over the other. It's again that you don't document
the change in the commit message for others.
Does anything depend on a particular error code returned?
Not that I know of. At least in the kexec-tools ENODATA and EKEYREJECT
are handled the same way.
Thanks
Philipp
Thanks
Michal
quoted
you could improve them a little.
Thanks
Philipp
On Thu, 25 Nov 2021 19:02:38 +0100
Michal Suchanek [off-list ref] wrote:
quoted
Hello,
This is resend of the KEXEC_SIG patchset.
The first patch is new because it'a a cleanup that does not require any
change to the module verification code.
The second patch is the only one that is intended to change any
functionality.
The rest only deduplicates code but I did not receive any review on that
part so I don't know if it's desirable as implemented.
The first two patches can be applied separately without the rest.
Thanks
Michal
Michal Suchanek (6):
s390/kexec_file: Don't opencode appended signature check.
powerpc/kexec_file: Add KEXEC_SIG support.
kexec_file: Don't opencode appended signature verification.
module: strip the signature marker in the verification function.
module: Use key_being_used_for for log messages in
verify_appended_signature
module: Move duplicate mod_check_sig users code to mod_parse_sig
arch/powerpc/Kconfig | 11 +++++
arch/powerpc/kexec/elf_64.c | 14 ++++++
arch/s390/kernel/machine_kexec_file.c | 42 ++----------------
crypto/asymmetric_keys/asymmetric_type.c | 1 +
include/linux/module_signature.h | 1 +
include/linux/verification.h | 4 ++
kernel/module-internal.h | 2 -
kernel/module.c | 12 +++--
kernel/module_signature.c | 56 +++++++++++++++++++++++-
kernel/module_signing.c | 33 +++++++-------
security/integrity/ima/ima_modsig.c | 22 ++--------
11 files changed, 113 insertions(+), 85 deletions(-)
This is resend of the KEXEC_SIG patchset.
The first patch is new because it'a a cleanup that does not require any
change to the module verification code.
The second patch is the only one that is intended to change any
functionality.
The rest only deduplicates code but I did not receive any review on that
part so I don't know if it's desirable as implemented.
The first two patches can be applied separately without the rest.
Patch 2 fails to apply on v5.16-rc4. Can you please also include git
tree/branch while posting the patches ?
Secondly, I see that you add the powerpc support in Patch 2 and then
modify it again in Patch 5 after cleanup. Why not add the support for
powerpc after the clean up ? This will reduce some rework and also
probably simplify patches.
Thanks & Regards,
- Nayna
From: Michal Suchánek <hidden> Date: 2021-12-09 14:57:51
Hello,
On Wed, Dec 08, 2021 at 08:50:54PM -0500, Nayna wrote:
On 11/25/21 13:02, Michal Suchanek wrote:
quoted
Hello,
Hi Michael,
quoted
This is resend of the KEXEC_SIG patchset.
The first patch is new because it'a a cleanup that does not require any
change to the module verification code.
The second patch is the only one that is intended to change any
functionality.
The rest only deduplicates code but I did not receive any review on that
part so I don't know if it's desirable as implemented.
The first two patches can be applied separately without the rest.
Patch 2 fails to apply on v5.16-rc4. Can you please also include git
tree/branch while posting the patches ?
Secondly, I see that you add the powerpc support in Patch 2 and then modify
it again in Patch 5 after cleanup. Why not add the support for powerpc after
the clean up ? This will reduce some rework and also probably simplify
patches.
That's because I don't know if the later patches will be accepted. By
queueing this patch first it can be applied standalone to ppc tree
without regard for the other patches. It's a copy of the s390 code so it
needs the same rework - not really adding complexity.
Thanks
Michal
Resending my last response as looks like it didn't go through mailing
list because of some wrong formatting. My apologies to those who are
receiving it twice.
Since powerpc also supports IMA_ARCH_POLICY for kernel image signature
verification, please include the following:
"An alternative implementation for the powerpc arch is IMA_ARCH_POLICY.
It verifies the appended kernel image signature and additionally
includes both the signed and unsigned file hashes in the IMA measurement
list, extends the IMA PCR in the TPM, and prevents blacklisted binary
kernel images from being kexec'd."
Thanks & Regards,
- Nayna
From: Michal Suchánek <hidden> Date: 2021-12-13 18:06:22
Hello,
On Tue, Dec 07, 2021 at 05:10:34PM +0100, Philipp Rudo wrote:
Hi Michal,
On Thu, 25 Nov 2021 19:02:44 +0100
Michal Suchanek [off-list ref] wrote:
quoted
Multiple users of mod_check_sig check for the marker, then call
mod_check_sig, extract signature length, and remove the signature.
Put this code in one place together with mod_check_sig.
Signed-off-by: Michal Suchanek <redacted>
---
include/linux/module_signature.h | 1 +
kernel/module_signature.c | 56 ++++++++++++++++++++++++++++-
kernel/module_signing.c | 26 +++-----------
security/integrity/ima/ima_modsig.c | 22 ++----------
4 files changed, 63 insertions(+), 42 deletions(-)
I personally don't like it when a function has a "check" in it's name
as it doesn't describe what the function is checking for. For me
It is consistent with mod_check_sig
mod_has_sig_marker is much more precise. I would use that instead.
It actually would not because it does more than that.
Thanks
Michal
Thanks
Philipp
quoted
+{
+ const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+
+ if (markerlen > *len)
+ return -ENODATA;
+
+ if (memcmp(data + *len - markerlen, MODULE_SIG_STRING,
+ markerlen))
+ return -ENODATA;
+
+ *len -= markerlen;
+ return 0;
+}
+
/**
* mod_check_sig - check that the given signature is sane
*
* @ms: Signature to check.
- * @file_len: Size of the file to which @ms is appended.
+ * @file_len: Size of the file to which @ms is appended (without the marker).
* @name: What is being checked. Used for error messages.
*/
int mod_check_sig(const struct module_signature *ms, size_t file_len,
@@ -44,3 +66,35 @@ int mod_check_sig(const struct module_signature *ms, size_t file_len, return 0; }++/**+ * mod_parse_sig - check that the given signature is sane and determine signature length+ *+ * @data: Data with appended signature.+ * @len: Length of data. Signature and marker length is subtracted on success.+ * @sig_len: Length of signature. Filled on success.+ * @name: What is being checked. Used for error messages.+ */+int mod_parse_sig(const void *data, size_t *len, size_t *sig_len, const char *name)+{+ const struct module_signature *sig;+ int rc;++ rc = mod_check_sig_marker(data, len);+ if (rc)+ return rc;++ if (*len < sizeof(*sig))+ return -ENODATA;++ sig = (const struct module_signature *)(data + (*len - sizeof(*sig)));++ rc = mod_check_sig(sig, *len, name);+ if (rc)+ return rc;++ *sig_len = be32_to_cpu(sig->sig_len);+ *len -= *sig_len + sizeof(*sig);++ return 0;+}
@@ -37,33 +37,17 @@ struct modsig {**Return:0onsuccess,errorcodeotherwise.*/-intima_read_modsig(enumima_hooksfunc,constvoid*buf,loff_tbuf_len,+intima_read_modsig(enumima_hooksfunc,constvoid*buf,loff_tlen,structmodsig**modsig){-constsize_tmarker_len=strlen(MODULE_SIG_STRING);-conststructmodule_signature*sig;structmodsig*hdr;-size_tsig_len;-constvoid*p;+size_tsig_len,buf_len=len;intrc;-if(buf_len<=marker_len+sizeof(*sig))-return-ENOENT;--p=buf+buf_len-marker_len;-if(memcmp(p,MODULE_SIG_STRING,marker_len))-return-ENOENT;--buf_len-=marker_len;-sig=(conststructmodule_signature*)(p-sizeof(*sig));--rc=mod_check_sig(sig,buf_len,func_tokens[func]);+rc=mod_parse_sig(buf,&buf_len,&sig_len,func_tokens[func]);if(rc)returnrc;-sig_len=be32_to_cpu(sig->sig_len);-buf_len-=sig_len+sizeof(*sig);-/* Allocate sig_len additional bytes to hold the raw PKCS#7 data. */hdr=kzalloc(sizeof(*hdr)+sig_len,GFP_KERNEL);if(!hdr)
Resending my last response as looks like it didn't go through mailing list
because of some wrong formatting. My apologies to those who are receiving it
twice.
Since powerpc also supports IMA_ARCH_POLICY for kernel image signature
verification, please include the following:
"An alternative implementation for the powerpc arch is IMA_ARCH_POLICY. It
verifies the appended kernel image signature and additionally includes both
the signed and unsigned file hashes in the IMA measurement list, extends the
IMA PCR in the TPM, and prevents blacklisted binary kernel images from being
kexec'd."
It also does blacklist based on the file hash?
There is a downstream patch that adds the support for the module
signatures, and when the code is reused for KEXEC_SIG the blacklist
also applies to it.
Which kind of shows that people really want to use the IMA features but
with no support on some major architectures it's not going to work.
Thanks
Michal