Re: [Freeipa-devel] [PATCH] 353 Added initial vault implementation.

2015-01-26 Thread Endi Sukma Dewata
Sorry for the long delay. Attached is an updated patch addressing most 
of the concerns. I think the rest can be addressed in subsequent patches.


On 11/5/2014 4:06 AM, Petr Viktorin wrote:

ipapython/dn.py: This change is not needed. If you have a sequence of
RNDs you can do `DN(*seq)`.


This is actually needed to construct a parent DN from an existing DN:

   parent_dn = DN(dn[1:])


Right, for that you can do
 parent_dn = DN(*dn[1:])
(note the asterisk)


Fixed.


All existing plugins are actually using LDAPObject directly, not
baseldap.LDAPObject. Is there a concern doing that? I fixed the code to
do the same thing.


Your choice.
If there are many objects imported from a module I prefer importing the
module itself, but individual names work too.


For now I'll keep it consistent with the other plugins. There aren't 
that many objects imported, and it's easier to copy codes between plugins.



The pattern and pattern_errmsg for the 'cn' param don't match. Which one
is right? Shouldn't a similar check be there for parent?


This is actually copied verbatim from user's uid. Could you give some
sample values that show the problem?


These messages are misleading:

$ ipa vaultcontainer-add '$$'
ipa: ERROR: invalid 'container_name': may only include letters, numbers,
_, -, . and $
$ ipa vaultcontainer-add -- '-myvault-'
ipa: ERROR: invalid 'container_name': may only include letters, numbers,
_, -, . and $

(Note that in webUI you wouldn't need the awkward quoting and `--`)


I think ideally there should be a similar check for parent, but I
haven't figured out the proper pattern yet. I think we can do this as a
later enhancement. Or maybe we should remove the validation for cn for
now.


Well, what are the PKI limitations on ids?
Are there any best practices for the id names?


I don't think KRA itself puts any restrictions on the ID, but see the 
problem with get_active_key_info() below.



I think we want to limit the names to avoid nasty surprises later;
'[a-zA-Z0-9_.-]+' seems like a good set.
For parent you also need the slash, otherwise use the same pattern.

I think this is quite an important decision; weird names could lead to
security issues.


OK, I changed the container name to use that pattern and added slash in 
the parent's pattern.



vaultcontainer.get_dn: Why put '/' at the end of container_id, if the
empty string is ignored later anyway?


I'm still considering some options for container ID:
a) use a slash at the end for all containers (e.g. /users/)
b) don't use a trailing slash, except for root container (/)

Maybe a better way is to use an array of names internally and the slash
is only used during input/output. The problem is when calling another
command the array has to be converted into string and parsed back into
array, so it may even introduce more redundancies. I don't see a way to
pass an object parameter to a command. Any suggestion?


That would indeed be better. Hopefully you can avoid re-parsing and
re-validating by not calling nested Commands, see below.


In vaultcontainer.get_id, what's the purpose of the len() check? Did you
want dn.endswith() instead?


Yes, but my assumption is the DN will always be within the namespace
because it's only used internally, and using len() will be faster than
endswith(). The error case will never happen, but I was just making sure
there won't be an infinite loop. Maybe instead of calling the method
recursively I should just use a loop? That will avoid repetitive
validations.


Well, if you end up with representing the id as a list the problem will
largely go away, but: use an internal error (e.g. ValueError) for
internal sanity checking. The public errors are for user errors, they
are presented nicely which makes them much harder to debug. And
Invalid container DN with no indication of what the DN is isn't
helpful to the user either.


I revised the code to use endswith() and a loop instead of recursion, 
but I'm keeping the string ID for now. I changed the exception to 
ValueError and included the DN value. We can improve this later.



I sugggest writing doctests for the id manipulation methods (get_id,
get_private_id, ...) – it's not always obvious what exactly they do.


Do you mean sample code in docs? Is it still necessary if we have some
test code?


Not strictly necessary, but it better explains what the method does.


I'm not sure how to write doctests for plugin objects/commands. I don't 
see any example in other plugins. Do I need to write the API 
initialization code? What if the output depends on the authenticated 
user? I suppose we can add this later. I already added some unit tests.



vaultcontainer_add: You should use ldap backend directly. Calling
Commands is costly, most of the call is spent doing validation of what
here is already validated. You should add a function to recursively
create vault container using just the ldap client, and call that here
and in vault_add.


Hmm.. I'm not sure if we should write a 

Re: [Freeipa-devel] [PATCH] 353 Added initial vault implementation.

2014-11-05 Thread Petr Viktorin

On 11/05/2014 01:24 AM, Endi Sukma Dewata wrote:

Thanks for the review. I have some questions below. I'll post a new
patch after the issues are addressed.

On 11/4/2014 11:36 AM, Petr Viktorin wrote:

The new schema can go to 60basev3.ldif, no need for a new file.


Fixed. Also removed nsContainer as suggested by Simo.


ipalib/plugins/user.py: Make the try: block as small as possible; maybe
something like this:

  try:
  vaultcontainer_id = ...
  except errors.NotFound:
  pass
  else:
 (vaultcontainer_name, vaultcontainer_parent_id) = ...
 self.api.Command.vaultcontainer_del(...)


Actually the command that generates the NotFound exception is the last
command. The first two commands are preparing the parameters. I moved
them before the try-block.


Thanks!


ipapython/dn.py: This change is not needed. If you have a sequence of
RNDs you can do `DN(*seq)`.


This is actually needed to construct a parent DN from an existing DN:

   parent_dn = DN(dn[1:])

Note that the parameter is an array of RDNs, not a sequence of RDNs. I
don't see an existing mechanism to get a parent DN from a DN. Without
this change it would generate an error:

ValueError: tuple or list must be 2-valued, not
[ipapython.dn.RDN('cn=admin'), ipapython.dn.RDN('cn=users'),
ipapython.dn.RDN('cn=vaults'), ipapython.dn.RDN('dc=idm'),
ipapython.dn.RDN('dc=lab'), ipapython.dn.RDN('dc=bos'),
ipapython.dn.RDN('dc=redhat'), ipapython.dn.RDN('dc=com')]


Right, for that you can do
parent_dn = DN(*dn[1:])
(note the asterisk)


ipalib/plugins/vault.py:
Do not use star imports in new code; remove the line
  from ipalib.plugins.baseldap import *
and use e.g. `baseldap.LDAPObject` instead of just `LDAPObject`.
Hopefully the only star-imported used are the base classes?


I found 20 star imports in the existing plugins :/


Yes, we need to fix those someday.

http://www.freeipa.org/page/Coding_Best_Practices#Do_not_use_star_imports

https://fedorahosted.org/freeipa/ticket/2653


All existing plugins are actually using LDAPObject directly, not
baseldap.LDAPObject. Is there a concern doing that? I fixed the code to
do the same thing.


Your choice.
If there are many objects imported from a module I prefer importing the 
module itself, but individual names work too.



To make translators' jobs easier, split the help text in __doc__ into
strings that can be translated individually.
Replace every blank line by:
) + _(


Fixed.


The pattern and pattern_errmsg for the 'cn' param don't match. Which one
is right? Shouldn't a similar check be there for parent?


This is actually copied verbatim from user's uid. Could you give some
sample values that show the problem?


These messages are misleading:

$ ipa vaultcontainer-add '$$'
ipa: ERROR: invalid 'container_name': may only include letters, numbers, 
_, -, . and $

$ ipa vaultcontainer-add -- '-myvault-'
ipa: ERROR: invalid 'container_name': may only include letters, numbers, 
_, -, . and $


(Note that in webUI you wouldn't need the awkward quoting and `--`)


I think ideally there should be a similar check for parent, but I
haven't figured out the proper pattern yet. I think we can do this as a
later enhancement. Or maybe we should remove the validation for cn for now.


Well, what are the PKI limitations on ids?
Are there any best practices for the id names?
I think we want to limit the names to avoid nasty surprises later; 
'[a-zA-Z0-9_.-]+' seems like a good set.

For parent you also need the slash, otherwise use the same pattern.

I think this is quite an important decision; weird names could lead to 
security issues.



vaultcontainer.get_dn: Why put '/' at the end of container_id, if the
empty string is ignored later anyway?


I'm still considering some options for container ID:
a) use a slash at the end for all containers (e.g. /users/)
b) don't use a trailing slash, except for root container (/)

I think option (a) is more consistent, container ID always ends with
slash, although in cases like this it can be redundant. This is how the
code is currently written. We can make an exception for
vaultcontainer.get_dn() though.

Option (b) will require more careful coding in many places (e.g.
concatenations) because it needs to handle a special case for root
container.

Maybe a better way is to use an array of names internally and the slash
is only used during input/output. The problem is when calling another
command the array has to be converted into string and parsed back into
array, so it may even introduce more redundancies. I don't see a way to
pass an object parameter to a command. Any suggestion?


That would indeed be better. Hopefully you can avoid re-parsing and 
re-validating by not calling nested Commands, see below.



Nitpick: In vaultcontainer.get_dn, prefer the if statement over the
if-else expressions; it's a bit longer but more readable.


Fixed.


In vaultcontainer.get_id, what's the purpose of the len() check? Did you
want 

Re: [Freeipa-devel] [PATCH] 353 Added initial vault implementation.

2014-11-04 Thread Petr Viktorin

On 11/04/2014 07:27 AM, Endi Sukma Dewata wrote:

On 10/28/2014 5:34 PM, Endi Sukma Dewata wrote:

The NSSConnection class has to be modified not to shutdown existing
database because some of the vault clients (e.g. vault-archive and
vault-retrieve) also use a database to encrypt/decrypt the secret.


The problem is described in more detail in this ticket:
https://fedorahosted.org/freeipa/ticket/4638

The changes to the NSSConnection in the first patch caused the
installation to fail. Attached is a new patch that uses the solution
proposed by jdennis.


New patch attached. It's now using the correct OID's for the schema. It
also has been rebased on top of #352-1 and #354.


New patch attached to fix the ticket URL. It depends on #352-2 and
#354-1.


New patch attached to fix vault/container ID problems and for some
cleanups.


The new schema can go to 60basev3.ldif, no need for a new file.

ipalib/plugins/user.py: Make the try: block as small as possible; maybe 
something like this:


 try:
 vaultcontainer_id = ...
 except errors.NotFound:
 pass
 else:
(vaultcontainer_name, vaultcontainer_parent_id) = ...
self.api.Command.vaultcontainer_del(...)

ipapython/dn.py: This change is not needed. If you have a sequence of 
RNDs you can do `DN(*seq)`.



ipalib/plugins/vault.py:
Do not use star imports in new code; remove the line
 from ipalib.plugins.baseldap import *
and use e.g. `baseldap.LDAPObject` instead of just `LDAPObject`.
Hopefully the only star-imported used are the base classes?

To make translators' jobs easier, split the help text in __doc__ into 
strings that can be translated individually.

Replace every blank line by:
) + _(

The pattern and pattern_errmsg for the 'cn' param don't match. Which one 
is right? Shouldn't a similar check be there for parent?


vaultcontainer.get_dn: Why put '/' at the end of container_id, if the 
empty string is ignored later anyway?


Nitpick: In vaultcontainer.get_dn, prefer the if statement over the 
if-else expressions; it's a bit longer but more readable.


In vaultcontainer.get_id, what's the purpose of the len() check? Did you 
want dn.endswith() instead?


I sugggest writing doctests for the id manipulation methods (get_id, 
get_private_id, ...) – it's not always obvious what exactly they do.
According to the design doc, container IDs shouldn't end in '/'. If you 
did that I think the manipulation would be simpler, and it could be 
shared with the vault equivalents.


vaultcontainer_add: You should use ldap backend directly. Calling 
Commands is costly, most of the call is spent doing validation of what 
here is already validated. You should add a function to recursively 
create vault container using just the ldap client, and call that here 
and in vault_add.


You can delete a container with children; is that expected?

vault_add should complain if it does not get exactly one of data/text/in.

What's the difference between vault_add and vault_archive? I don't see 
vault_archive in the design.


It seems '/' is equivalent to '-' as far as KRA is concerned; should we 
disallow '-' in container/vault names?


You can specify an absolute id by starting it with a slash, but only in 
--parent and not in the name itself. I think this should be possible in 
the name too.


You can't include slashes in the name, so you always need to specify the 
prefix with --parent. I don't think there's a technical reason for this 
limitation.


There are no tests.

--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 353 Added initial vault implementation.

2014-11-04 Thread Endi Sukma Dewata
Thanks for the review. I have some questions below. I'll post a new 
patch after the issues are addressed.


On 11/4/2014 11:36 AM, Petr Viktorin wrote:

The new schema can go to 60basev3.ldif, no need for a new file.


Fixed. Also removed nsContainer as suggested by Simo.


ipalib/plugins/user.py: Make the try: block as small as possible; maybe
something like this:

  try:
  vaultcontainer_id = ...
  except errors.NotFound:
  pass
  else:
 (vaultcontainer_name, vaultcontainer_parent_id) = ...
 self.api.Command.vaultcontainer_del(...)


Actually the command that generates the NotFound exception is the last 
command. The first two commands are preparing the parameters. I moved 
them before the try-block.



ipapython/dn.py: This change is not needed. If you have a sequence of
RNDs you can do `DN(*seq)`.


This is actually needed to construct a parent DN from an existing DN:

  parent_dn = DN(dn[1:])

Note that the parameter is an array of RDNs, not a sequence of RDNs. I 
don't see an existing mechanism to get a parent DN from a DN. Without 
this change it would generate an error:


ValueError: tuple or list must be 2-valued, not 
[ipapython.dn.RDN('cn=admin'), ipapython.dn.RDN('cn=users'), 
ipapython.dn.RDN('cn=vaults'), ipapython.dn.RDN('dc=idm'), 
ipapython.dn.RDN('dc=lab'), ipapython.dn.RDN('dc=bos'), 
ipapython.dn.RDN('dc=redhat'), ipapython.dn.RDN('dc=com')]



ipalib/plugins/vault.py:
Do not use star imports in new code; remove the line
  from ipalib.plugins.baseldap import *
and use e.g. `baseldap.LDAPObject` instead of just `LDAPObject`.
Hopefully the only star-imported used are the base classes?


I found 20 star imports in the existing plugins :/

All existing plugins are actually using LDAPObject directly, not 
baseldap.LDAPObject. Is there a concern doing that? I fixed the code to 
do the same thing.



To make translators' jobs easier, split the help text in __doc__ into
strings that can be translated individually.
Replace every blank line by:
) + _(


Fixed.


The pattern and pattern_errmsg for the 'cn' param don't match. Which one
is right? Shouldn't a similar check be there for parent?


This is actually copied verbatim from user's uid. Could you give some 
sample values that show the problem?


I think ideally there should be a similar check for parent, but I 
haven't figured out the proper pattern yet. I think we can do this as a 
later enhancement. Or maybe we should remove the validation for cn for now.



vaultcontainer.get_dn: Why put '/' at the end of container_id, if the
empty string is ignored later anyway?


I'm still considering some options for container ID:
a) use a slash at the end for all containers (e.g. /users/)
b) don't use a trailing slash, except for root container (/)

I think option (a) is more consistent, container ID always ends with 
slash, although in cases like this it can be redundant. This is how the 
code is currently written. We can make an exception for 
vaultcontainer.get_dn() though.


Option (b) will require more careful coding in many places (e.g. 
concatenations) because it needs to handle a special case for root 
container.


Maybe a better way is to use an array of names internally and the slash 
is only used during input/output. The problem is when calling another 
command the array has to be converted into string and parsed back into 
array, so it may even introduce more redundancies. I don't see a way to 
pass an object parameter to a command. Any suggestion?



Nitpick: In vaultcontainer.get_dn, prefer the if statement over the
if-else expressions; it's a bit longer but more readable.


Fixed.


In vaultcontainer.get_id, what's the purpose of the len() check? Did you
want dn.endswith() instead?


Yes, but my assumption is the DN will always be within the namespace 
because it's only used internally, and using len() will be faster than 
endswith(). The error case will never happen, but I was just making sure 
there won't be an infinite loop. Maybe instead of calling the method 
recursively I should just use a loop? That will avoid repetitive 
validations.



I sugggest writing doctests for the id manipulation methods (get_id,
get_private_id, ...) – it's not always obvious what exactly they do.


Do you mean sample code in docs? Is it still necessary if we have some 
test code?



According to the design doc, container IDs shouldn't end in '/'. If you
did that I think the manipulation would be simpler, and it could be
shared with the vault equivalents.


Yeah, as mentioned above I'm still considering several options. The 
design may need to be adjusted.



vaultcontainer_add: You should use ldap backend directly. Calling
Commands is costly, most of the call is spent doing validation of what
here is already validated. You should add a function to recursively
create vault container using just the ldap client, and call that here
and in vault_add.


Hmm.. I'm not sure if we should write a code that will 

Re: [Freeipa-devel] [PATCH] 353 Added initial vault implementation.

2014-10-28 Thread Endi Sukma Dewata

On 10/22/2014 3:04 PM, Endi Sukma Dewata wrote:

On 10/16/2014 4:12 PM, Endi Sukma Dewata wrote:

On 10/15/2014 10:59 PM, Endi Sukma Dewata wrote:

The NSSConnection class has to be modified not to shutdown existing
database because some of the vault clients (e.g. vault-archive and
vault-retrieve) also use a database to encrypt/decrypt the secret.


The problem is described in more detail in this ticket:
https://fedorahosted.org/freeipa/ticket/4638

The changes to the NSSConnection in the first patch caused the
installation to fail. Attached is a new patch that uses the solution
proposed by jdennis.


New patch attached. It's now using the correct OID's for the schema. It
also has been rebased on top of #352-1 and #354.


New patch attached to fix the ticket URL. It depends on #352-2 and #354-1.

--
Endi S. Dewata
From cd3daa901f7139801ea61ae1e2716810da131bcc Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Tue, 21 Oct 2014 10:57:08 -0400
Subject: [PATCH] Added initial vault implementation.

This patch provides the initial vault implementation which allows
the admin to create a vault, archive a secret, and retrieve the
secret using a standard vault. It also included the initial LDAP
schema.

It currently has limitations including:
 - The vault only supports the standard vault type.
 - The vault can only be used by the admin user.
 - The transport certificate has to be installed manually.

These limitations, other vault features, schema and ACL changes will
be addressed in subsequent patches.

https://fedorahosted.org/freeipa/ticket/3872
---
 API.txt| 160 
 VERSION|   4 +-
 install/share/60basev4.ldif|   3 +
 install/share/Makefile.am  |   1 +
 install/share/copy-schema-to-ca.py |   1 +
 install/updates/40-vault.update|  27 ++
 install/updates/Makefile.am|   1 +
 ipa-client/man/default.conf.5  |   1 +
 ipalib/constants.py|   1 +
 ipalib/plugins/user.py |   9 +
 ipalib/plugins/vault.py| 724 +
 ipaserver/install/dsinstance.py|   1 +
 12 files changed, 931 insertions(+), 2 deletions(-)
 create mode 100644 install/share/60basev4.ldif
 create mode 100644 install/updates/40-vault.update
 create mode 100644 ipalib/plugins/vault.py

diff --git a/API.txt b/API.txt
index 
491d7a76fd1d2d50208d314d1600839ce295..cfa6558fcf678e5915a90407da517f9a591a41bf
 100644
--- a/API.txt
+++ b/API.txt
@@ -4475,6 +4475,166 @@ option: Str('version?', exclude='webui')
 output: Output('result', type 'bool', None)
 output: Output('summary', (type 'unicode', type 'NoneType'), None)
 output: PrimaryKey('value', None, None)
+command: vault_add
+args: 1,8,3
+arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, 
pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', 
primary_key=True, required=True)
+option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
+option: Str('description', attribute=True, cli_name='desc', multivalue=False, 
required=False)
+option: Str('in?', cli_name='in')
+option: Str('parent', attribute=False, cli_name='parent', multivalue=False, 
required=False)
+option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
+option: Flag('rights', autofill=True, default=False)
+option: Bytes('secret', attribute=True, cli_name='secret', multivalue=False, 
required=False)
+option: Str('version?', exclude='webui')
+output: Entry('result', type 'dict', Gettext('A dictionary representing an 
LDAP entry', domain='ipa', localedir=None))
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: PrimaryKey('value', None, None)
+command: vault_archive
+args: 1,10,3
+arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, 
pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', 
primary_key=True, query=True, required=True)
+option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
+option: Bytes('encrypted_data?', cli_name='encrypted_data')
+option: Str('in?', cli_name='in')
+option: Bytes('nonce?', cli_name='nonce')
+option: Str('parent?', cli_name='parent')
+option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
+option: Flag('rights', autofill=True, default=False)
+option: Bytes('secret?', cli_name='secret')
+option: Str('version?', exclude='webui')
+option: Bytes('wrapped_session_key?', cli_name='wrapped_session_key')
+output: Entry('result', type 'dict', Gettext('A dictionary representing an 
LDAP entry', domain='ipa', localedir=None))
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: PrimaryKey('value', None, None)
+command: vault_del
+args: 1,3,3
+arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=True, 
pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', 

Re: [Freeipa-devel] [PATCH] 353 Added initial vault implementation.

2014-10-22 Thread Endi Sukma Dewata

On 10/16/2014 4:12 PM, Endi Sukma Dewata wrote:

On 10/15/2014 10:59 PM, Endi Sukma Dewata wrote:

The NSSConnection class has to be modified not to shutdown existing
database because some of the vault clients (e.g. vault-archive and
vault-retrieve) also use a database to encrypt/decrypt the secret.


The problem is described in more detail in this ticket:
https://fedorahosted.org/freeipa/ticket/4638

The changes to the NSSConnection in the first patch caused the
installation to fail. Attached is a new patch that uses the solution
proposed by jdennis.


New patch attached. It's now using the correct OID's for the schema. It 
also has been rebased on top of #352-1 and #354.


--
Endi S. Dewata
From 2284f5684149e9fdfb7cde13865fe28e265ff5a3 Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Tue, 21 Oct 2014 10:57:08 -0400
Subject: [PATCH] Added initial vault implementation.

This patch provides the initial vault implementation which allows
the admin to create a vault, archive a secret, and retrieve the
secret using a standard vault. It also included the initial LDAP
schema.

It currently has limitations including:
 - The vault only supports the standard vault type.
 - The vault can only be used by the admin user.
 - The transport certificate has to be installed manually.

These limitations, other vault features, schema and ACL changes will
be addressed in subsequent patches.

Ticket #3872
---
 API.txt| 160 
 VERSION|   4 +-
 install/share/60basev4.ldif|   3 +
 install/share/Makefile.am  |   1 +
 install/share/copy-schema-to-ca.py |   1 +
 install/updates/40-vault.update|  27 ++
 install/updates/Makefile.am|   1 +
 ipa-client/man/default.conf.5  |   1 +
 ipalib/constants.py|   1 +
 ipalib/plugins/user.py |   9 +
 ipalib/plugins/vault.py| 724 +
 ipaserver/install/dsinstance.py|   1 +
 12 files changed, 931 insertions(+), 2 deletions(-)
 create mode 100644 install/share/60basev4.ldif
 create mode 100644 install/updates/40-vault.update
 create mode 100644 ipalib/plugins/vault.py

diff --git a/API.txt b/API.txt
index 
491d7a76fd1d2d50208d314d1600839ce295..cfa6558fcf678e5915a90407da517f9a591a41bf
 100644
--- a/API.txt
+++ b/API.txt
@@ -4475,6 +4475,166 @@ option: Str('version?', exclude='webui')
 output: Output('result', type 'bool', None)
 output: Output('summary', (type 'unicode', type 'NoneType'), None)
 output: PrimaryKey('value', None, None)
+command: vault_add
+args: 1,8,3
+arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, 
pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', 
primary_key=True, required=True)
+option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
+option: Str('description', attribute=True, cli_name='desc', multivalue=False, 
required=False)
+option: Str('in?', cli_name='in')
+option: Str('parent', attribute=False, cli_name='parent', multivalue=False, 
required=False)
+option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
+option: Flag('rights', autofill=True, default=False)
+option: Bytes('secret', attribute=True, cli_name='secret', multivalue=False, 
required=False)
+option: Str('version?', exclude='webui')
+output: Entry('result', type 'dict', Gettext('A dictionary representing an 
LDAP entry', domain='ipa', localedir=None))
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: PrimaryKey('value', None, None)
+command: vault_archive
+args: 1,10,3
+arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, 
pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', 
primary_key=True, query=True, required=True)
+option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
+option: Bytes('encrypted_data?', cli_name='encrypted_data')
+option: Str('in?', cli_name='in')
+option: Bytes('nonce?', cli_name='nonce')
+option: Str('parent?', cli_name='parent')
+option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
+option: Flag('rights', autofill=True, default=False)
+option: Bytes('secret?', cli_name='secret')
+option: Str('version?', exclude='webui')
+option: Bytes('wrapped_session_key?', cli_name='wrapped_session_key')
+output: Entry('result', type 'dict', Gettext('A dictionary representing an 
LDAP entry', domain='ipa', localedir=None))
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: PrimaryKey('value', None, None)
+command: vault_del
+args: 1,3,3
+arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=True, 
pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', 
primary_key=True, query=True, required=True)
+option: Flag('continue', autofill=True, cli_name='continue', default=False)
+option: Str('parent?', 

Re: [Freeipa-devel] [PATCH] 353 Added initial vault implementation.

2014-10-16 Thread Endi Sukma Dewata

On 10/15/2014 10:59 PM, Endi Sukma Dewata wrote:

The NSSConnection class has to be modified not to shutdown existing
database because some of the vault clients (e.g. vault-archive and
vault-retrieve) also use a database to encrypt/decrypt the secret.


The problem is described in more detail in this ticket:
https://fedorahosted.org/freeipa/ticket/4638

The changes to the NSSConnection in the first patch caused the 
installation to fail. Attached is a new patch that uses the solution 
proposed by jdennis.


--
Endi S. Dewata
From 65929b64da8d273a8b991845e47e69c1b3182f79 Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Tue, 16 Sep 2014 20:11:35 -0400
Subject: [PATCH] Added initial vault implementation.

This patch provides the initial vault implementation which allows
the admin to create a vault, archive a secret, and retrieve the
secret using a standard vault.

It currently has limitations including:
 - The vault only supports the standard vault type.
 - The vault can only be used by the admin user.
 - The transport certificate has to be installed manually.

These limitations, other vault features, schema and ACL changes will
be addressed in subsequent patches.

The NSSConnection class has been modified not to shutdown existing
database because some of the vault clients (e.g. vault-archive and
vault-retrieve) also use a database to encrypt/decrypt the secret.

Ticket #4638, #3872
---
 API.txt| 160 
 VERSION|   4 +-
 install/share/60basev4.ldif|   3 +
 install/share/Makefile.am  |   1 +
 install/share/copy-schema-to-ca.py |   1 +
 install/updates/40-vault.update|  27 ++
 install/updates/Makefile.am|   1 +
 ipa-client/man/default.conf.5  |   1 +
 ipalib/constants.py|   1 +
 ipalib/plugins/user.py |   9 +
 ipalib/plugins/vault.py| 726 +
 ipalib/rpc.py  |  34 +-
 ipapython/nsslib.py|  31 +-
 ipaserver/install/dsinstance.py|   1 +
 14 files changed, 973 insertions(+), 27 deletions(-)
 create mode 100644 install/share/60basev4.ldif
 create mode 100644 install/updates/40-vault.update
 create mode 100644 ipalib/plugins/vault.py

diff --git a/API.txt b/API.txt
index 
1af78509732b13eec07208114cea00e56c1059b4..1eec3527e36bc250acddbf0e2fe7a6baa30abd74
 100644
--- a/API.txt
+++ b/API.txt
@@ -4373,6 +4373,166 @@ option: Str('version?', exclude='webui')
 output: Output('result', type 'bool', None)
 output: Output('summary', (type 'unicode', type 'NoneType'), None)
 output: PrimaryKey('value', None, None)
+command: vault_add
+args: 1,8,3
+arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, 
pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', 
primary_key=True, required=True)
+option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
+option: Str('description', attribute=True, cli_name='desc', multivalue=False, 
required=False)
+option: Str('in?', cli_name='in')
+option: Str('parent', attribute=False, cli_name='parent', multivalue=False, 
required=False)
+option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
+option: Flag('rights', autofill=True, default=False)
+option: Bytes('secret', attribute=True, cli_name='secret', multivalue=False, 
required=False)
+option: Str('version?', exclude='webui')
+output: Entry('result', type 'dict', Gettext('A dictionary representing an 
LDAP entry', domain='ipa', localedir=None))
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: PrimaryKey('value', None, None)
+command: vault_archive
+args: 1,10,3
+arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, 
pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', 
primary_key=True, query=True, required=True)
+option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
+option: Bytes('encrypted_data?', cli_name='encrypted_data')
+option: Str('in?', cli_name='in')
+option: Bytes('nonce?', cli_name='nonce')
+option: Str('parent?', cli_name='parent')
+option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
+option: Flag('rights', autofill=True, default=False)
+option: Bytes('secret?', cli_name='secret')
+option: Str('version?', exclude='webui')
+option: Bytes('wrapped_session_key?', cli_name='wrapped_session_key')
+output: Entry('result', type 'dict', Gettext('A dictionary representing an 
LDAP entry', domain='ipa', localedir=None))
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: PrimaryKey('value', None, None)
+command: vault_del
+args: 1,3,3
+arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=True, 
pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', 
primary_key=True, query=True, required=True)
+option: Flag('continue', 

[Freeipa-devel] [PATCH] 353 Added initial vault implementation.

2014-10-15 Thread Endi Sukma Dewata

This patch provides the initial vault implementation which allows
the admin to create a vault, archive a secret, and retrieve the
secret using a standard vault.

It currently has limitations including:
 - The vault only supports the standard vault type.
 - The vault can only be used by the admin user.
 - The transport certificate has to be installed manually.

These limitations, other vault features, schema and ACL changes will
be addressed in subsequent patches.

The NSSConnection class has to be modified not to shutdown existing
database because some of the vault clients (e.g. vault-archive and
vault-retrieve) also use a database to encrypt/decrypt the secret.

Ticket #3872

--
Endi S. Dewata
From 1ad4307323c9e76ed51e5cdbd736e8834864f6fc Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Tue, 16 Sep 2014 20:11:35 -0400
Subject: [PATCH] Added initial vault implementation.

This patch provides the initial vault implementation which allows
the admin to create a vault, archive a secret, and retrieve the
secret using a standard vault.

It currently has limitations including:
 - The vault only supports the standard vault type.
 - The vault can only be used by the admin user.
 - The transport certificate has to be installed manually.

These limitations, other vault features, schema and ACL changes will
be addressed in subsequent patches.

The NSSConnection class has to be modified not to shutdown existing
database because some of the vault clients (e.g. vault-archive and
vault-retrieve) also use a database to encrypt/decrypt the secret.

Ticket #3872
---
 API.txt| 160 
 VERSION|   4 +-
 install/share/60basev4.ldif|   3 +
 install/share/Makefile.am  |   1 +
 install/share/copy-schema-to-ca.py |   1 +
 install/updates/40-vault.update|  27 ++
 install/updates/Makefile.am|   1 +
 ipa-client/man/default.conf.5  |   1 +
 ipalib/constants.py|   1 +
 ipalib/plugins/user.py |   9 +
 ipalib/plugins/vault.py| 726 +
 ipapython/nsslib.py|  22 +-
 ipaserver/install/dsinstance.py|   1 +
 13 files changed, 943 insertions(+), 14 deletions(-)
 create mode 100644 install/share/60basev4.ldif
 create mode 100644 install/updates/40-vault.update
 create mode 100644 ipalib/plugins/vault.py

diff --git a/API.txt b/API.txt
index 
1af78509732b13eec07208114cea00e56c1059b4..1eec3527e36bc250acddbf0e2fe7a6baa30abd74
 100644
--- a/API.txt
+++ b/API.txt
@@ -4373,6 +4373,166 @@ option: Str('version?', exclude='webui')
 output: Output('result', type 'bool', None)
 output: Output('summary', (type 'unicode', type 'NoneType'), None)
 output: PrimaryKey('value', None, None)
+command: vault_add
+args: 1,8,3
+arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, 
pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', 
primary_key=True, required=True)
+option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
+option: Str('description', attribute=True, cli_name='desc', multivalue=False, 
required=False)
+option: Str('in?', cli_name='in')
+option: Str('parent', attribute=False, cli_name='parent', multivalue=False, 
required=False)
+option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
+option: Flag('rights', autofill=True, default=False)
+option: Bytes('secret', attribute=True, cli_name='secret', multivalue=False, 
required=False)
+option: Str('version?', exclude='webui')
+output: Entry('result', type 'dict', Gettext('A dictionary representing an 
LDAP entry', domain='ipa', localedir=None))
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: PrimaryKey('value', None, None)
+command: vault_archive
+args: 1,10,3
+arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, 
pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', 
primary_key=True, query=True, required=True)
+option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
+option: Bytes('encrypted_data?', cli_name='encrypted_data')
+option: Str('in?', cli_name='in')
+option: Bytes('nonce?', cli_name='nonce')
+option: Str('parent?', cli_name='parent')
+option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
+option: Flag('rights', autofill=True, default=False)
+option: Bytes('secret?', cli_name='secret')
+option: Str('version?', exclude='webui')
+option: Bytes('wrapped_session_key?', cli_name='wrapped_session_key')
+output: Entry('result', type 'dict', Gettext('A dictionary representing an 
LDAP entry', domain='ipa', localedir=None))
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: PrimaryKey('value', None, None)
+command: vault_del
+args: 1,3,3
+arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=True,