Re: [Freeipa-devel] [PATCH] 353 Added initial vault implementation.
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.
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 dn.e
Re: [Freeipa-devel] [PATCH] 353 Added initial vault implementation.
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 w
Re: [Freeipa-devel] [PATCH] 353 Added initial vault implementation.
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.
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. -- Endi S. Dewata >From 71c91726e1c442baa24321000556a3c9ce8e Mon Sep 17 00:00:00 2001 From: "Endi S. Dewata" 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 data, and retrieve data using a standard vault. It also includes 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. The DN class has been enhanched to accept a list of RDNs. https://fedorahosted.org/freeipa/ticket/3872 --- API.txt| 167 +++ 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| 865 + ipapython/dn.py| 2 + ipaserver/install/dsinstance.py| 1 + 13 files changed, 1081 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..b73da0af55a3c514de73ae4e1b2a4d13c01c903d 100644 --- a/API.txt +++ b/API.txt @@ -4475,6 +4475,173 @@ option: Str('version?', exclude='webui') output: Output('result', , None) output: Output('summary', (, ), None) output: PrimaryKey('value', None, None) +command: vault_add +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, required=True) +option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') +option: Bytes('data?', cli_name='data') +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: Str('text?', cli_name='text') +option: Str('vault_id', attribute=False, cli_name='vault_id', multivalue=False, required=False) +option: Str('version?', exclude='webui') +output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) +output: Output('summary', (, ), None) +output: PrimaryKey('value', None, None) +command: vault_archive +args: 1,11,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('data?', cli_name='data') +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: Str('text?', cli_name='text') +option: Str('version?', exclude='webui') +option: Bytes('wrapped_session_key?', cli_name='wrapped_session_key') +output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) +output: Output('summary', (, ), 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-z
Re: [Freeipa-devel] [PATCH] 353 Added initial vault implementation.
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" 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', , None) output: Output('summary', (, ), 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', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) +output: Output('summary', (, ), 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', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) +output: Output('summary', (, ), 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('paren
Re: [Freeipa-devel] [PATCH] 353 Added initial vault implementation.
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" 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', , None) output: Output('summary', (, ), 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', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) +output: Output('summary', (, ), 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', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) +output: Output('summary', (, ), 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?', cli_name='parent') +option: Str('version?', exclude='webui') +output: Output('result', , None) +output: Output('summary', (, ), None) +output: ListOfPr
Re: [Freeipa-devel] [PATCH] 353 Added initial vault implementation.
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" 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', , None) output: Output('summary', (, ), 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', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) +output: Output('summary', (, ), 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', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) +output: Output('summary', (, ), 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?', cli_name='parent') +option: Str('version?', exclude='webui') +ou