Re: [RFC][PATCH 0/9] encrypted keys & key control op
Mimi Zohar wrote: > > > I'm sure there is/was a good reason for add_key() to do both. > > > > Yes. No race. > > > > > > But you can't pre-search for the existence of a key and mould the > > > > payload accordingly because that means you can race against both > > > > add_key() and keyctl_unlink(). > > > > > > Would this still be the case, if you differentiated between > > > instantiating and updating a key? > > > > Yes. Imagine, you try to add a key and it gets rejected because the key > > already exists. You then try to update the existing key, but that gets > > rejected because someone unlinked the key in the meantime. So you try and > > add it again, but this now fails because someone added a new key. Repeat. > > A counter example would be two processes, having nothing to do with each > other, attempt to a create a key with the same name. Instead of each > process getting its own key, they land up sharing the same key. > Not only are they sharing the same key, but neither process knows that > there is another process using the same key. I would think this is a > bigger problem. > > Failing to create/update a key, at least to me, seems safer than having > two apps trying to create a key with same name, but instead land up > using the same key. Yes. Two keys of the same type with the same description should be able to substitute for one another and should be able to fulfil the same roll. Safety should not be an issue. > > Or add_key() could immediately displace a key someone else just added, > > leaving them with a key ID that disappeared as soon as it was returned due > > to an add/add race. > > This is a separate issue. If a key/keyring exists, a new key/keyring, > with the same name, should not be created replacing the existing > key/keyring. It should simply fail. (Removing a key/keyring first, > before creating a key/keyring of the same name, is different.) If you have a key type that's not "updateable" then you'd have to unlink it before trying to add a new one. This would give you a gap in time where the key does not exist. So, no, creating a new key with the same name *should* atomically displace an old one if it exists - if it doesn't update it instead. Note that keys have an "under construction" concept so that the core can create a partially formed key and then instantiate it at its leisure whilst using it to block those that would like to use it. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 0/9] encrypted keys key control op
Mimi Zohar zo...@linux.vnet.ibm.com wrote: I'm sure there is/was a good reason for add_key() to do both. Yes. No race. But you can't pre-search for the existence of a key and mould the payload accordingly because that means you can race against both add_key() and keyctl_unlink(). Would this still be the case, if you differentiated between instantiating and updating a key? Yes. Imagine, you try to add a key and it gets rejected because the key already exists. You then try to update the existing key, but that gets rejected because someone unlinked the key in the meantime. So you try and add it again, but this now fails because someone added a new key. Repeat. A counter example would be two processes, having nothing to do with each other, attempt to a create a key with the same name. Instead of each process getting its own key, they land up sharing the same key. Not only are they sharing the same key, but neither process knows that there is another process using the same key. I would think this is a bigger problem. Failing to create/update a key, at least to me, seems safer than having two apps trying to create a key with same name, but instead land up using the same key. Yes. Two keys of the same type with the same description should be able to substitute for one another and should be able to fulfil the same roll. Safety should not be an issue. Or add_key() could immediately displace a key someone else just added, leaving them with a key ID that disappeared as soon as it was returned due to an add/add race. This is a separate issue. If a key/keyring exists, a new key/keyring, with the same name, should not be created replacing the existing key/keyring. It should simply fail. (Removing a key/keyring first, before creating a key/keyring of the same name, is different.) If you have a key type that's not updateable then you'd have to unlink it before trying to add a new one. This would give you a gap in time where the key does not exist. So, no, creating a new key with the same name *should* atomically displace an old one if it exists - if it doesn't update it instead. Note that keys have an under construction concept so that the core can create a partially formed key and then instantiate it at its leisure whilst using it to block those that would like to use it. David -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 0/9] encrypted keys & key control op
On Mon, 2013-11-11 at 22:34 +, David Howells wrote: > Mimi Zohar wrote: > > > > Further, the existence of encrypted_update() means that add_key() will > > > sometimes get things wrong with encrypted keys (add_key() will call > > > ->update() if a matching key already exists rather than creating a new > > > key). > > > > I see. The key_type structure defines a number of methods, > > including .instantiate and .update. I would have thought that > > only .update would be allowed to update an existing key. > > That is correct. ->instantiate() is only called for a new key and ->update() > for an existing key. > > > Instantiating a new key should create a new key or fail, not update a key. > > That is subjective and depends on how you want your interface to work. If you > look upon it as the "key struct" is a box with some content and the content is > switchable, then does it matter? I think it does. (Example below) > And, yes, ->update() should probably have been called something like replace > or reinstantiate. The benefits of hindsight... I don't have a problem with the existing name. > > I'm sure there is/was a good reason for add_key() to do both. > > Yes. No race. > > > > But you can't pre-search for the existence of a key and mould the payload > > > accordingly because that means you can race against both add_key() and > > > keyctl_unlink(). > > > > Would this still be the case, if you differentiated between > > instantiating and updating a key? > > Yes. Imagine, you try to add a key and it gets rejected because the key > already exists. You then try to update the existing key, but that gets > rejected because someone unlinked the key in the meantime. So you try and add > it again, but this now fails because someone added a new key. Repeat. A counter example would be two processes, having nothing to do with each other, attempt to a create a key with the same name. Instead of each process getting its own key, they land up sharing the same key. Not only are they sharing the same key, but neither process knows that there is another process using the same key. I would think this is a bigger problem. Failing to create/update a key, at least to me, seems safer than having two apps trying to create a key with same name, but instead land up using the same key. > Or add_key() could immediately displace a key someone else just added, leaving > them with a key ID that disappeared as soon as it was returned due to an > add/add race. This is a separate issue. If a key/keyring exists, a new key/keyring, with the same name, should not be created replacing the existing key/keyring. It should simply fail. (Removing a key/keyring first, before creating a key/keyring of the same name, is different.) Mimi > The right behaviour can be argued in different ways. > > > My solution is to add a new keyctl function that allows the caller to > > > perform a type-specific operation on a key: > > > > > > long keyctl_control(key_serial_t key_id, > > > const char *command, > > > char *reply_buffer, > > > size_t reply_size); > > > > > This would then take a NUL-terminated string indicating the command and > > > arguments and potentially return a reply up to the buffer length. > > > > What is the usecase scenario that would require a reply_buffer? > > I don't want to have to add a keyctl_control2() down the line that has a > reply_buffer if this one doesn't when someone finds a use it if it's easy > enough and small enough to provide the facility here. This is aimed at being > a general interface rather than being specifically for encrypted keys. > > David > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 0/9] encrypted keys & key control op
Mimi Zohar wrote: > > The control op could also be used for other things like pushing a key > > into a TPM. > > > > What do you think? > > Trusted keys already creates a symmetric key based on the TPM RNG. > What type of key would I be interested in pushing to the TPM? What > usecase scenario would this solve? Dmitry mentioned something along these lines when I talked to him in Edinburgh. Anyway, it was just an example suggestion. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 0/9] encrypted keys & key control op
Mimi Zohar wrote: > > Further, the existence of encrypted_update() means that add_key() will > > sometimes get things wrong with encrypted keys (add_key() will call > > ->update() if a matching key already exists rather than creating a new > > key). > > I see. The key_type structure defines a number of methods, > including .instantiate and .update. I would have thought that > only .update would be allowed to update an existing key. That is correct. ->instantiate() is only called for a new key and ->update() for an existing key. > Instantiating a new key should create a new key or fail, not update a key. That is subjective and depends on how you want your interface to work. If you look upon it as the "key struct" is a box with some content and the content is switchable, then does it matter? And, yes, ->update() should probably have been called something like replace or reinstantiate. The benefits of hindsight... > I'm sure there is/was a good reason for add_key() to do both. Yes. No race. > > But you can't pre-search for the existence of a key and mould the payload > > accordingly because that means you can race against both add_key() and > > keyctl_unlink(). > > Would this still be the case, if you differentiated between > instantiating and updating a key? Yes. Imagine, you try to add a key and it gets rejected because the key already exists. You then try to update the existing key, but that gets rejected because someone unlinked the key in the meantime. So you try and add it again, but this now fails because someone added a new key. Repeat. Or add_key() could immediately displace a key someone else just added, leaving them with a key ID that disappeared as soon as it was returned due to an add/add race. The right behaviour can be argued in different ways. > > My solution is to add a new keyctl function that allows the caller to > > perform a type-specific operation on a key: > > > > long keyctl_control(key_serial_t key_id, > > const char *command, > > char *reply_buffer, > > size_t reply_size); > > > This would then take a NUL-terminated string indicating the command and > > arguments and potentially return a reply up to the buffer length. > > What is the usecase scenario that would require a reply_buffer? I don't want to have to add a keyctl_control2() down the line that has a reply_buffer if this one doesn't when someone finds a use it if it's easy enough and small enough to provide the facility here. This is aimed at being a general interface rather than being specifically for encrypted keys. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 0/9] encrypted keys & key control op
On Mon, 2013-11-04 at 16:22 +, David Howells wrote: > > The control op could also be used for other things like pushing a key > into a TPM. > > What do you think? Trusted keys already creates a symmetric key based on the TPM RNG. What type of key would I be interested in pushing to the TPM? What usecase scenario would this solve? thanks, Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 0/9] encrypted keys & key control op
On Mon, 2013-11-04 at 16:22 +, David Howells wrote: > Hi Mimi, Dmitry, > > Here's a series of patches, the last three of which attempt to fix up a > problem with encrypted keys update method. The preceding patches are fixes or > are preparatory for other changes that I want to put underneath. > > I really want to make all key types use ->preparse() to avoid a deadlock in > the garbage collector with quota recycling, but encrypted_update() requires > access to the old key - which you can't have in ->preparse() because the > keyring isn't locked (and this lock is the root of the gc deadlock anyway). Ok. > Further, the existence of encrypted_update() means that add_key() will > sometimes get things wrong with encrypted keys (add_key() will call ->update() > if a matching key already exists rather than creating a new key). I see. The key_type structure defines a number of methods, including .instantiate and .update. I would have thought that only .update would be allowed to update an existing key. Instantiating a new key should create a new key or fail, not update a key. I'm sure there is/was a good reason for add_key() to do both. > But you > can't pre-search for the existence of a key and mould the payload accordingly > because that means you can race against both add_key() and keyctl_unlink(). Would this still be the case, if you differentiated between instantiating and updating a key? > My solution is to add a new keyctl function that allows the caller to perform > a type-specific operation on a key: > > long keyctl_control(key_serial_t key_id, > const char *command, > char *reply_buffer, > size_t reply_size); > This would then take a NUL-terminated string indicating the command and > arguments and potentially return a reply up to the buffer length. What is the usecase scenario that would require a reply_buffer? > For instance: > > keyctl_control(1234, "encrypted change-master-key fred's key", NULL, 0); > > or, from the shell: > > keyctl control 1234 "encrypted change-master-key fred's key" > > (I think that requiring the command string to be prefixed with the expected > key type is probably a good idea). Agreed. Mimi > The control op could also be used for other things like pushing a key into a > TPM. > What do you think? > > David > --- > David Howells (9): > KEYS: The RSA public key algorithm needs to select MPILIB > KEYS: Provide a generic instantiation function > KEYS: struct key_preparsed_payload should have two payload pointers > KEYS: Allow expiry time to be set when preparsing a key > KEYS: Call ->free_preparse() even after ->preparse() returns an error > KEYS: Trusted: Use key preparsing > KEYS: Add a keyctl function to alter/control a key in type-dependent way > KEYS: Implement keyctl control for encrypted keys > KEYS: Fix encrypted key type update method > > > Documentation/security/keys.txt | 48 +++- > crypto/asymmetric_keys/Kconfig |1 > crypto/asymmetric_keys/asymmetric_type.c | 27 > crypto/asymmetric_keys/x509_public_key.c |2 > include/linux/key-type.h | 11 ++ > include/uapi/linux/keyctl.h |1 > security/keys/compat.c |6 + > security/keys/encrypted-keys/encrypted.c | 111 +- > security/keys/internal.h |2 > security/keys/key.c | 49 +++- > security/keys/keyctl.c | 104 > security/keys/trusted.c | 190 > ++ > 12 files changed, 385 insertions(+), 167 deletions(-) > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 0/9] encrypted keys key control op
On Mon, 2013-11-04 at 16:22 +, David Howells wrote: Hi Mimi, Dmitry, Here's a series of patches, the last three of which attempt to fix up a problem with encrypted keys update method. The preceding patches are fixes or are preparatory for other changes that I want to put underneath. I really want to make all key types use -preparse() to avoid a deadlock in the garbage collector with quota recycling, but encrypted_update() requires access to the old key - which you can't have in -preparse() because the keyring isn't locked (and this lock is the root of the gc deadlock anyway). Ok. Further, the existence of encrypted_update() means that add_key() will sometimes get things wrong with encrypted keys (add_key() will call -update() if a matching key already exists rather than creating a new key). I see. The key_type structure defines a number of methods, including .instantiate and .update. I would have thought that only .update would be allowed to update an existing key. Instantiating a new key should create a new key or fail, not update a key. I'm sure there is/was a good reason for add_key() to do both. But you can't pre-search for the existence of a key and mould the payload accordingly because that means you can race against both add_key() and keyctl_unlink(). Would this still be the case, if you differentiated between instantiating and updating a key? My solution is to add a new keyctl function that allows the caller to perform a type-specific operation on a key: long keyctl_control(key_serial_t key_id, const char *command, char *reply_buffer, size_t reply_size); This would then take a NUL-terminated string indicating the command and arguments and potentially return a reply up to the buffer length. What is the usecase scenario that would require a reply_buffer? For instance: keyctl_control(1234, encrypted change-master-key fred's key, NULL, 0); or, from the shell: keyctl control 1234 encrypted change-master-key fred's key (I think that requiring the command string to be prefixed with the expected key type is probably a good idea). Agreed. Mimi The control op could also be used for other things like pushing a key into a TPM. What do you think? David --- David Howells (9): KEYS: The RSA public key algorithm needs to select MPILIB KEYS: Provide a generic instantiation function KEYS: struct key_preparsed_payload should have two payload pointers KEYS: Allow expiry time to be set when preparsing a key KEYS: Call -free_preparse() even after -preparse() returns an error KEYS: Trusted: Use key preparsing KEYS: Add a keyctl function to alter/control a key in type-dependent way KEYS: Implement keyctl control for encrypted keys KEYS: Fix encrypted key type update method Documentation/security/keys.txt | 48 +++- crypto/asymmetric_keys/Kconfig |1 crypto/asymmetric_keys/asymmetric_type.c | 27 crypto/asymmetric_keys/x509_public_key.c |2 include/linux/key-type.h | 11 ++ include/uapi/linux/keyctl.h |1 security/keys/compat.c |6 + security/keys/encrypted-keys/encrypted.c | 111 +- security/keys/internal.h |2 security/keys/key.c | 49 +++- security/keys/keyctl.c | 104 security/keys/trusted.c | 190 ++ 12 files changed, 385 insertions(+), 167 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 0/9] encrypted keys key control op
On Mon, 2013-11-04 at 16:22 +, David Howells wrote: The control op could also be used for other things like pushing a key into a TPM. What do you think? Trusted keys already creates a symmetric key based on the TPM RNG. What type of key would I be interested in pushing to the TPM? What usecase scenario would this solve? thanks, Mimi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 0/9] encrypted keys key control op
Mimi Zohar zo...@linux.vnet.ibm.com wrote: Further, the existence of encrypted_update() means that add_key() will sometimes get things wrong with encrypted keys (add_key() will call -update() if a matching key already exists rather than creating a new key). I see. The key_type structure defines a number of methods, including .instantiate and .update. I would have thought that only .update would be allowed to update an existing key. That is correct. -instantiate() is only called for a new key and -update() for an existing key. Instantiating a new key should create a new key or fail, not update a key. That is subjective and depends on how you want your interface to work. If you look upon it as the key struct is a box with some content and the content is switchable, then does it matter? And, yes, -update() should probably have been called something like replace or reinstantiate. The benefits of hindsight... I'm sure there is/was a good reason for add_key() to do both. Yes. No race. But you can't pre-search for the existence of a key and mould the payload accordingly because that means you can race against both add_key() and keyctl_unlink(). Would this still be the case, if you differentiated between instantiating and updating a key? Yes. Imagine, you try to add a key and it gets rejected because the key already exists. You then try to update the existing key, but that gets rejected because someone unlinked the key in the meantime. So you try and add it again, but this now fails because someone added a new key. Repeat. Or add_key() could immediately displace a key someone else just added, leaving them with a key ID that disappeared as soon as it was returned due to an add/add race. The right behaviour can be argued in different ways. My solution is to add a new keyctl function that allows the caller to perform a type-specific operation on a key: long keyctl_control(key_serial_t key_id, const char *command, char *reply_buffer, size_t reply_size); This would then take a NUL-terminated string indicating the command and arguments and potentially return a reply up to the buffer length. What is the usecase scenario that would require a reply_buffer? I don't want to have to add a keyctl_control2() down the line that has a reply_buffer if this one doesn't when someone finds a use it if it's easy enough and small enough to provide the facility here. This is aimed at being a general interface rather than being specifically for encrypted keys. David -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 0/9] encrypted keys key control op
Mimi Zohar zo...@linux.vnet.ibm.com wrote: The control op could also be used for other things like pushing a key into a TPM. What do you think? Trusted keys already creates a symmetric key based on the TPM RNG. What type of key would I be interested in pushing to the TPM? What usecase scenario would this solve? Dmitry mentioned something along these lines when I talked to him in Edinburgh. Anyway, it was just an example suggestion. David -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 0/9] encrypted keys key control op
On Mon, 2013-11-11 at 22:34 +, David Howells wrote: Mimi Zohar zo...@linux.vnet.ibm.com wrote: Further, the existence of encrypted_update() means that add_key() will sometimes get things wrong with encrypted keys (add_key() will call -update() if a matching key already exists rather than creating a new key). I see. The key_type structure defines a number of methods, including .instantiate and .update. I would have thought that only .update would be allowed to update an existing key. That is correct. -instantiate() is only called for a new key and -update() for an existing key. Instantiating a new key should create a new key or fail, not update a key. That is subjective and depends on how you want your interface to work. If you look upon it as the key struct is a box with some content and the content is switchable, then does it matter? I think it does. (Example below) And, yes, -update() should probably have been called something like replace or reinstantiate. The benefits of hindsight... I don't have a problem with the existing name. I'm sure there is/was a good reason for add_key() to do both. Yes. No race. But you can't pre-search for the existence of a key and mould the payload accordingly because that means you can race against both add_key() and keyctl_unlink(). Would this still be the case, if you differentiated between instantiating and updating a key? Yes. Imagine, you try to add a key and it gets rejected because the key already exists. You then try to update the existing key, but that gets rejected because someone unlinked the key in the meantime. So you try and add it again, but this now fails because someone added a new key. Repeat. A counter example would be two processes, having nothing to do with each other, attempt to a create a key with the same name. Instead of each process getting its own key, they land up sharing the same key. Not only are they sharing the same key, but neither process knows that there is another process using the same key. I would think this is a bigger problem. Failing to create/update a key, at least to me, seems safer than having two apps trying to create a key with same name, but instead land up using the same key. Or add_key() could immediately displace a key someone else just added, leaving them with a key ID that disappeared as soon as it was returned due to an add/add race. This is a separate issue. If a key/keyring exists, a new key/keyring, with the same name, should not be created replacing the existing key/keyring. It should simply fail. (Removing a key/keyring first, before creating a key/keyring of the same name, is different.) Mimi The right behaviour can be argued in different ways. My solution is to add a new keyctl function that allows the caller to perform a type-specific operation on a key: long keyctl_control(key_serial_t key_id, const char *command, char *reply_buffer, size_t reply_size); This would then take a NUL-terminated string indicating the command and arguments and potentially return a reply up to the buffer length. What is the usecase scenario that would require a reply_buffer? I don't want to have to add a keyctl_control2() down the line that has a reply_buffer if this one doesn't when someone finds a use it if it's easy enough and small enough to provide the facility here. This is aimed at being a general interface rather than being specifically for encrypted keys. David -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 0/9] encrypted keys & key control op
Dmitry Kasatkin wrote: > I will be looking to patches today... Excellent, thanks! David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 0/9] encrypted keys & key control op
Hello David, I will be looking to patches today... - Dmitry On 04/11/13 18:22, David Howells wrote: > Hi Mimi, Dmitry, > > Here's a series of patches, the last three of which attempt to fix up a > problem with encrypted keys update method. The preceding patches are fixes or > are preparatory for other changes that I want to put underneath. > > I really want to make all key types use ->preparse() to avoid a deadlock in > the garbage collector with quota recycling, but encrypted_update() requires > access to the old key - which you can't have in ->preparse() because the > keyring isn't locked (and this lock is the root of the gc deadlock anyway). > > Further, the existence of encrypted_update() means that add_key() will > sometimes get things wrong with encrypted keys (add_key() will call ->update() > if a matching key already exists rather than creating a new key). But you > can't pre-search for the existence of a key and mould the payload accordingly > because that means you can race against both add_key() and keyctl_unlink(). > > My solution is to add a new keyctl function that allows the caller to perform > a type-specific operation on a key: > > long keyctl_control(key_serial_t key_id, > const char *command, > char *reply_buffer, > size_t reply_size); > > This would then take a NUL-terminated string indicating the command and > arguments and potentially return a reply up to the buffer length. > > For instance: > > keyctl_control(1234, "encrypted change-master-key fred's key", NULL, 0); > > or, from the shell: > > keyctl control 1234 "encrypted change-master-key fred's key" > > (I think that requiring the command string to be prefixed with the expected > key type is probably a good idea). > > The control op could also be used for other things like pushing a key into a > TPM. > > What do you think? > > David > --- > David Howells (9): > KEYS: The RSA public key algorithm needs to select MPILIB > KEYS: Provide a generic instantiation function > KEYS: struct key_preparsed_payload should have two payload pointers > KEYS: Allow expiry time to be set when preparsing a key > KEYS: Call ->free_preparse() even after ->preparse() returns an error > KEYS: Trusted: Use key preparsing > KEYS: Add a keyctl function to alter/control a key in type-dependent way > KEYS: Implement keyctl control for encrypted keys > KEYS: Fix encrypted key type update method > > > Documentation/security/keys.txt | 48 +++- > crypto/asymmetric_keys/Kconfig |1 > crypto/asymmetric_keys/asymmetric_type.c | 27 > crypto/asymmetric_keys/x509_public_key.c |2 > include/linux/key-type.h | 11 ++ > include/uapi/linux/keyctl.h |1 > security/keys/compat.c |6 + > security/keys/encrypted-keys/encrypted.c | 111 +- > security/keys/internal.h |2 > security/keys/key.c | 49 +++- > security/keys/keyctl.c | 104 > security/keys/trusted.c | 190 > ++ > 12 files changed, 385 insertions(+), 167 deletions(-) > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 0/9] encrypted keys key control op
Hello David, I will be looking to patches today... - Dmitry On 04/11/13 18:22, David Howells wrote: Hi Mimi, Dmitry, Here's a series of patches, the last three of which attempt to fix up a problem with encrypted keys update method. The preceding patches are fixes or are preparatory for other changes that I want to put underneath. I really want to make all key types use -preparse() to avoid a deadlock in the garbage collector with quota recycling, but encrypted_update() requires access to the old key - which you can't have in -preparse() because the keyring isn't locked (and this lock is the root of the gc deadlock anyway). Further, the existence of encrypted_update() means that add_key() will sometimes get things wrong with encrypted keys (add_key() will call -update() if a matching key already exists rather than creating a new key). But you can't pre-search for the existence of a key and mould the payload accordingly because that means you can race against both add_key() and keyctl_unlink(). My solution is to add a new keyctl function that allows the caller to perform a type-specific operation on a key: long keyctl_control(key_serial_t key_id, const char *command, char *reply_buffer, size_t reply_size); This would then take a NUL-terminated string indicating the command and arguments and potentially return a reply up to the buffer length. For instance: keyctl_control(1234, encrypted change-master-key fred's key, NULL, 0); or, from the shell: keyctl control 1234 encrypted change-master-key fred's key (I think that requiring the command string to be prefixed with the expected key type is probably a good idea). The control op could also be used for other things like pushing a key into a TPM. What do you think? David --- David Howells (9): KEYS: The RSA public key algorithm needs to select MPILIB KEYS: Provide a generic instantiation function KEYS: struct key_preparsed_payload should have two payload pointers KEYS: Allow expiry time to be set when preparsing a key KEYS: Call -free_preparse() even after -preparse() returns an error KEYS: Trusted: Use key preparsing KEYS: Add a keyctl function to alter/control a key in type-dependent way KEYS: Implement keyctl control for encrypted keys KEYS: Fix encrypted key type update method Documentation/security/keys.txt | 48 +++- crypto/asymmetric_keys/Kconfig |1 crypto/asymmetric_keys/asymmetric_type.c | 27 crypto/asymmetric_keys/x509_public_key.c |2 include/linux/key-type.h | 11 ++ include/uapi/linux/keyctl.h |1 security/keys/compat.c |6 + security/keys/encrypted-keys/encrypted.c | 111 +- security/keys/internal.h |2 security/keys/key.c | 49 +++- security/keys/keyctl.c | 104 security/keys/trusted.c | 190 ++ 12 files changed, 385 insertions(+), 167 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 0/9] encrypted keys key control op
Dmitry Kasatkin d.kasat...@samsung.com wrote: I will be looking to patches today... Excellent, thanks! David -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC][PATCH 0/9] encrypted keys & key control op
Hi Mimi, Dmitry, Here's a series of patches, the last three of which attempt to fix up a problem with encrypted keys update method. The preceding patches are fixes or are preparatory for other changes that I want to put underneath. I really want to make all key types use ->preparse() to avoid a deadlock in the garbage collector with quota recycling, but encrypted_update() requires access to the old key - which you can't have in ->preparse() because the keyring isn't locked (and this lock is the root of the gc deadlock anyway). Further, the existence of encrypted_update() means that add_key() will sometimes get things wrong with encrypted keys (add_key() will call ->update() if a matching key already exists rather than creating a new key). But you can't pre-search for the existence of a key and mould the payload accordingly because that means you can race against both add_key() and keyctl_unlink(). My solution is to add a new keyctl function that allows the caller to perform a type-specific operation on a key: long keyctl_control(key_serial_t key_id, const char *command, char *reply_buffer, size_t reply_size); This would then take a NUL-terminated string indicating the command and arguments and potentially return a reply up to the buffer length. For instance: keyctl_control(1234, "encrypted change-master-key fred's key", NULL, 0); or, from the shell: keyctl control 1234 "encrypted change-master-key fred's key" (I think that requiring the command string to be prefixed with the expected key type is probably a good idea). The control op could also be used for other things like pushing a key into a TPM. What do you think? David --- David Howells (9): KEYS: The RSA public key algorithm needs to select MPILIB KEYS: Provide a generic instantiation function KEYS: struct key_preparsed_payload should have two payload pointers KEYS: Allow expiry time to be set when preparsing a key KEYS: Call ->free_preparse() even after ->preparse() returns an error KEYS: Trusted: Use key preparsing KEYS: Add a keyctl function to alter/control a key in type-dependent way KEYS: Implement keyctl control for encrypted keys KEYS: Fix encrypted key type update method Documentation/security/keys.txt | 48 +++- crypto/asymmetric_keys/Kconfig |1 crypto/asymmetric_keys/asymmetric_type.c | 27 crypto/asymmetric_keys/x509_public_key.c |2 include/linux/key-type.h | 11 ++ include/uapi/linux/keyctl.h |1 security/keys/compat.c |6 + security/keys/encrypted-keys/encrypted.c | 111 +- security/keys/internal.h |2 security/keys/key.c | 49 +++- security/keys/keyctl.c | 104 security/keys/trusted.c | 190 ++ 12 files changed, 385 insertions(+), 167 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC][PATCH 0/9] encrypted keys key control op
Hi Mimi, Dmitry, Here's a series of patches, the last three of which attempt to fix up a problem with encrypted keys update method. The preceding patches are fixes or are preparatory for other changes that I want to put underneath. I really want to make all key types use -preparse() to avoid a deadlock in the garbage collector with quota recycling, but encrypted_update() requires access to the old key - which you can't have in -preparse() because the keyring isn't locked (and this lock is the root of the gc deadlock anyway). Further, the existence of encrypted_update() means that add_key() will sometimes get things wrong with encrypted keys (add_key() will call -update() if a matching key already exists rather than creating a new key). But you can't pre-search for the existence of a key and mould the payload accordingly because that means you can race against both add_key() and keyctl_unlink(). My solution is to add a new keyctl function that allows the caller to perform a type-specific operation on a key: long keyctl_control(key_serial_t key_id, const char *command, char *reply_buffer, size_t reply_size); This would then take a NUL-terminated string indicating the command and arguments and potentially return a reply up to the buffer length. For instance: keyctl_control(1234, encrypted change-master-key fred's key, NULL, 0); or, from the shell: keyctl control 1234 encrypted change-master-key fred's key (I think that requiring the command string to be prefixed with the expected key type is probably a good idea). The control op could also be used for other things like pushing a key into a TPM. What do you think? David --- David Howells (9): KEYS: The RSA public key algorithm needs to select MPILIB KEYS: Provide a generic instantiation function KEYS: struct key_preparsed_payload should have two payload pointers KEYS: Allow expiry time to be set when preparsing a key KEYS: Call -free_preparse() even after -preparse() returns an error KEYS: Trusted: Use key preparsing KEYS: Add a keyctl function to alter/control a key in type-dependent way KEYS: Implement keyctl control for encrypted keys KEYS: Fix encrypted key type update method Documentation/security/keys.txt | 48 +++- crypto/asymmetric_keys/Kconfig |1 crypto/asymmetric_keys/asymmetric_type.c | 27 crypto/asymmetric_keys/x509_public_key.c |2 include/linux/key-type.h | 11 ++ include/uapi/linux/keyctl.h |1 security/keys/compat.c |6 + security/keys/encrypted-keys/encrypted.c | 111 +- security/keys/internal.h |2 security/keys/key.c | 49 +++- security/keys/keyctl.c | 104 security/keys/trusted.c | 190 ++ 12 files changed, 385 insertions(+), 167 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
