Re: [PATCH v2] security/integrity: fix pointer to ESL data and its size on pseries
On Thu, 08 Jun 2023 08:04:44 -0400, Nayna Jain wrote: > On PowerVM guest, variable data is prefixed with 8 bytes of timestamp. > Extract ESL by stripping off the timestamp before passing to ESL parser. > > Applied to powerpc/next. [1/1] security/integrity: fix pointer to ESL data and its size on pseries https://git.kernel.org/powerpc/c/e66effaf61ffb1dc6088492ca3a0e98dcbf1c10d cheers
Re: [PATCH v2] security/integrity: fix pointer to ESL data and its size on pseries
On 08/06/23 5:34 pm, Nayna Jain wrote: On PowerVM guest, variable data is prefixed with 8 bytes of timestamp. Extract ESL by stripping off the timestamp before passing to ESL parser. Fixes: 4b3e71e9a34c ("integrity/powerpc: Support loading keys from PLPKS") Cc: sta...@vger.kenrnel.org # v6.3 Signed-off-by: Nayna Jain --- Tested-by: Nageswara R Sastry Seeing the keyring difference with and with out patch: With out patch: [root@ltcrain80-lp2 ~]# grep keyring /proc/keys | grep -v _ses 0131ebb4 I--Q--- 1 perm 0c03 0 65534 keyring .user_reg: 2 039dfd93 I-- 1 perm 1f0f 0 0 keyring .ima: 1 03a160b5 I-- 1 perm 1f0b 0 0 keyring .builtin_trusted_keys: 2 05377b73 I-- 1 perm 1f0b 0 0 keyring .platform: empty 0d7ea730 I-- 1 perm 0f0b 0 0 keyring .blacklist: empty 16235f2d I--Q--- 6 perm 1f3f 0 65534 keyring _uid.0: empty 1721f130 I-- 1 perm 1f0f 0 0 keyring .evm: empty With patch: [root@ltcrain80-lp2 ~]# grep keyring /proc/keys | grep -v _ses 04820159 I-- 1 perm 0f0b 0 0 keyring .blacklist: 1 16d05827 I--Q--- 1 perm 0c03 0 65534 keyring .user_reg: 2 17648d6a I-- 1 perm 1f0b 0 0 keyring .builtin_trusted_keys: 2 2158b34f I--Q--- 6 perm 1f3f 0 65534 keyring _uid.0: empty 2237eff6 I-- 1 perm 1f0f 0 0 keyring .evm: empty 26d0330c I-- 1 perm 1f0b 0 0 keyring .platform: 1 2daa48ab I-- 1 perm 1f0f 0 0 keyring .ima: 1 Thank you. Changelog: v2: Fixed feedback from Jarkko * added CC to stable * moved *data declaration to same line as *db,*dbx Renamed extract_data() macro to extract_esl() for clarity .../integrity/platform_certs/load_powerpc.c | 40 --- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c index b9de70b90826..170789dc63d2 100644 --- a/security/integrity/platform_certs/load_powerpc.c +++ b/security/integrity/platform_certs/load_powerpc.c @@ -15,6 +15,9 @@ #include "keyring_handler.h" #include "../integrity.h" +#define extract_esl(db, data, size, offset) \ + do { db = data + offset; size = size - offset; } while (0) + /* * Get a certificate list blob from the named secure variable. * @@ -55,8 +58,9 @@ static __init void *get_cert_list(u8 *key, unsigned long keylen, u64 *size) */ static int __init load_powerpc_certs(void) { - void *db = NULL, *dbx = NULL; - u64 dbsize = 0, dbxsize = 0; + void *db = NULL, *dbx = NULL, *data = NULL; + u64 dsize = 0; + u64 offset = 0; int rc = 0; ssize_t len; char buf[32]; @@ -74,38 +78,46 @@ static int __init load_powerpc_certs(void) return -ENODEV; } + if (strcmp("ibm,plpks-sb-v1", buf) == 0) + /* PLPKS authenticated variables ESL data is prefixed with 8 bytes of timestamp */ + offset = 8; + /* * Get db, and dbx. They might not exist, so it isn't an error if we * can't get them. */ - db = get_cert_list("db", 3, ); - if (!db) { + data = get_cert_list("db", 3, ); + if (!data) { pr_info("Couldn't get db list from firmware\n"); - } else if (IS_ERR(db)) { - rc = PTR_ERR(db); + } else if (IS_ERR(data)) { + rc = PTR_ERR(data); pr_err("Error reading db from firmware: %d\n", rc); return rc; } else { - rc = parse_efi_signature_list("powerpc:db", db, dbsize, + extract_esl(db, data, dsize, offset); + + rc = parse_efi_signature_list("powerpc:db", db, dsize, get_handler_for_db); if (rc) pr_err("Couldn't parse db signatures: %d\n", rc); - kfree(db); + kfree(data); } - dbx = get_cert_list("dbx", 4, ); - if (!dbx) { + data = get_cert_list("dbx", 4, ); + if (!data) { pr_info("Couldn't get dbx list from firmware\n"); - } else if (IS_ERR(dbx)) { - rc = PTR_ERR(dbx); + } else if (IS_ERR(data)) { + rc = PTR_ERR(data); pr_err("Error reading dbx from firmware: %d\n", rc); return rc; } else { - rc = parse_efi_signature_list("powerpc:dbx", dbx, dbxsize, + extract_esl(dbx, data, dsize, offset); + + rc = parse_efi_signature_list("powerpc:dbx", dbx, dsize, get_handler_for_dbx); if (rc) pr_err("Couldn't parse dbx signatures: %d\n", rc); - kfree(dbx); +
Re: [PATCH v2] security/integrity: fix pointer to ESL data and its size on pseries
On Thu Jun 8, 2023 at 3:04 PM EEST, Nayna Jain wrote: > On PowerVM guest, variable data is prefixed with 8 bytes of timestamp. > Extract ESL by stripping off the timestamp before passing to ESL parser. > > Fixes: 4b3e71e9a34c ("integrity/powerpc: Support loading keys from PLPKS") > Cc: sta...@vger.kenrnel.org # v6.3 > Signed-off-by: Nayna Jain > --- > Changelog: > v2: Fixed feedback from Jarkko > * added CC to stable > * moved *data declaration to same line as *db,*dbx > Renamed extract_data() macro to extract_esl() for clarity > .../integrity/platform_certs/load_powerpc.c | 40 --- > 1 file changed, 26 insertions(+), 14 deletions(-) > > diff --git a/security/integrity/platform_certs/load_powerpc.c > b/security/integrity/platform_certs/load_powerpc.c > index b9de70b90826..170789dc63d2 100644 > --- a/security/integrity/platform_certs/load_powerpc.c > +++ b/security/integrity/platform_certs/load_powerpc.c > @@ -15,6 +15,9 @@ > #include "keyring_handler.h" > #include "../integrity.h" > > +#define extract_esl(db, data, size, offset) \ > + do { db = data + offset; size = size - offset; } while (0) > + > /* > * Get a certificate list blob from the named secure variable. > * > @@ -55,8 +58,9 @@ static __init void *get_cert_list(u8 *key, unsigned long > keylen, u64 *size) > */ > static int __init load_powerpc_certs(void) > { > - void *db = NULL, *dbx = NULL; > - u64 dbsize = 0, dbxsize = 0; > + void *db = NULL, *dbx = NULL, *data = NULL; > + u64 dsize = 0; > + u64 offset = 0; > int rc = 0; > ssize_t len; > char buf[32]; > @@ -74,38 +78,46 @@ static int __init load_powerpc_certs(void) > return -ENODEV; > } > > + if (strcmp("ibm,plpks-sb-v1", buf) == 0) > + /* PLPKS authenticated variables ESL data is prefixed with 8 > bytes of timestamp */ > + offset = 8; > + > /* >* Get db, and dbx. They might not exist, so it isn't an error if we >* can't get them. >*/ > - db = get_cert_list("db", 3, ); > - if (!db) { > + data = get_cert_list("db", 3, ); > + if (!data) { > pr_info("Couldn't get db list from firmware\n"); > - } else if (IS_ERR(db)) { > - rc = PTR_ERR(db); > + } else if (IS_ERR(data)) { > + rc = PTR_ERR(data); > pr_err("Error reading db from firmware: %d\n", rc); > return rc; > } else { > - rc = parse_efi_signature_list("powerpc:db", db, dbsize, > + extract_esl(db, data, dsize, offset); > + > + rc = parse_efi_signature_list("powerpc:db", db, dsize, > get_handler_for_db); > if (rc) > pr_err("Couldn't parse db signatures: %d\n", rc); > - kfree(db); > + kfree(data); > } > > - dbx = get_cert_list("dbx", 4, ); > - if (!dbx) { > + data = get_cert_list("dbx", 4, ); > + if (!data) { > pr_info("Couldn't get dbx list from firmware\n"); > - } else if (IS_ERR(dbx)) { > - rc = PTR_ERR(dbx); > + } else if (IS_ERR(data)) { > + rc = PTR_ERR(data); > pr_err("Error reading dbx from firmware: %d\n", rc); > return rc; > } else { > - rc = parse_efi_signature_list("powerpc:dbx", dbx, dbxsize, > + extract_esl(dbx, data, dsize, offset); > + > + rc = parse_efi_signature_list("powerpc:dbx", dbx, dsize, > get_handler_for_dbx); > if (rc) > pr_err("Couldn't parse dbx signatures: %d\n", rc); > - kfree(dbx); > + kfree(data); > } > > return rc; > -- > 2.31.1 Acked-by: Jarkko Sakkinen BR, Jarkko
[PATCH v2] security/integrity: fix pointer to ESL data and its size on pseries
On PowerVM guest, variable data is prefixed with 8 bytes of timestamp. Extract ESL by stripping off the timestamp before passing to ESL parser. Fixes: 4b3e71e9a34c ("integrity/powerpc: Support loading keys from PLPKS") Cc: sta...@vger.kenrnel.org # v6.3 Signed-off-by: Nayna Jain --- Changelog: v2: Fixed feedback from Jarkko * added CC to stable * moved *data declaration to same line as *db,*dbx Renamed extract_data() macro to extract_esl() for clarity .../integrity/platform_certs/load_powerpc.c | 40 --- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c index b9de70b90826..170789dc63d2 100644 --- a/security/integrity/platform_certs/load_powerpc.c +++ b/security/integrity/platform_certs/load_powerpc.c @@ -15,6 +15,9 @@ #include "keyring_handler.h" #include "../integrity.h" +#define extract_esl(db, data, size, offset)\ + do { db = data + offset; size = size - offset; } while (0) + /* * Get a certificate list blob from the named secure variable. * @@ -55,8 +58,9 @@ static __init void *get_cert_list(u8 *key, unsigned long keylen, u64 *size) */ static int __init load_powerpc_certs(void) { - void *db = NULL, *dbx = NULL; - u64 dbsize = 0, dbxsize = 0; + void *db = NULL, *dbx = NULL, *data = NULL; + u64 dsize = 0; + u64 offset = 0; int rc = 0; ssize_t len; char buf[32]; @@ -74,38 +78,46 @@ static int __init load_powerpc_certs(void) return -ENODEV; } + if (strcmp("ibm,plpks-sb-v1", buf) == 0) + /* PLPKS authenticated variables ESL data is prefixed with 8 bytes of timestamp */ + offset = 8; + /* * Get db, and dbx. They might not exist, so it isn't an error if we * can't get them. */ - db = get_cert_list("db", 3, ); - if (!db) { + data = get_cert_list("db", 3, ); + if (!data) { pr_info("Couldn't get db list from firmware\n"); - } else if (IS_ERR(db)) { - rc = PTR_ERR(db); + } else if (IS_ERR(data)) { + rc = PTR_ERR(data); pr_err("Error reading db from firmware: %d\n", rc); return rc; } else { - rc = parse_efi_signature_list("powerpc:db", db, dbsize, + extract_esl(db, data, dsize, offset); + + rc = parse_efi_signature_list("powerpc:db", db, dsize, get_handler_for_db); if (rc) pr_err("Couldn't parse db signatures: %d\n", rc); - kfree(db); + kfree(data); } - dbx = get_cert_list("dbx", 4, ); - if (!dbx) { + data = get_cert_list("dbx", 4, ); + if (!data) { pr_info("Couldn't get dbx list from firmware\n"); - } else if (IS_ERR(dbx)) { - rc = PTR_ERR(dbx); + } else if (IS_ERR(data)) { + rc = PTR_ERR(data); pr_err("Error reading dbx from firmware: %d\n", rc); return rc; } else { - rc = parse_efi_signature_list("powerpc:dbx", dbx, dbxsize, + extract_esl(dbx, data, dsize, offset); + + rc = parse_efi_signature_list("powerpc:dbx", dbx, dsize, get_handler_for_dbx); if (rc) pr_err("Couldn't parse dbx signatures: %d\n", rc); - kfree(dbx); + kfree(data); } return rc; -- 2.31.1