Re: [Freeipa-devel] [PATCH 019] Asymmetric vault: validate public key in client

2015-08-13 Thread Petr Vobornik

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

2015-08-13 Thread Christian Heimes
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

2015-08-13 Thread Christian Heimes
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

2015-08-13 Thread Petr Vobornik

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

2015-08-13 Thread Petr Vobornik

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

2015-07-23 Thread Christian Heimes
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