On 5/13/2015 4:09 AM, Jan Cholasta wrote:
Dne 12.5.2015 v 12:52 Endi Sukma Dewata napsal(a):
Please take a look at the attached patch (#353-9). It obsoletes all
previous patches. See comments below.

On 4/20/2015 1:12 AM, Jan Cholasta wrote:
I'm planning to merge the vault and vault container object and use the
vault type attribute to distinguish between the two. See more
discussion
about that below.

OK.

The vault container plugin has been removed instead of merged (see
explanation below). Internally the vaults are still stored in built-in
containers in the DS, but there won't be an interface to manage them.
The following containers are available for use: private, shared, and
services, but they are flat, not hierarchical.

To speed up the review, I have amended your patch with code and coding
style fixes (attached), please review my changes.

Question: Services in IPA are identified by Kerberos principal. Why are
service vaults identified by hostname alone?

The service vaults are actually identified by the hostname and service name assuming the principal is in this format: <name>/<host>@<realm>. The vault is stored in cn=<name>, cn=<host>, cn=services, cn=vaults, <suffix>. When accessing a vault service you are supposed to specify the name and the host (except for vault-find which will return all services in the same host):

$ ipa vault-find --host <host>
$ ipa vault-add <name> --host <host>

Are there other service principal formats that we need to support? Do we need to support services of different realms?

Some comments about your changes:

1. The following code in get_dn() is causing problems for vault-find.

  dn = super(vault, self).get_dn(*keys, **options)
  rdn = dn[0]
  container_dn = DN(*dn[1:])

The super.get_dn() will return cn=vaults, <suffix>, so the rdn will become cn=vaults and container_dn will become <suffix>. When combined with the subcontainer (private/shared/service) it will produce an invalid DN.

2. Not sure if it'related to #1, the vault-find is failing.

$ ipa vault-find
ipa: ERROR: an internal error has occurred

The error_log shows TypeError: 'NoneType' object is not iterable

3. The --shared option in vault-find is now requiring an argument even though it's a Flag.

$ ipa vault-find --shared
Usage: ipa [global-options] vault-find [CRITERIA] [options]

ipa: error: --shared option requires an argument

4. The following code in get_dn() is incorrect:

  principal = getattr(context, 'principal')
  (name, realm) = split_principal(principal)
  name = name.split('/')
  if len(name) == 1:
      container_dn = DN(('cn', 'users'), container_dn)
  else:
      container_dn = DN(('cn', 'services'), container_dn)
  container_dn = DN(('cn', name[-1]), container_dn)

A service does not have a private container like users (cn=<username>, cn=users, cn=vaults). The entry cn=<name>, cn=<host>, cn=services, cn=vaults is a service vault, not a container. The service vault is used by the admin to provide a secret for a service.

I'm not sure what the behavior should be if a service is executing a vault command that uses a private container such as:

  $ ipa vault-add test

Maybe it should just generate an error.

5. In create_container() why do you need to reconstruct the container_dn on each invocation even though the value is fixed?

  container_dn = DN(self.container_dn, self.api.env.basedn)

6. The loop in create_container() is incorrect. Suppose we're creating a container cn=A, cn=B, <suffix> and the parent container cn=B, <suffix> doesn't exist yet. The first add_entry() invocation will fail as expected, but instead of adding the parent entry the whole method will fail.

  while dn.endswith(container_dn):
      entry = self.backend.make_entry(
          dn,
          objectclass=['nsContainer'],
          cn=[dn[0]['cn']],
      )

      try:
          self.backend.add_entry(entry)
      except errors.DuplicateEntry:
          break

      dn = DN(*dn[1:])

7. The host and shared parameters are no longer available in vault-show and vault-del commands. The unit tests are failing because of that. Why do these commands not automatically inherit all parameters from the class?

--
Endi S. Dewata

--
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

Reply via email to