Re: [Freeipa-devel] [PATCH 019] Asymmetric vault: validate public key in client
On 07/23/2015 08:38 PM, Christian Heimes wrote: The ipa vault commands now load the public keys in order to verify them. The validation also prevents a user from accidentally sending her private keys to the server. The patch fixes #5142 and #5142. $ ./ipa vault-add AsymmetricVault --desc Asymmetric vault --type asymmetric --public-key-file mykey.pem ipa: ERROR: invalid 'ipavaultpublickey': Invalid or unsupported vault public key: Could not unserialize key data. https://fedorahosted.org/freeipa/ticket/5142 https://fedorahosted.org/freeipa/ticket/5143 ACK as fix for 5142. I don't think that it fixes 5143. The traceback is fixed therefore 5143 doesn't occur but if there was other traceback raised by `self.api.Command.vault_archive(*args, **opts)` then the vault added in `response = self.api.Command.vault_add_internal(*args, **options)` would be still created. -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 019] Asymmetric vault: validate public key in client
On 2015-08-13 12:10, Petr Vobornik wrote: On 07/23/2015 08:38 PM, Christian Heimes wrote: The ipa vault commands now load the public keys in order to verify them. The validation also prevents a user from accidentally sending her private keys to the server. The patch fixes #5142 and #5142. $ ./ipa vault-add AsymmetricVault --desc Asymmetric vault --type asymmetric --public-key-file mykey.pem ipa: ERROR: invalid 'ipavaultpublickey': Invalid or unsupported vault public key: Could not unserialize key data. https://fedorahosted.org/freeipa/ticket/5142 https://fedorahosted.org/freeipa/ticket/5143 ACK as fix for 5142. I don't think that it fixes 5143. The traceback is fixed therefore 5143 doesn't occur but if there was other traceback raised by `self.api.Command.vault_archive(*args, **opts)` then the vault added in `response = self.api.Command.vault_add_internal(*args, **options)` would be still created. Yes, that is correct. There aren't any arguments that can lead to an exception. The arguments are either already validated by vault_add() or don't raise an error. Of course there are plenty of opportunities errors. The connection to the IPA or LDAP server could fail, NSS DB could be missing and so on. How should we handle an error in vault_archive? Is there another way then to delete the new vault all along? try: self.api.Command.vault_archive(*args, **opts) except Exception: log_error() self.api.Command.vault_del(*args, **opts) report_error() Christian signature.asc Description: OpenPGP digital signature -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 019] Asymmetric vault: validate public key in client
On 2015-08-13 14:05, Petr Vobornik wrote: On 08/13/2015 12:38 PM, Christian Heimes wrote: On 2015-08-13 12:10, Petr Vobornik wrote: On 07/23/2015 08:38 PM, Christian Heimes wrote: The ipa vault commands now load the public keys in order to verify them. The validation also prevents a user from accidentally sending her private keys to the server. The patch fixes #5142 and #5142. $ ./ipa vault-add AsymmetricVault --desc Asymmetric vault --type asymmetric --public-key-file mykey.pem ipa: ERROR: invalid 'ipavaultpublickey': Invalid or unsupported vault public key: Could not unserialize key data. https://fedorahosted.org/freeipa/ticket/5142 https://fedorahosted.org/freeipa/ticket/5143 ACK as fix for 5142. I don't think that it fixes 5143. The traceback is fixed therefore 5143 doesn't occur but if there was other traceback raised by `self.api.Command.vault_archive(*args, **opts)` then the vault added in `response = self.api.Command.vault_add_internal(*args, **options)` would be still created. Yes, that is correct. There aren't any arguments that can lead to an exception. The arguments are either already validated by vault_add() or don't raise an error. Of course there are plenty of opportunities errors. The connection to the IPA or LDAP server could fail, NSS DB could be missing and so on. How should we handle an error in vault_archive? Is there another way then to delete the new vault all along? try: self.api.Command.vault_archive(*args, **opts) except Exception: log_error() self.api.Command.vault_del(*args, **opts) report_error() Christian Imho this is the way. But it may fail because of the same root cause as vault_archive. That said I don't see #5142 as a priority and would defer it. I'd still like to see my patch for #5142 in RHEL, too. It prevents accidental exposure of private keys, too. In the test case the test uploads his private keys to the server. FreeIPA should not leak a user's private key. My patch prevents that, too. signature.asc Description: OpenPGP digital signature -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 019] Asymmetric vault: validate public key in client
On 08/13/2015 12:38 PM, Christian Heimes wrote: On 2015-08-13 12:10, Petr Vobornik wrote: On 07/23/2015 08:38 PM, Christian Heimes wrote: The ipa vault commands now load the public keys in order to verify them. The validation also prevents a user from accidentally sending her private keys to the server. The patch fixes #5142 and #5142. $ ./ipa vault-add AsymmetricVault --desc Asymmetric vault --type asymmetric --public-key-file mykey.pem ipa: ERROR: invalid 'ipavaultpublickey': Invalid or unsupported vault public key: Could not unserialize key data. https://fedorahosted.org/freeipa/ticket/5142 https://fedorahosted.org/freeipa/ticket/5143 ACK as fix for 5142. I don't think that it fixes 5143. The traceback is fixed therefore 5143 doesn't occur but if there was other traceback raised by `self.api.Command.vault_archive(*args, **opts)` then the vault added in `response = self.api.Command.vault_add_internal(*args, **options)` would be still created. Yes, that is correct. There aren't any arguments that can lead to an exception. The arguments are either already validated by vault_add() or don't raise an error. Of course there are plenty of opportunities errors. The connection to the IPA or LDAP server could fail, NSS DB could be missing and so on. How should we handle an error in vault_archive? Is there another way then to delete the new vault all along? try: self.api.Command.vault_archive(*args, **opts) except Exception: log_error() self.api.Command.vault_del(*args, **opts) report_error() Christian Imho this is the way. But it may fail because of the same root cause as vault_archive. That said I don't see #5142 as a priority and would defer it. -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 019] Asymmetric vault: validate public key in client
On 08/13/2015 02:12 PM, Christian Heimes wrote: On 2015-08-13 14:05, Petr Vobornik wrote: On 08/13/2015 12:38 PM, Christian Heimes wrote: On 2015-08-13 12:10, Petr Vobornik wrote: On 07/23/2015 08:38 PM, Christian Heimes wrote: The ipa vault commands now load the public keys in order to verify them. The validation also prevents a user from accidentally sending her private keys to the server. The patch fixes #5142 and #5142. $ ./ipa vault-add AsymmetricVault --desc Asymmetric vault --type asymmetric --public-key-file mykey.pem ipa: ERROR: invalid 'ipavaultpublickey': Invalid or unsupported vault public key: Could not unserialize key data. https://fedorahosted.org/freeipa/ticket/5142 https://fedorahosted.org/freeipa/ticket/5143 ACK as fix for 5142. I don't think that it fixes 5143. The traceback is fixed therefore 5143 doesn't occur but if there was other traceback raised by `self.api.Command.vault_archive(*args, **opts)` then the vault added in `response = self.api.Command.vault_add_internal(*args, **options)` would be still created. Yes, that is correct. There aren't any arguments that can lead to an exception. The arguments are either already validated by vault_add() or don't raise an error. Of course there are plenty of opportunities errors. The connection to the IPA or LDAP server could fail, NSS DB could be missing and so on. How should we handle an error in vault_archive? Is there another way then to delete the new vault all along? try: self.api.Command.vault_archive(*args, **opts) except Exception: log_error() self.api.Command.vault_del(*args, **opts) report_error() Christian Imho this is the way. But it may fail because of the same root cause as vault_archive. That said I don't see #5142 as a priority and would defer it. I'd still like to see my patch for #5142 in RHEL, too. It prevents accidental exposure of private keys, too. In the test case the test uploads his private keys to the server. FreeIPA should not leak a user's private key. My patch prevents that, too. Sorry, wrong number. I meant defer #5143. The patch for #5142 is OK. Pushed to: master: e4dff25838f7a2342779851bd68460080d77683b ipa-4-2: 06d68b447ff62b64a3a6e4a3c565fcf406a457ec -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [PATCH 019] Asymmetric vault: validate public key in client
The ipa vault commands now load the public keys in order to verify them. The validation also prevents a user from accidentally sending her private keys to the server. The patch fixes #5142 and #5142. $ ./ipa vault-add AsymmetricVault --desc Asymmetric vault --type asymmetric --public-key-file mykey.pem ipa: ERROR: invalid 'ipavaultpublickey': Invalid or unsupported vault public key: Could not unserialize key data. https://fedorahosted.org/freeipa/ticket/5142 https://fedorahosted.org/freeipa/ticket/5143 From fd380c4539fdd18a7d10786230c15a259b097af6 Mon Sep 17 00:00:00 2001 From: Christian Heimes chei...@redhat.com Date: Thu, 23 Jul 2015 20:30:21 +0200 Subject: [PATCH] Asymmetric vault: validate public key in client The ipa vault commands now load and validate the public key for asymmetric encryption, before sending it to the server. This prevents invalid vaults and prohibits accidental exposure of private key material. https://fedorahosted.org/freeipa/ticket/5142 https://fedorahosted.org/freeipa/ticket/5143 --- ipalib/plugins/vault.py | 13 + 1 file changed, 13 insertions(+) diff --git a/ipalib/plugins/vault.py b/ipalib/plugins/vault.py index 81197f9328c7ed890fa336f464bfcda475ac6189..5d493ae183da48412a38e7074b88ec0ab4402311 100644 --- a/ipalib/plugins/vault.py +++ b/ipalib/plugins/vault.py @@ -622,6 +622,19 @@ class vault_add(PKQuery, Local): name='ipavaultpublickey', error=_('Missing vault public key')) +# validate public key and prevent users from accidentally +# sending a private key to the server. +try: +load_pem_public_key( +data=public_key, +backend=default_backend() +) +except ValueError as e: +raise errors.ValidationError( +name='ipavaultpublickey', +error=_('Invalid or unsupported vault public key: %s') % e, +) + # create vault response = self.api.Command.vault_add_internal(*args, **options) -- 2.4.3 signature.asc Description: OpenPGP digital signature -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code