Re: [PATCH v2 2/2] integrity: convert digsig to akcipher api

[Tadeusz Struk]

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, );
>> -
>> -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: [PATCH v2 3/3] keys, trusted: seal with a TPM2 authorization policy

[Mimi Zohar]

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(, payload->migratable);
> 
>   /* public */
> - tpm_buf_append_u16(, 14);
> + if (options->policydigest)
> + tpm_buf_append_u16(, 14 + options->digest_len);
> + else
> + tpm_buf_append_u16(, 14);
> 
>   tpm_buf_append_u16(, TPM2_ALG_KEYEDHASH);
>   tpm_buf_append_u16(, hash);
> - tpm_buf_append_u32(, TPM2_ATTR_USER_WITH_AUTH);
> - tpm_buf_append_u16(, 0); /* policy digest size */
> +
> + /* policy */
> + if (options->policydigest) {
> + tpm_buf_append_u32(, 0);
> + tpm_buf_append_u16(, options->digest_len);
> + tpm_buf_append(, options->policydigest,
> +options->digest_len);
> + } else {
> + tpm_buf_append_u32(, TPM2_ATTR_USER_WITH_AUTH);
> + tpm_buf_append_u16(, 0);
> + }
> +
> + /* 

Re: [PATCH v2 1/3] keys, trusted: fix: *do not* allow duplicate key options

[Jarkko Sakkinen]

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(, " \t"))) {
> > if (*p == '\0' || *p == ' ' || *p == '\t')
> > continue;
> > token = match_token(p, key_tokens, args);
> > +   if (test_and_set_bit(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 1/3] keys, trusted: fix: *do not* allow duplicate key options

[Mimi Zohar]

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(, " \t"))) {
>   if (*p == '\0' || *p == ' ' || *p == '\t')
>   continue;
>   token = match_token(p, key_tokens, args);
> + if (test_and_set_bit(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

[Jarkko Sakkinen]

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(, payload->migratable);
> > 
> > /* public */
> > -   tpm_buf_append_u16(, 14);
> > +   if (options->policydigest)
> > +   tpm_buf_append_u16(, 14 + options->digest_len);
> > +   else
> > +   tpm_buf_append_u16(, 14);
> > 
> > tpm_buf_append_u16(, TPM2_ALG_KEYEDHASH);
> > tpm_buf_append_u16(, hash);
> > -   tpm_buf_append_u32(, TPM2_ATTR_USER_WITH_AUTH);
> > -   tpm_buf_append_u16(, 0); /* policy digest size */
> > +
> > +   /* policy */
> > +   if (options->policydigest) {
> > +   tpm_buf_append_u32(, 0);
> > +   tpm_buf_append_u16(, options->digest_len);
> > +   tpm_buf_append(, 

Re: [PATCH v2 2/2] integrity: convert digsig to akcipher api

[Mimi Zohar]

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, );
> -
> - 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, );
>   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: Exposing secid to secctx mapping to user-space

[Stephen Smalley]

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

[Mike Palmiotto]

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

[Casey Schaufler]

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

[Roberts, William C]

> 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

[Roberts, William C]

> >
> > 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