Re: [PATCH v2 2/2] integrity: convert digsig to akcipher api
On Sat, 2015-12-12 at 18:26 -0800, Tadeusz Struk wrote: > Convert asymmetric_verify to akcipher api. > > Signed-off-by: Tadeusz Struk > --- > security/integrity/Kconfig |1 + > security/integrity/digsig_asymmetric.c | 10 +++--- > 2 files changed, 4 insertions(+), 7 deletions(-) > > diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig > index 73c457b..f0b2463 100644 > --- a/security/integrity/Kconfig > +++ b/security/integrity/Kconfig > @@ -36,6 +36,7 @@ config INTEGRITY_ASYMMETRIC_KEYS > select ASYMMETRIC_KEY_TYPE > select ASYMMETRIC_PUBLIC_KEY_SUBTYPE > select PUBLIC_KEY_ALGO_RSA > +select CRYPTO_RSA > select X509_CERTIFICATE_PARSER > help > This option enables digital signature verification using > diff --git a/security/integrity/digsig_asymmetric.c > b/security/integrity/digsig_asymmetric.c > index 4fec181..5629372 100644 > --- a/security/integrity/digsig_asymmetric.c > +++ b/security/integrity/digsig_asymmetric.c > @@ -92,13 +92,9 @@ int asymmetric_verify(struct key *keyring, const char *sig, > pks.pkey_hash_algo = hdr->hash_algo; > pks.digest = (u8 *)data; > pks.digest_size = datalen; > - pks.nr_mpi = 1; > - pks.rsa.s = mpi_read_raw_data(hdr->sig, siglen); > - > - if (pks.rsa.s) > - ret = verify_signature(key, &pks); > - > - mpi_free(pks.rsa.s); > + pks.s = hdr->sig; Thanks! With this change, my system is now able to boot. Mimi > + pks.s_size = siglen; > + ret = verify_signature(key, &pks); > key_put(key); > pr_debug("%s() = %d\n", __func__, ret); > return ret; -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] keys, trusted: fix: *do not* allow duplicate key options
On Sun, 2015-12-13 at 17:42 +0200, Jarkko Sakkinen wrote: > The trusted keys option parsing allows specifying the same option > multiple times. The last option value specified is used. > > This can be seen as a regression because: > > * No gain. > * Could be problematic if there is be options dependent on other > options. Thanks, Jarkko. Although it should be obvious that patch limits the number of times an option can be specified, you should explicitly mention it in the patch description. Mimi > Reported-by: James Morris James Morris > Signed-off-by: Jarkko Sakkinen > --- > security/keys/trusted.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/security/keys/trusted.c b/security/keys/trusted.c > index 903dace..7c183c7 100644 > --- a/security/keys/trusted.c > +++ b/security/keys/trusted.c > @@ -736,11 +736,14 @@ static int getoptions(char *c, struct > trusted_key_payload *pay, > int res; > unsigned long handle; > unsigned long lock; > + unsigned long token_mask = 0; > > while ((p = strsep(&c, " \t"))) { > if (*p == '\0' || *p == ' ' || *p == '\t') > continue; > token = match_token(p, key_tokens, args); > + if (test_and_set_bit(token, &token_mask)) > + return -EINVAL; > > switch (token) { > case Opt_pcrinfo: -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] keys, trusted: seal with a TPM2 authorization policy
On Sun, 2015-12-13 at 17:42 +0200, Jarkko Sakkinen wrote: > TPM2 supports authorization policies, which are essentially > combinational logic statements repsenting the conditions where the data > can be unsealed based on the TPM state. This patch enables to use > authorization policies to seal trusted keys. > > Two following new options have been added for trusted keys: > > * 'policydigest=': provide an auth policy digest for sealing. > * 'policyhandle=': provide a policy session handle for unsealing. > > If 'hash=' option is supplied after 'policydigest=' option, this > will result an error because the state of the option would become > mixed. > > Signed-off-by: Jarkko Sakkinen > Tested-by: Colin Ian King > --- > Documentation/security/keys-trusted-encrypted.txt | 34 > +-- > drivers/char/tpm/tpm2-cmd.c | 24 +--- > include/keys/trusted-type.h | 4 +++ > security/keys/trusted.c | 26 + > 4 files changed, 70 insertions(+), 18 deletions(-) > > diff --git a/Documentation/security/keys-trusted-encrypted.txt > b/Documentation/security/keys-trusted-encrypted.txt > index fd2565b..324ddf5 100644 > --- a/Documentation/security/keys-trusted-encrypted.txt > +++ b/Documentation/security/keys-trusted-encrypted.txt > @@ -27,20 +27,26 @@ Usage: > keyctl print keyid > > options: > - keyhandle= ascii hex value of sealing key default 0x4000 (SRK) > - keyauth=ascii hex auth for sealing key default 0x00...i > - (40 ascii zeros) > - blobauth= ascii hex auth for sealed data default 0x00... > - (40 ascii zeros) > - blobauth= ascii hex auth for sealed data default 0x00... > - (40 ascii zeros) > - pcrinfo=ascii hex of PCR_INFO or PCR_INFO_LONG (no default) > - pcrlock=pcr number to be extended to "lock" blob > - migratable= 0|1 indicating permission to reseal to new PCR values, > - default 1 (resealing allowed) > - hash= hash algorithm name as a string. For TPM 1.x the only > - allowed value is sha1. For TPM 2.x the allowed values > - are sha1, sha256, sha384, sha512 and sm3-256. > + keyhandle=ascii hex value of sealing key default 0x4000 (SRK) > + keyauth= ascii hex auth for sealing key default 0x00...i > + (40 ascii zeros) > + blobauth= ascii hex auth for sealed data default 0x00... > + (40 ascii zeros) > + blobauth= ascii hex auth for sealed data default 0x00... > + (40 ascii zeros) > + pcrinfo= ascii hex of PCR_INFO or PCR_INFO_LONG (no default) > + pcrlock= pcr number to be extended to "lock" blob > + migratable= 0|1 indicating permission to reseal to new PCR values, > + default 1 (resealing allowed) > + hash= hash algorithm name as a string. For TPM 1.x the only > + allowed value is sha1. For TPM 2.x the allowed values > + are sha1, sha256, sha384, sha512 and sm3-256. > + policydigest= digest for the authorization policy. must be calculated > + with the same hash algorithm as specified by the 'hash=' > + option. > + policyhandle= handle to an authorization policy session that defines > the > + same policy and with the same hash algorithm as was > used to > + seal the key. > > "keyctl print" returns an ascii hex copy of the sealed key, which is in > standard > TPM_STORED_DATA format. The key length for new keys are always in bytes. > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > index d9d0822..45a6340 100644 > --- a/drivers/char/tpm/tpm2-cmd.c > +++ b/drivers/char/tpm/tpm2-cmd.c > @@ -478,12 +478,26 @@ int tpm2_seal_trusted(struct tpm_chip *chip, > tpm_buf_append_u8(&buf, payload->migratable); > > /* public */ > - tpm_buf_append_u16(&buf, 14); > + if (options->policydigest) > + tpm_buf_append_u16(&buf, 14 + options->digest_len); > + else > + tpm_buf_append_u16(&buf, 14); > > tpm_buf_append_u16(&buf, TPM2_ALG_KEYEDHASH); > tpm_buf_append_u16(&buf, hash); > - tpm_buf_append_u32(&buf, TPM2_ATTR_USER_WITH_AUTH); > - tpm_buf_append_u16(&buf, 0); /* policy digest size */ > + > + /* policy */ > + if (options->policydigest) { > + tpm_buf_append_u32(&buf, 0); > + tpm_buf_append_u16(&buf, options->digest_len); > + tpm_buf_append(&buf, options->policydigest, > +options->digest_len); > + } else { > + tpm_buf_append_u32(&buf, TPM2_ATTR_USER_WITH_AUTH); > + tpm_buf_append_u16(&buf, 0); > + } > + > + /* public par
Re: [PATCH v2 1/3] keys, trusted: fix: *do not* allow duplicate key options
On Mon, Dec 14, 2015 at 08:46:33AM -0500, Mimi Zohar wrote: > On Sun, 2015-12-13 at 17:42 +0200, Jarkko Sakkinen wrote: > > The trusted keys option parsing allows specifying the same option > > multiple times. The last option value specified is used. > > > > This can be seen as a regression because: > > > > * No gain. > > * Could be problematic if there is be options dependent on other > > options. > > Thanks, Jarkko. Although it should be obvious that patch limits the > number of times an option can be specified, you should explicitly > mention it in the patch description. OK, I'll update the commit message with this information before I send the pull request. Thanks for the advice! > Mimi /Jarkko > > > Reported-by: James Morris James Morris > > Signed-off-by: Jarkko Sakkinen > > --- > > security/keys/trusted.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/security/keys/trusted.c b/security/keys/trusted.c > > index 903dace..7c183c7 100644 > > --- a/security/keys/trusted.c > > +++ b/security/keys/trusted.c > > @@ -736,11 +736,14 @@ static int getoptions(char *c, struct > > trusted_key_payload *pay, > > int res; > > unsigned long handle; > > unsigned long lock; > > + unsigned long token_mask = 0; > > > > while ((p = strsep(&c, " \t"))) { > > if (*p == '\0' || *p == ' ' || *p == '\t') > > continue; > > token = match_token(p, key_tokens, args); > > + if (test_and_set_bit(token, &token_mask)) > > + return -EINVAL; > > > > switch (token) { > > case Opt_pcrinfo: > > > -- > To unsubscribe from this list: send the line "unsubscribe > linux-security-module" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] keys, trusted: seal with a TPM2 authorization policy
On Mon, Dec 14, 2015 at 08:49:00AM -0500, Mimi Zohar wrote: > On Sun, 2015-12-13 at 17:42 +0200, Jarkko Sakkinen wrote: > > TPM2 supports authorization policies, which are essentially > > combinational logic statements repsenting the conditions where the data > > can be unsealed based on the TPM state. This patch enables to use > > authorization policies to seal trusted keys. > > > > Two following new options have been added for trusted keys: > > > > * 'policydigest=': provide an auth policy digest for sealing. > > * 'policyhandle=': provide a policy session handle for unsealing. > > > > If 'hash=' option is supplied after 'policydigest=' option, this > > will result an error because the state of the option would become > > mixed. > > > > Signed-off-by: Jarkko Sakkinen > > Tested-by: Colin Ian King > > --- > > Documentation/security/keys-trusted-encrypted.txt | 34 > > +-- > > drivers/char/tpm/tpm2-cmd.c | 24 +--- > > include/keys/trusted-type.h | 4 +++ > > security/keys/trusted.c | 26 + > > 4 files changed, 70 insertions(+), 18 deletions(-) > > > > diff --git a/Documentation/security/keys-trusted-encrypted.txt > > b/Documentation/security/keys-trusted-encrypted.txt > > index fd2565b..324ddf5 100644 > > --- a/Documentation/security/keys-trusted-encrypted.txt > > +++ b/Documentation/security/keys-trusted-encrypted.txt > > @@ -27,20 +27,26 @@ Usage: > > keyctl print keyid > > > > options: > > - keyhandle= ascii hex value of sealing key default 0x4000 (SRK) > > - keyauth= ascii hex auth for sealing key default 0x00...i > > - (40 ascii zeros) > > - blobauth= ascii hex auth for sealed data default 0x00... > > - (40 ascii zeros) > > - blobauth= ascii hex auth for sealed data default 0x00... > > - (40 ascii zeros) > > - pcrinfo= ascii hex of PCR_INFO or PCR_INFO_LONG (no default) > > - pcrlock= pcr number to be extended to "lock" blob > > - migratable= 0|1 indicating permission to reseal to new PCR values, > > - default 1 (resealing allowed) > > - hash= hash algorithm name as a string. For TPM 1.x the only > > - allowed value is sha1. For TPM 2.x the allowed values > > - are sha1, sha256, sha384, sha512 and sm3-256. > > + keyhandle=ascii hex value of sealing key default 0x4000 > > (SRK) > > + keyauth= ascii hex auth for sealing key default 0x00...i > > + (40 ascii zeros) > > + blobauth= ascii hex auth for sealed data default 0x00... > > + (40 ascii zeros) > > + blobauth= ascii hex auth for sealed data default 0x00... > > + (40 ascii zeros) > > + pcrinfo= ascii hex of PCR_INFO or PCR_INFO_LONG (no default) > > + pcrlock= pcr number to be extended to "lock" blob > > + migratable= 0|1 indicating permission to reseal to new PCR values, > > + default 1 (resealing allowed) > > + hash= hash algorithm name as a string. For TPM 1.x the only > > + allowed value is sha1. For TPM 2.x the allowed values > > + are sha1, sha256, sha384, sha512 and sm3-256. > > + policydigest= digest for the authorization policy. must be > > calculated > > + with the same hash algorithm as specified by the > > 'hash=' > > + option. > > + policyhandle= handle to an authorization policy session that > > defines the > > + same policy and with the same hash algorithm as was > > used to > > + seal the key. > > > > "keyctl print" returns an ascii hex copy of the sealed key, which is in > > standard > > TPM_STORED_DATA format. The key length for new keys are always in bytes. > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > > index d9d0822..45a6340 100644 > > --- a/drivers/char/tpm/tpm2-cmd.c > > +++ b/drivers/char/tpm/tpm2-cmd.c > > @@ -478,12 +478,26 @@ int tpm2_seal_trusted(struct tpm_chip *chip, > > tpm_buf_append_u8(&buf, payload->migratable); > > > > /* public */ > > - tpm_buf_append_u16(&buf, 14); > > + if (options->policydigest) > > + tpm_buf_append_u16(&buf, 14 + options->digest_len); > > + else > > + tpm_buf_append_u16(&buf, 14); > > > > tpm_buf_append_u16(&buf, TPM2_ALG_KEYEDHASH); > > tpm_buf_append_u16(&buf, hash); > > - tpm_buf_append_u32(&buf, TPM2_ATTR_USER_WITH_AUTH); > > - tpm_buf_append_u16(&buf, 0); /* policy digest size */ > > + > > + /* policy */ > > + if (options->policydigest) { > > + tpm_buf_append_u32(&buf, 0); > > + tpm_buf_append_u16(&buf, options->digest_len); > > + tpm_buf_append(&buf, options->policydigest,
Re: [PATCH v2 2/2] integrity: convert digsig to akcipher api
On 12/14/2015 05:24 AM, Mimi Zohar wrote: > On Sat, 2015-12-12 at 18:26 -0800, Tadeusz Struk wrote: >> Convert asymmetric_verify to akcipher api. >> >> Signed-off-by: Tadeusz Struk >> --- >> security/integrity/Kconfig |1 + >> security/integrity/digsig_asymmetric.c | 10 +++--- >> 2 files changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig >> index 73c457b..f0b2463 100644 >> --- a/security/integrity/Kconfig >> +++ b/security/integrity/Kconfig >> @@ -36,6 +36,7 @@ config INTEGRITY_ASYMMETRIC_KEYS >> select ASYMMETRIC_KEY_TYPE >> select ASYMMETRIC_PUBLIC_KEY_SUBTYPE >> select PUBLIC_KEY_ALGO_RSA >> +select CRYPTO_RSA >> select X509_CERTIFICATE_PARSER >> help >>This option enables digital signature verification using >> diff --git a/security/integrity/digsig_asymmetric.c >> b/security/integrity/digsig_asymmetric.c >> index 4fec181..5629372 100644 >> --- a/security/integrity/digsig_asymmetric.c >> +++ b/security/integrity/digsig_asymmetric.c >> @@ -92,13 +92,9 @@ int asymmetric_verify(struct key *keyring, const char >> *sig, >> pks.pkey_hash_algo = hdr->hash_algo; >> pks.digest = (u8 *)data; >> pks.digest_size = datalen; >> -pks.nr_mpi = 1; >> -pks.rsa.s = mpi_read_raw_data(hdr->sig, siglen); >> - >> -if (pks.rsa.s) >> -ret = verify_signature(key, &pks); >> - >> -mpi_free(pks.rsa.s); >> +pks.s = hdr->sig; > > Thanks! With this change, my system is now able to boot. > Thanks Mimi. David, do you have any comments to this patch set? If not could you ACK please. Thanks, -- TS -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Exposing secid to secctx mapping to user-space
On Sun, Dec 13, 2015 at 5:06 PM, Paul Moore wrote: > On Friday, December 11, 2015 05:14:38 PM Stephen Smalley wrote: >> Perhaps we could provide a new fixed-size tokenized version of the >> security context string for export to userspace that could be embedded >> in the binder transaction structure? This could avoid both the >> limitations of the current secid (e.g. limited to 32 bits, no >> stackability) and the overhead of copying context strings on every IPC. > > On Friday, December 11, 2015 04:24:48 PM Casey Schaufler wrote: >> How about this: Provide an alias mechanism for secctx. There would then >> be a secid (32bits) a secctx (arbitrary text string) and a secalias which >> could be a limited string of some length. You could use the alias in place >> of the secctx anywhere you liked. > > My initial reaction to the secalias idea isn't overly positive. It seems like > a kludge with a lot of duplication, both in terms of code and concept, and a > lot of risk for confusion both by users and policy writers. I think if we > really wanted to limit the security label string format to a small size we > should have done that from the start, it's too late now. > > Assuming we see some binder performance numbers, and the numbers are bad, I'm > a little more open to doing something with the secid token. Up to this point > we haven't made any guarantees about the token and we haven't exported it > outside the kernel so there is some ability to change it to fit our needs. > Granted, this isn't perfect solution either, and perhaps ultimately we would > need something else, but I think it is worth looking into this first before we > introduce another string label. Agreed here. I can definitely see a use for security identifier tokens in SE Postgres as well. Ideally these tokens would be 32 bit uints as opposed to shorter string aliases. --Mike > > -- > paul moore > www.paul-moore.com > > ___ > Selinux mailing list > seli...@tycho.nsa.gov > To unsubscribe, send email to selinux-le...@tycho.nsa.gov. > To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov. -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Exposing secid to secctx mapping to user-space
On 12/14/2015 9:03 AM, Mike Palmiotto wrote: > On Sun, Dec 13, 2015 at 5:06 PM, Paul Moore wrote: >> On Friday, December 11, 2015 05:14:38 PM Stephen Smalley wrote: >>> Perhaps we could provide a new fixed-size tokenized version of the >>> security context string for export to userspace that could be embedded >>> in the binder transaction structure? This could avoid both the >>> limitations of the current secid (e.g. limited to 32 bits, no >>> stackability) and the overhead of copying context strings on every IPC. >> On Friday, December 11, 2015 04:24:48 PM Casey Schaufler wrote: >>> How about this: Provide an alias mechanism for secctx. There would then >>> be a secid (32bits) a secctx (arbitrary text string) and a secalias which >>> could be a limited string of some length. You could use the alias in place >>> of the secctx anywhere you liked. >> My initial reaction to the secalias idea isn't overly positive. It seems >> like >> a kludge with a lot of duplication, both in terms of code and concept, and a >> lot of risk for confusion both by users and policy writers. I think if we >> really wanted to limit the security label string format to a small size we >> should have done that from the start, it's too late now. >> >> Assuming we see some binder performance numbers, and the numbers are bad, I'm >> a little more open to doing something with the secid token. Up to this point >> we haven't made any guarantees about the token and we haven't exported it >> outside the kernel so there is some ability to change it to fit our needs. >> Granted, this isn't perfect solution either, and perhaps ultimately we would >> need something else, but I think it is worth looking into this first before >> we >> introduce another string label. > Agreed here. I can definitely see a use for security identifier tokens > in SE Postgres as well. Ideally these tokens would be 32 bit uints as > opposed to shorter string aliases. If you need something persistent you can't use what the kernel would provide, and if you don't you can make it up on the fly. The binder case is different (and evil) because the binder driver is letting user space make decisions on behalf of the kernel. > > --Mike > >> -- >> paul moore >> www.paul-moore.com >> >> ___ >> Selinux mailing list >> seli...@tycho.nsa.gov >> To unsubscribe, send email to selinux-le...@tycho.nsa.gov. >> To get help, send an email containing "help" to >> selinux-requ...@tycho.nsa.gov. -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Exposing secid to secctx mapping to user-space
On 12/14/2015 12:03 PM, Mike Palmiotto wrote: On Sun, Dec 13, 2015 at 5:06 PM, Paul Moore wrote: On Friday, December 11, 2015 05:14:38 PM Stephen Smalley wrote: Perhaps we could provide a new fixed-size tokenized version of the security context string for export to userspace that could be embedded in the binder transaction structure? This could avoid both the limitations of the current secid (e.g. limited to 32 bits, no stackability) and the overhead of copying context strings on every IPC. On Friday, December 11, 2015 04:24:48 PM Casey Schaufler wrote: How about this: Provide an alias mechanism for secctx. There would then be a secid (32bits) a secctx (arbitrary text string) and a secalias which could be a limited string of some length. You could use the alias in place of the secctx anywhere you liked. My initial reaction to the secalias idea isn't overly positive. It seems like a kludge with a lot of duplication, both in terms of code and concept, and a lot of risk for confusion both by users and policy writers. I think if we really wanted to limit the security label string format to a small size we should have done that from the start, it's too late now. Assuming we see some binder performance numbers, and the numbers are bad, I'm a little more open to doing something with the secid token. Up to this point we haven't made any guarantees about the token and we haven't exported it outside the kernel so there is some ability to change it to fit our needs. Granted, this isn't perfect solution either, and perhaps ultimately we would need something else, but I think it is worth looking into this first before we introduce another string label. Agreed here. I can definitely see a use for security identifier tokens in SE Postgres as well. Ideally these tokens would be 32 bit uints as opposed to shorter string aliases. The userspace AVC provides its own SID mapping (man avc_context_to_sid, avc_has_perm), but that mapping is process-local (unlike kernel SIDs, which are global) and non-persistent (like kernel SIDs, which also aren't guaranteed to remain stable across reboot). That's what is conventionally used by userspace object managers like dbus-daemon, Xorg/XSELinux, and SE-Postgres (although IIRC SE-Postgres rolled their own custom AVC in order to optimize it specifically for their needs). Those SIDs can be used for in-core objects, but you still need to store the security context strings for persistent objects, although you can obviously store each unique one only once and maintain your own indices (ala the persistent SIDs of the Flask and original SELinux labeled filesystem implementation). -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Exposing secid to secctx mapping to user-space
On 12/13/2015 2:06 PM, Paul Moore wrote: > On Friday, December 11, 2015 05:14:38 PM Stephen Smalley wrote: >> Perhaps we could provide a new fixed-size tokenized version of the >> security context string for export to userspace that could be embedded >> in the binder transaction structure? This could avoid both the >> limitations of the current secid (e.g. limited to 32 bits, no >> stackability) and the overhead of copying context strings on every IPC. > On Friday, December 11, 2015 04:24:48 PM Casey Schaufler wrote: >> How about this: Provide an alias mechanism for secctx. There would then >> be a secid (32bits) a secctx (arbitrary text string) and a secalias which >> could be a limited string of some length. You could use the alias in place >> of the secctx anywhere you liked. > My initial reaction to the secalias idea isn't overly positive. It seems > like > a kludge with a lot of duplication, both in terms of code and concept, and a > lot of risk for confusion both by users and policy writers. I think if we > really wanted to limit the security label string format to a small size we > should have done that from the start, it's too late now. The alias would be a user space controlled mapping. The kernel code would only be involved at the border. I would never expect policy to be written using aliases. As for being a kludge, yeah, there's some of that, but I think that's true with the secid, too. > Assuming we see some binder performance numbers, and the numbers are bad, I'm > a little more open to doing something with the secid token. Up to this point > we haven't made any guarantees about the token and we haven't exported it > outside the kernel so there is some ability to change it to fit our needs. > Granted, this isn't perfect solution either, and perhaps ultimately we would > need something else, but I think it is worth looking into this first before > we > introduce another string label. I agree with getting numbers before someone dashes off to make a premature optimization that exposes secids. If the numbers are bad I would hope that the developers would look at fixing binder rather than exposing (and forever requiring) secids. -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Exposing secid to secctx mapping to user-space
> Subject: Re: Exposing secid to secctx mapping to user-space > > On 12/13/2015 2:06 PM, Paul Moore wrote: > > On Friday, December 11, 2015 05:14:38 PM Stephen Smalley wrote: > >> Perhaps we could provide a new fixed-size tokenized version of the > >> security context string for export to userspace that could be > >> embedded in the binder transaction structure? This could avoid both > >> the limitations of the current secid (e.g. limited to 32 bits, no > >> stackability) and the overhead of copying context strings on every IPC. > > On Friday, December 11, 2015 04:24:48 PM Casey Schaufler wrote: > >> How about this: Provide an alias mechanism for secctx. There would > >> then be a secid (32bits) a secctx (arbitrary text string) and a > >> secalias which could be a limited string of some length. You could > >> use the alias in place of the secctx anywhere you liked. > > My initial reaction to the secalias idea isn't overly positive. It > > seems like a kludge with a lot of duplication, both in terms of code > > and concept, and a lot of risk for confusion both by users and policy > > writers. I think if we really wanted to limit the security label > > string format to a small size we should have done that from the start, it's > > too > late now. > > The alias would be a user space controlled mapping. The kernel code would only > be involved at the border. I would never expect policy to be written using > aliases. > As for being a kludge, yeah, there's some of that, but I think that's true > with the > secid, too. > > > Assuming we see some binder performance numbers, and the numbers are > > bad, I'm a little more open to doing something with the secid token. > > Up to this point we haven't made any guarantees about the token and we > > haven't exported it outside the kernel so there is some ability to change > > it to fit > our needs. > > Granted, this isn't perfect solution either, and perhaps ultimately we > > would need something else, but I think it is worth looking into this > > first before we introduce another string label. > > I agree with getting numbers before someone dashes off to make a premature > optimization that exposes secids. If the numbers are bad I would hope that the > developers would look at fixing binder rather than exposing (and forever > requiring) secids. > If I understand correctly, the goal here is to avoid the lookup from pid to context. If we somehow Had the context or a token to a context during the ipc transaction to userspace, we could just use that In computing the access decision. If that is correct, then since we have PID, why not just extend the SE Linux compute av decision interface to support passing of PID and then it can do the lookup in the Kernel? -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Exposing secid to secctx mapping to user-space
On 12/14/2015 04:29 PM, Roberts, William C wrote: Subject: Re: Exposing secid to secctx mapping to user-space On 12/13/2015 2:06 PM, Paul Moore wrote: On Friday, December 11, 2015 05:14:38 PM Stephen Smalley wrote: Perhaps we could provide a new fixed-size tokenized version of the security context string for export to userspace that could be embedded in the binder transaction structure? This could avoid both the limitations of the current secid (e.g. limited to 32 bits, no stackability) and the overhead of copying context strings on every IPC. On Friday, December 11, 2015 04:24:48 PM Casey Schaufler wrote: How about this: Provide an alias mechanism for secctx. There would then be a secid (32bits) a secctx (arbitrary text string) and a secalias which could be a limited string of some length. You could use the alias in place of the secctx anywhere you liked. My initial reaction to the secalias idea isn't overly positive. It seems like a kludge with a lot of duplication, both in terms of code and concept, and a lot of risk for confusion both by users and policy writers. I think if we really wanted to limit the security label string format to a small size we should have done that from the start, it's too late now. The alias would be a user space controlled mapping. The kernel code would only be involved at the border. I would never expect policy to be written using aliases. As for being a kludge, yeah, there's some of that, but I think that's true with the secid, too. Assuming we see some binder performance numbers, and the numbers are bad, I'm a little more open to doing something with the secid token. Up to this point we haven't made any guarantees about the token and we haven't exported it outside the kernel so there is some ability to change it to fit our needs. Granted, this isn't perfect solution either, and perhaps ultimately we would need something else, but I think it is worth looking into this first before we introduce another string label. I agree with getting numbers before someone dashes off to make a premature optimization that exposes secids. If the numbers are bad I would hope that the developers would look at fixing binder rather than exposing (and forever requiring) secids. If I understand correctly, the goal here is to avoid the lookup from pid to context. If we somehow Had the context or a token to a context during the ipc transaction to userspace, we could just use that In computing the access decision. If that is correct, then since we have PID, why not just extend the SE Linux compute av decision interface to support passing of PID and then it can do the lookup in the Kernel? That's no less racy than getpidcon(). -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Exposing secid to secctx mapping to user-space
> > > > If I understand correctly, the goal here is to avoid the lookup from > > pid to context. If we somehow Had the context or a token to a context > > during the ipc transaction to userspace, we could just use that In > > computing the access decision. If that is correct, then since we have > > PID, why not just extend the SE Linux compute av decision interface to > > support > passing of PID and then it can do the lookup in the Kernel? > > That's no less racy than getpidcon(). > I got a bounce from when I sent this from gmail, resending. True, but in this case the binder transaction would be dead... Why not just pass ctx? It's less than ideal, but it might be good enough for now until contexts get unwieldy big. grep -rn '^type ' * | grep domain | cut -d' ' -f 2-2 | sed s/','//g | awk ' { thislen=length($0); printf("%-5s %dn", NR, thislen); totlen+=thislen} END { printf("average: %dn", totlen/NR); } ' The avg type length for domain types in external/sepolicy is 7. Add the full ctx: u:r:xxx:s0(cat) 1. We're looking at like 18 or so bytes, how do we know this won't be "fast enough"? 2. What's the current perf numbers, and what's agreed upon on what you need to hit to be fast enough? 3. I'm assuming the use case is in service manager, but would a userspace cache of AVD's help? Then you can (possibly) avoid both kernel trips, and you can invalidate the cache on policy reload and on PID deaths? In the case of service manager would it always be a miss based on usage pattern? I'm assuming things would say hey resolve this once, and be done. However, you could only do the ctx lookup and do the avd based on the policy in user space, thus avoiding 1 of two trips. -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html