On 6/15/2015 2:22 AM, Jan Cholasta wrote:
I think it would be better to use a new attribute type which inherits
from ipaPublicKey (ipaVaultPublicKey?) rather than ipaPublicKey directly
for assymetric vault public keys, so that assymetric public key and
escrow public key are on the same level and you can still use
ipaPublicKey to refer to either one:

     ipaPublicKey
         ipaVaultPublicKey
         ipaEscrowPublicKey

     ( 2.16.840.1.113730.3.8.18.2.? NAME 'ipaVaultPublicKey' DESC
'Assymetric vault public key as DER-encoded SubjectPublicKeyInfo (RFC
5280)' SUP ipaPublicKey EQUALITY octetStringMatch SYNTAX
1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'IPA v4.2' )
     ( 2.16.840.1.113730.3.8.18.2.3 NAME 'ipaEscrowPublicKey' DESC 'IPA
escrow public key as DER-encoded SubjectPublicKeyInfo (RFC 5280)' SUP
ipaPublicKey EQUALITY octetStringMatch SYNTAX
1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'IPA v4.2' )

OK. To be consistent the parameters need to be renamed too: --vault-public-key and --vault-public-key-file.

1. The vault_add was split into a client-side vault_add and server-side
vault_add_internal since the parameters are different (i.e. public
key file and
future escrow-related params). Since vault_add inherits from Local all
non-primary-key attributes have to be added explicitly.

The split is not really necessary, since the only difference is the
public_key_file option, which exists only because of the lack of proper
file support in the framework. This is a different situation from
vault_{archive,retrieve}, which has two different sets of options on
client and server side. Escrow adds only ipaescrowpublickey and
escrow_public_key_file, right? If yes, we can safely keep the command in
a single piece.

We know the vault-add will have at least two client-only parameters: vault_public_key_file and escrow_public_key_file. Keeping these parameters on the server API would be wrong and confusing. If the API is called on the server side with vault_public_key_file the operation will fail. In the previous discussion you considered this as broken API:

Server API is used not only by the server itself, but also by installers
for example. Anyway the point is that there *can't* be a broken API like
this, you should at least raise an error if the command is called from
server API, although actually separating it into client and server parts
would be preferable.

Also, originally the vault was designed like this: when you create a symmetric vault you're supposed to specify the password as well, similar to adding a public key when creating an asymmetric vault. When you archive, you're supposed to enter the same password for verification, not a new password. So it would look like this:

$ ipa vault-add test --type symmetric
New password: ********
Verify password: ********

$ ipa vault-archive test --in secret1.txt
Password: ******** (same password)

$ ipa vault-archive test --in secret2.txt
Password: ******** (same password)

In the original design the vault-add would also archive a blank data, which later could be used to verify the password during vault-archive by decrypting the existing data first. There's also a plan to add a mechanism to change the password after the ACL patch.

In the current design the vault-add doesn't archive anything, so during vault-archive it cannot verify the password because there is nothing to decrypt. In other words you can specify different passwords on each archival, regardless of previous archivals:

$ ipa vault-add test --type symmetric

$ ipa vault-archive test --in secret1.txt
New password: ********
Verify password: ********

$ ipa vault-archive test --in secret2.txt
New password: ********
Verify password: ********

So basically here are the options:

1. Specify the crypto parameters once during vault creation, then reuse/verify the parameters on each archival & retrieval. You can change the parameters only with a special command.

2. Don't specify the crypto parameters during vault creation, but specify new parameters on each archival. For retrieval you'd have to use/verify the parameters specified in the last archival.

I think the first one makes more sense and is easier to use. That also means the vault-add will have additional client-only parameters such as --password and --password-file.

2. Since the vault_archive_internal inherits from Update, it accepts
all non
primary-key attributes automatically. This is incorrect since we
don't want to
update these parameters during archival. Can this behavior be
overridden?

Inherit from PKQuery instead (don't forget to add "has_output =
output.standard_entry").

Previously you didn't want to use LDAPQuery because of semantics reasons. Is PKQuery fine semantically? Why not use LDAPQuery since vault is an LDAPObject? And to be consistent should vault_retrieve_internal inherit from the same class?

BTW the correct solution would be to have a separate object and commands
for vault data (e.g. vaultdata object, vault_archive -> vaultdata_mod,
vault_retrieve -> vauldata_show), then we wouldn't have to deal with
mixing vault attributes with vault data and could use proper crud base
classes.

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