Re: [Freeipa-devel] Design Review Keytab Retrieval
On (23/06/14 14:35), Simo Sorce wrote:
- Original Message -
- Original Message -
Can you check if ipaProtectedOperation is in the aci attribute in the
base tree object ?
It should be there as excluded, and that should cause admin to not be
able to retrieve keytabs.
It was not. While running ipa-ldap-updater I got the following:
InvalidSyntax: ACL Syntax Error(-5):(targetattr=
\22ipaProtectedOperation;write_keys\22)(version 3.0; acl \22Admins are
allowed to rekey any entity\22; allow(write) groupdn =
\22ldap:///cn=admins: Invalid syntax.
Uhmm I do not see anything obviously wrong with ACI instruction, it looks
just like the one I replace, Ideas ?
Do you have ipaProtectedOperation in the schema ?
(I rebased patch 3 but will wait to send a patchset until we understand (and
fix) why this is failing to update.
Ok, apparently it was a quoting issue in the .update files, hopefully that's
the only issue (I am at a conference today and do not have my test env. handy).
The attached patches are rebased on the latest master.
Simo.
From af7364cda7f5ef5948ce782bd5eded0b57583029 Mon Sep 17 00:00:00 2001
From: Simo Sorce s...@redhat.com
Date: Tue, 17 Sep 2013 00:30:14 -0400
Subject: [PATCH 3/6] keytab: Add new extended operation to get a keytab.
This new extended operation allow to create new keys or retrieve
existing ones. The new set of keys is returned as a ASN.1 structure
similar to the one that is passed in by the 'set keytab' extended
operation.
Access to the operation is regulated through a new special ACI that
allows 'retrieval' only if the user has access to an attribute named
ipaProtectedOperation postfixed by the subtypes 'read_keys' and
'write_keys' to distinguish between creation and retrieval operation.
For example for allowing retrieval by a specific user the following ACI
is set on cn=accounts:
(targetattr=ipaProtectedOperation;read_keys) ...
... userattr=ipaAllowedToPerform;read_keys#USERDN)
This ACI matches only if the service object hosts a new attribute named
ipaAllowedToPerform that holds the DN of the user attempting the
operation.
Resolves:
https://fedorahosted.org/freeipa/ticket/3859
---
.../ipa-pwd-extop/ipa_pwd_extop.c | 571 +
install/share/60basev3.ldif| 3 +
install/share/default-aci.ldif | 7 +
install/updates/20-aci.update | 13 +-
util/ipa_krb5.h| 1 +
5 files changed, 594 insertions(+), 1 deletion(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
index
c0bb9fda26172699e9ae7628f61b763c746188fe..65a5d41f9ea85689952e818fa4e8018c39786db8
100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -1268,6 +1268,571 @@ free_and_return:
return SLAPI_PLUGIN_EXTENDED_SENT_RESULT;
}
+/* Format of getkeytab request
+ *
+ * KeytabGetRequest ::= CHOICE {
+ * newkeys [0] Newkeys,
+ * curkeys [1] CurrentKeys,
+ * reply[2] Reply
+ * }
+ *
+ * NewKeys ::= SEQUENCE {
+ * serviceIdentity [0] OCTET STRING,
+ * enctypes[1] SEQUENCE OF Int16
+ * password[2] OCTET STRING OPTIONAL,
+ * }
+ *
+ * CurrentKeys ::= SEQUENCE {
+ * serviceIdentity [0] OCTET STRING,
+ * }
+ */
+
+#define GK_REQUEST_NEWKEYS (LBER_CLASS_CONTEXT | LBER_CONSTRUCTED | 0)
+#define GK_REQUEST_CURKEYS (LBER_CLASS_CONTEXT | LBER_CONSTRUCTED | 1)
+#define GKREQ_SVCNAME_TAG (LBER_CLASS_CONTEXT | LBER_CONSTRUCTED | 1)
+#define GKREQ_ENCTYPES_TAG (LBER_CLASS_CONTEXT | LBER_CONSTRUCTED | 1)
+#define GKREQ_PASSWORD_TAG (LBER_CLASS_CONTEXT | LBER_CONSTRUCTED | 2)
+
+static int decode_getkeytab_request(struct berval *extop, bool *wantold,
+char **_svcname, char **_password,
+krb5_key_salt_tuple **kenctypes,
+int *num_kenctypes, char **_err_msg)
+{
+int rc = LDAP_OPERATIONS_ERROR;
+char *err_msg = NULL;
+BerElement *ber = NULL;
+ber_len_t tlen;
+ber_tag_t rtag;
+ber_tag_t ttag;
+ber_tag_t ctag;
+char *svcname = NULL;
+char *password = NULL;
+ber_int_t enctype;
+krb5_key_salt_tuple *enctypes = NULL;
+int num = 0;
+
+ber = ber_init(extop);
+if (ber == NULL) {
+err_msg = KeytabGet Request decode failed.\n;
+rc = LDAP_PROTOCOL_ERROR;
+goto done;
+}
+
+/* check this is a request */
+rtag = ber_peek_tag(ber, tlen);
+if (rtag != GK_REQUEST_NEWKEYS rtag != GK_REQUEST_CURKEYS) {
+LOG_FATAL(ber_peek_tag failed, wrong request type\n);
+err_msg = Invalid payload.\n;
+rc = LDAP_PROTOCOL_ERROR;
+goto done;
+}
+
+/* ber parse code */
+ttag = ber_scanf(ber, {t[a], ctag, svcname);
+if (ttag
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Thu, 26 Jun 2014, Simo Sorce wrote: On Thu, 2014-06-26 at 10:20 -0400, Simo Sorce wrote: On Thu, 2014-06-26 at 15:33 +0300, Alexander Bokovoy wrote: On Thu, 26 Jun 2014, Martin Kosek wrote: On 06/26/2014 04:29 AM, Nathaniel McCallum wrote: On Mon, 2014-06-23 at 17:24 -0400, Nathaniel McCallum wrote: On Mon, 2014-06-23 at 14:35 -0400, Simo Sorce wrote: - Original Message - - Original Message - Can you check if ipaProtectedOperation is in the aci attribute in the base tree object ? It should be there as excluded, and that should cause admin to not be able to retrieve keytabs. It was not. While running ipa-ldap-updater I got the following: InvalidSyntax: ACL Syntax Error(-5):(targetattr= \22ipaProtectedOperation;write_keys\22)(version 3.0; acl \22Admins are allowed to rekey any entity\22; allow(write) groupdn = \22ldap:///cn=admins: Invalid syntax. Uhmm I do not see anything obviously wrong with ACI instruction, it looks just like the one I replace, Ideas ? Do you have ipaProtectedOperation in the schema ? (I rebased patch 3 but will wait to send a patchset until we understand (and fix) why this is failing to update. Ok, apparently it was a quoting issue in the .update files, hopefully that's the only issue (I am at a conference today and do not have my test env. handy). The attached patches are rebased on the latest master. 0001: Line 555 has very wrong indentation. I don't see anything else wrong in the other patches. I've tested everything and it works as designed. I have CC'd everyone who was involved with review at any point on these patches. This serves as my public notice that I'd like to ACK the next round of patches. If anyone has anything else to add, please do it before tomorrow evening. Thanks! Nathaniel ACK Nathaniel Pushed all 6 patches to master. Thanks for careful review! Unfortunately, at least enctype marshalling is wrong with these patches. Samba does not work anymore with the keytab fetched in new version. We see following in the keytab: Keytab name: FILE:/etc/samba/samba.keytab KVNO Timestamp Principal - 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 274) 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 273) 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 272) 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 279) Note that etype is unresolvable. In the build without these patches we get something like 1 06/23/2014 16:28:59 cifs/vm-139.dom139.tbad.idm.lab.eng.brq.redhat@dom139.tbad.idm.lab.eng.brq.redhat.com (aes256-cts-hmac-sha1-96) So this patchset needs an improvement before release. Working on this. I know, roughly what's going on, but still trying to pinpoint exactly the offender. (It is the ber marshalling/unmarshalling indeed). Simo. The attached patch fixes it for me. ACK, works for me too. Martin: it makes sense to merge both this and the indentation fix together prior to commit. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On 06/27/2014 10:00 AM, Alexander Bokovoy wrote: On Thu, 26 Jun 2014, Simo Sorce wrote: On Thu, 2014-06-26 at 10:20 -0400, Simo Sorce wrote: On Thu, 2014-06-26 at 15:33 +0300, Alexander Bokovoy wrote: On Thu, 26 Jun 2014, Martin Kosek wrote: On 06/26/2014 04:29 AM, Nathaniel McCallum wrote: On Mon, 2014-06-23 at 17:24 -0400, Nathaniel McCallum wrote: On Mon, 2014-06-23 at 14:35 -0400, Simo Sorce wrote: - Original Message - - Original Message - Can you check if ipaProtectedOperation is in the aci attribute in the base tree object ? It should be there as excluded, and that should cause admin to not be able to retrieve keytabs. It was not. While running ipa-ldap-updater I got the following: InvalidSyntax: ACL Syntax Error(-5):(targetattr= \22ipaProtectedOperation;write_keys\22)(version 3.0; acl \22Admins are allowed to rekey any entity\22; allow(write) groupdn = \22ldap:///cn=admins: Invalid syntax. Uhmm I do not see anything obviously wrong with ACI instruction, it looks just like the one I replace, Ideas ? Do you have ipaProtectedOperation in the schema ? (I rebased patch 3 but will wait to send a patchset until we understand (and fix) why this is failing to update. Ok, apparently it was a quoting issue in the .update files, hopefully that's the only issue (I am at a conference today and do not have my test env. handy). The attached patches are rebased on the latest master. 0001: Line 555 has very wrong indentation. I don't see anything else wrong in the other patches. I've tested everything and it works as designed. I have CC'd everyone who was involved with review at any point on these patches. This serves as my public notice that I'd like to ACK the next round of patches. If anyone has anything else to add, please do it before tomorrow evening. Thanks! Nathaniel ACK Nathaniel Pushed all 6 patches to master. Thanks for careful review! Unfortunately, at least enctype marshalling is wrong with these patches. Samba does not work anymore with the keytab fetched in new version. We see following in the keytab: Keytab name: FILE:/etc/samba/samba.keytab KVNO Timestamp Principal - 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 274) 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 273) 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 272) 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 279) Note that etype is unresolvable. In the build without these patches we get something like 1 06/23/2014 16:28:59 cifs/vm-139.dom139.tbad.idm.lab.eng.brq.redhat@dom139.tbad.idm.lab.eng.brq.redhat.com (aes256-cts-hmac-sha1-96) So this patchset needs an improvement before release. Working on this. I know, roughly what's going on, but still trying to pinpoint exactly the offender. (It is the ber marshalling/unmarshalling indeed). Simo. The attached patch fixes it for me. ACK, works for me too. Martin: it makes sense to merge both this and the indentation fix together prior to commit. +1. (The bad indentation fix is my fault as I wanted to fix that before pushing, based on Nathaniel's point, but did not notice the source use tabs). Merged both patches, pushed to master. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On 06/26/2014 04:29 AM, Nathaniel McCallum wrote: On Mon, 2014-06-23 at 17:24 -0400, Nathaniel McCallum wrote: On Mon, 2014-06-23 at 14:35 -0400, Simo Sorce wrote: - Original Message - - Original Message - Can you check if ipaProtectedOperation is in the aci attribute in the base tree object ? It should be there as excluded, and that should cause admin to not be able to retrieve keytabs. It was not. While running ipa-ldap-updater I got the following: InvalidSyntax: ACL Syntax Error(-5):(targetattr= \22ipaProtectedOperation;write_keys\22)(version 3.0; acl \22Admins are allowed to rekey any entity\22; allow(write) groupdn = \22ldap:///cn=admins: Invalid syntax. Uhmm I do not see anything obviously wrong with ACI instruction, it looks just like the one I replace, Ideas ? Do you have ipaProtectedOperation in the schema ? (I rebased patch 3 but will wait to send a patchset until we understand (and fix) why this is failing to update. Ok, apparently it was a quoting issue in the .update files, hopefully that's the only issue (I am at a conference today and do not have my test env. handy). The attached patches are rebased on the latest master. 0001: Line 555 has very wrong indentation. I don't see anything else wrong in the other patches. I've tested everything and it works as designed. I have CC'd everyone who was involved with review at any point on these patches. This serves as my public notice that I'd like to ACK the next round of patches. If anyone has anything else to add, please do it before tomorrow evening. Thanks! Nathaniel ACK Nathaniel Pushed all 6 patches to master. Thanks for careful review! Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Thu, 26 Jun 2014, Martin Kosek wrote: On 06/26/2014 04:29 AM, Nathaniel McCallum wrote: On Mon, 2014-06-23 at 17:24 -0400, Nathaniel McCallum wrote: On Mon, 2014-06-23 at 14:35 -0400, Simo Sorce wrote: - Original Message - - Original Message - Can you check if ipaProtectedOperation is in the aci attribute in the base tree object ? It should be there as excluded, and that should cause admin to not be able to retrieve keytabs. It was not. While running ipa-ldap-updater I got the following: InvalidSyntax: ACL Syntax Error(-5):(targetattr= \22ipaProtectedOperation;write_keys\22)(version 3.0; acl \22Admins are allowed to rekey any entity\22; allow(write) groupdn = \22ldap:///cn=admins: Invalid syntax. Uhmm I do not see anything obviously wrong with ACI instruction, it looks just like the one I replace, Ideas ? Do you have ipaProtectedOperation in the schema ? (I rebased patch 3 but will wait to send a patchset until we understand (and fix) why this is failing to update. Ok, apparently it was a quoting issue in the .update files, hopefully that's the only issue (I am at a conference today and do not have my test env. handy). The attached patches are rebased on the latest master. 0001: Line 555 has very wrong indentation. I don't see anything else wrong in the other patches. I've tested everything and it works as designed. I have CC'd everyone who was involved with review at any point on these patches. This serves as my public notice that I'd like to ACK the next round of patches. If anyone has anything else to add, please do it before tomorrow evening. Thanks! Nathaniel ACK Nathaniel Pushed all 6 patches to master. Thanks for careful review! Unfortunately, at least enctype marshalling is wrong with these patches. Samba does not work anymore with the keytab fetched in new version. We see following in the keytab: Keytab name: FILE:/etc/samba/samba.keytab KVNO Timestamp Principal - 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 274) 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 273) 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 272) 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 279) Note that etype is unresolvable. In the build without these patches we get something like 1 06/23/2014 16:28:59 cifs/vm-139.dom139.tbad.idm.lab.eng.brq.redhat@dom139.tbad.idm.lab.eng.brq.redhat.com (aes256-cts-hmac-sha1-96) So this patchset needs an improvement before release. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On 06/26/2014 02:33 PM, Alexander Bokovoy wrote: On Thu, 26 Jun 2014, Martin Kosek wrote: On 06/26/2014 04:29 AM, Nathaniel McCallum wrote: On Mon, 2014-06-23 at 17:24 -0400, Nathaniel McCallum wrote: On Mon, 2014-06-23 at 14:35 -0400, Simo Sorce wrote: - Original Message - - Original Message - Can you check if ipaProtectedOperation is in the aci attribute in the base tree object ? It should be there as excluded, and that should cause admin to not be able to retrieve keytabs. It was not. While running ipa-ldap-updater I got the following: InvalidSyntax: ACL Syntax Error(-5):(targetattr= \22ipaProtectedOperation;write_keys\22)(version 3.0; acl \22Admins are allowed to rekey any entity\22; allow(write) groupdn = \22ldap:///cn=admins: Invalid syntax. Uhmm I do not see anything obviously wrong with ACI instruction, it looks just like the one I replace, Ideas ? Do you have ipaProtectedOperation in the schema ? (I rebased patch 3 but will wait to send a patchset until we understand (and fix) why this is failing to update. Ok, apparently it was a quoting issue in the .update files, hopefully that's the only issue (I am at a conference today and do not have my test env. handy). The attached patches are rebased on the latest master. 0001: Line 555 has very wrong indentation. I don't see anything else wrong in the other patches. I've tested everything and it works as designed. I have CC'd everyone who was involved with review at any point on these patches. This serves as my public notice that I'd like to ACK the next round of patches. If anyone has anything else to add, please do it before tomorrow evening. Thanks! Nathaniel ACK Nathaniel Pushed all 6 patches to master. Thanks for careful review! Unfortunately, at least enctype marshalling is wrong with these patches. Samba does not work anymore with the keytab fetched in new version. We see following in the keytab: Keytab name: FILE:/etc/samba/samba.keytab KVNO Timestamp Principal - 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 274) 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 273) 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 272) 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 279) Note that etype is unresolvable. In the build without these patches we get something like 1 06/23/2014 16:28:59 cifs/vm-139.dom139.tbad.idm.lab.eng.brq.redhat@dom139.tbad.idm.lab.eng.brq.redhat.com (aes256-cts-hmac-sha1-96) So this patchset needs an improvement before release. FYI: I filed https://fedorahosted.org/freeipa/ticket/4404 , setting up this as blocker. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Thu, 2014-06-26 at 15:33 +0300, Alexander Bokovoy wrote: On Thu, 26 Jun 2014, Martin Kosek wrote: On 06/26/2014 04:29 AM, Nathaniel McCallum wrote: On Mon, 2014-06-23 at 17:24 -0400, Nathaniel McCallum wrote: On Mon, 2014-06-23 at 14:35 -0400, Simo Sorce wrote: - Original Message - - Original Message - Can you check if ipaProtectedOperation is in the aci attribute in the base tree object ? It should be there as excluded, and that should cause admin to not be able to retrieve keytabs. It was not. While running ipa-ldap-updater I got the following: InvalidSyntax: ACL Syntax Error(-5):(targetattr= \22ipaProtectedOperation;write_keys\22)(version 3.0; acl \22Admins are allowed to rekey any entity\22; allow(write) groupdn = \22ldap:///cn=admins: Invalid syntax. Uhmm I do not see anything obviously wrong with ACI instruction, it looks just like the one I replace, Ideas ? Do you have ipaProtectedOperation in the schema ? (I rebased patch 3 but will wait to send a patchset until we understand (and fix) why this is failing to update. Ok, apparently it was a quoting issue in the .update files, hopefully that's the only issue (I am at a conference today and do not have my test env. handy). The attached patches are rebased on the latest master. 0001: Line 555 has very wrong indentation. I don't see anything else wrong in the other patches. I've tested everything and it works as designed. I have CC'd everyone who was involved with review at any point on these patches. This serves as my public notice that I'd like to ACK the next round of patches. If anyone has anything else to add, please do it before tomorrow evening. Thanks! Nathaniel ACK Nathaniel Pushed all 6 patches to master. Thanks for careful review! Unfortunately, at least enctype marshalling is wrong with these patches. Samba does not work anymore with the keytab fetched in new version. We see following in the keytab: Keytab name: FILE:/etc/samba/samba.keytab KVNO Timestamp Principal - 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 274) 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 273) 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 272) 1 06/26/2014 13:03:01 cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com (etype 279) Note that etype is unresolvable. In the build without these patches we get something like 1 06/23/2014 16:28:59 cifs/vm-139.dom139.tbad.idm.lab.eng.brq.redhat@dom139.tbad.idm.lab.eng.brq.redhat.com (aes256-cts-hmac-sha1-96) So this patchset needs an improvement before release. Working on this. I know, roughly what's going on, but still trying to pinpoint exactly the offender. (It is the ber marshalling/unmarshalling indeed). Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Thu, 2014-06-26 at 10:20 -0400, Simo Sorce wrote:
On Thu, 2014-06-26 at 15:33 +0300, Alexander Bokovoy wrote:
On Thu, 26 Jun 2014, Martin Kosek wrote:
On 06/26/2014 04:29 AM, Nathaniel McCallum wrote:
On Mon, 2014-06-23 at 17:24 -0400, Nathaniel McCallum wrote:
On Mon, 2014-06-23 at 14:35 -0400, Simo Sorce wrote:
- Original Message -
- Original Message -
Can you check if ipaProtectedOperation is in the aci attribute in
the
base tree object ?
It should be there as excluded, and that should cause admin to not
be
able to retrieve keytabs.
It was not. While running ipa-ldap-updater I got the following:
InvalidSyntax: ACL Syntax Error(-5):(targetattr=
\22ipaProtectedOperation;write_keys\22)(version 3.0; acl \22Admins
are
allowed to rekey any entity\22; allow(write) groupdn =
\22ldap:///cn=admins: Invalid syntax.
Uhmm I do not see anything obviously wrong with ACI instruction, it
looks
just like the one I replace, Ideas ?
Do you have ipaProtectedOperation in the schema ?
(I rebased patch 3 but will wait to send a patchset until we
understand (and
fix) why this is failing to update.
Ok, apparently it was a quoting issue in the .update files, hopefully
that's
the only issue (I am at a conference today and do not have my test
env. handy).
The attached patches are rebased on the latest master.
0001: Line 555 has very wrong indentation.
I don't see anything else wrong in the other patches. I've tested
everything and it works as designed.
I have CC'd everyone who was involved with review at any point on these
patches. This serves as my public notice that I'd like to ACK the next
round of patches. If anyone has anything else to add, please do it
before tomorrow evening. Thanks!
Nathaniel
ACK
Nathaniel
Pushed all 6 patches to master. Thanks for careful review!
Unfortunately, at least enctype marshalling is wrong with these patches.
Samba does not work anymore with the keytab fetched in new version.
We see following in the keytab:
Keytab name: FILE:/etc/samba/samba.keytab
KVNO Timestamp Principal
-
1 06/26/2014 13:03:01
cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com
(etype 274)
1 06/26/2014 13:03:01
cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com
(etype 273)
1 06/26/2014 13:03:01
cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com
(etype 272)
1 06/26/2014 13:03:01
cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat@dom136.tbad.idm.lab.eng.brq.redhat.com
(etype 279)
Note that etype is unresolvable. In the build without these patches we
get something like
1 06/23/2014 16:28:59
cifs/vm-139.dom139.tbad.idm.lab.eng.brq.redhat@dom139.tbad.idm.lab.eng.brq.redhat.com
(aes256-cts-hmac-sha1-96)
So this patchset needs an improvement before release.
Working on this.
I know, roughly what's going on, but still trying to pinpoint exactly
the offender. (It is the ber marshalling/unmarshalling indeed).
Simo.
The attached patch fixes it for me.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
From 1b69d2248c19e82b811ff80d5fe39b6012c0ea52 Mon Sep 17 00:00:00 2001
From: Simo Sorce s...@redhat.com
Date: Thu, 26 Jun 2014 11:43:47 -0400
Subject: [PATCH] Fix getkeytab code to always use implicit tagging.
A mixture of implicit and explicit tagging was being used and this caused
a bug in retrieving the enctype number due to the way ber_scanf() loosely
treat sequences and explicit tagging.
The ASN.1 notation used to describe the getkeytab operation uses implicit
tagging, so by changing the code we simply follow to the specified encoding.
Resolves: https://fedorahosted.org/freeipa/ticket/4404
Signed-off-by: Simo Sorce s...@redhat.com
---
daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c | 8
ipa-client/ipa-getkeytab.c | 8
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
index 90a92f1eff054e0667323e5cf6222bc40436cf9e..b0cdea315913dbfdce3dead7a2054b6fa917a9ae 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -1328,7 +1328,7 @@ static int decode_getkeytab_request(struct berval *extop, bool *wantold,
}
/* ber parse code */
-ttag = ber_scanf(ber, {t[a], ctag, svcname);
+ttag = ber_scanf(ber, {ta, ctag, svcname);
if (ttag == LBER_ERROR || ctag != GKREQ_SVCNAME_TAG) {
LOG_FATAL(ber_scanf failed to decode service name\n);
err_msg = Invalid payload.\n;
@@ -1378,7
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Thu, 2014-06-26 at 10:37 +0200, Martin Kosek wrote:
On 06/26/2014 04:29 AM, Nathaniel McCallum wrote:
On Mon, 2014-06-23 at 17:24 -0400, Nathaniel McCallum wrote:
On Mon, 2014-06-23 at 14:35 -0400, Simo Sorce wrote:
- Original Message -
- Original Message -
Can you check if ipaProtectedOperation is in the aci attribute in the
base tree object ?
It should be there as excluded, and that should cause admin to not be
able to retrieve keytabs.
It was not. While running ipa-ldap-updater I got the following:
InvalidSyntax: ACL Syntax Error(-5):(targetattr=
\22ipaProtectedOperation;write_keys\22)(version 3.0; acl \22Admins are
allowed to rekey any entity\22; allow(write) groupdn =
\22ldap:///cn=admins: Invalid syntax.
Uhmm I do not see anything obviously wrong with ACI instruction, it looks
just like the one I replace, Ideas ?
Do you have ipaProtectedOperation in the schema ?
(I rebased patch 3 but will wait to send a patchset until we understand
(and
fix) why this is failing to update.
Ok, apparently it was a quoting issue in the .update files, hopefully
that's
the only issue (I am at a conference today and do not have my test env.
handy).
The attached patches are rebased on the latest master.
0001: Line 555 has very wrong indentation.
I don't see anything else wrong in the other patches. I've tested
everything and it works as designed.
I have CC'd everyone who was involved with review at any point on these
patches. This serves as my public notice that I'd like to ACK the next
round of patches. If anyone has anything else to add, please do it
before tomorrow evening. Thanks!
Nathaniel
ACK
Nathaniel
Pushed all 6 patches to master. Thanks for careful review!
Not sure what happened but the indentation issue I sent a patch for was
not fixed in the final push and instead of a tab now there are 4 spaces:
Attached find patch that fixes the issue as seen in master.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
From dc0a99c688e697daefeca36d6773aa2b402ee715 Mon Sep 17 00:00:00 2001
From: Simo Sorce s...@redhat.com
Date: Thu, 26 Jun 2014 13:49:33 -0400
Subject: [PATCH] Fix incorrect indentation
---
daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
index b0cdea315913dbfdce3dead7a2054b6fa917a9ae..ca021cac71da690a498fe3003fae1babb30456c1 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -1073,7 +1073,7 @@ static int encode_setkeytab_reply(struct ipapwd_keyset *kset,
for (int i = 0; i kset-num_keys; i++) {
rc = ber_printf(ber, {i}, (ber_int_t)kset-keys[i].key_data_type[0]);
-if (rc == -1) {
+if (rc == -1) {
rc = LDAP_OPERATIONS_ERROR;
LOG_FATAL(Failed to ber_printf the enctype);
goto done;
--
1.9.3
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Mon, 2014-06-23 at 17:24 -0400, Nathaniel McCallum wrote: On Mon, 2014-06-23 at 14:35 -0400, Simo Sorce wrote: - Original Message - - Original Message - Can you check if ipaProtectedOperation is in the aci attribute in the base tree object ? It should be there as excluded, and that should cause admin to not be able to retrieve keytabs. It was not. While running ipa-ldap-updater I got the following: InvalidSyntax: ACL Syntax Error(-5):(targetattr= \22ipaProtectedOperation;write_keys\22)(version 3.0; acl \22Admins are allowed to rekey any entity\22; allow(write) groupdn = \22ldap:///cn=admins: Invalid syntax. Uhmm I do not see anything obviously wrong with ACI instruction, it looks just like the one I replace, Ideas ? Do you have ipaProtectedOperation in the schema ? (I rebased patch 3 but will wait to send a patchset until we understand (and fix) why this is failing to update. Ok, apparently it was a quoting issue in the .update files, hopefully that's the only issue (I am at a conference today and do not have my test env. handy). The attached patches are rebased on the latest master. 0001: Line 555 has very wrong indentation. I don't see anything else wrong in the other patches. I've tested everything and it works as designed. I have CC'd everyone who was involved with review at any point on these patches. This serves as my public notice that I'd like to ACK the next round of patches. If anyone has anything else to add, please do it before tomorrow evening. Thanks! Nathaniel ACK Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
- Original Message -
On Fri, 2014-06-20 at 16:05 -0400, Simo Sorce wrote:
On Fri, 2014-06-20 at 14:47 -0400, Nathaniel McCallum wrote:
This change would have very small impact on your patch set, but would
be
much clearer for the future consumers of this protocol. Code can be
changed; protocols can't.
Ok this new patchset implements the requested change.
Initial testing seem to indicate it all works as expected.
0001: Line 555 has very wrong indentation.
Attached a new patch 0001 hat fixes this.
Simo.From 214d1e167f9c9ea687f219ef74162e557ae496b0 Mon Sep 17 00:00:00 2001
From: Simo Sorce s...@redhat.com
Date: Tue, 17 Sep 2013 00:25:14 -0400
Subject: [PATCH 1/6] keytabs: Modularize setkeytab operation
In preparation of adding another function to avoid code duplication.
Related:
https://fedorahosted.org/freeipa/ticket/3859
---
.../ipa-pwd-extop/ipa_pwd_extop.c | 1112 +++-
1 file changed, 623 insertions(+), 489 deletions(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
index d8af3915300384933e621ffe8adea8904588985d..2648a53703fc6766bd2e29b830ea034fb519beff 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -93,8 +93,8 @@ void *ipapwd_get_plugin_id(void)
return ipapwd_plugin_id;
}
-static int filter_keys(struct ipapwd_krbcfg *krbcfg,
- struct ipapwd_keyset *kset)
+static void filter_keys(struct ipapwd_krbcfg *krbcfg,
+struct ipapwd_keyset *kset)
{
int i, j;
@@ -123,8 +123,6 @@ static int filter_keys(struct ipapwd_krbcfg *krbcfg,
i--;
}
}
-
-return 0;
}
static int ipapwd_to_ldap_pwpolicy_error(int ipapwderr)
@@ -562,49 +560,570 @@ free_and_return:
}
+static char *check_service_name(krb5_context krbctx, char *svc)
+{
+krb5_principal krbname = NULL;
+krb5_error_code krberr;
+char *name = NULL;
+
+krberr = krb5_parse_name(krbctx, svc, krbname);
+if (krberr) {
+LOG_FATAL(krb5_parse_name failed\n);
+} else {
+/* invert so that we get the canonical form (add REALM if not present
+ * for example) */
+krberr = krb5_unparse_name(krbctx, krbname, name);
+if (krberr) {
+LOG_FATAL(krb5_unparse_name failed\n);
+}
+}
+
+krb5_free_principal(krbctx, krbname);
+return name;
+}
+
+static Slapi_Backend *get_realm_backend(void)
+{
+Slapi_Backend *be;
+Slapi_DN *sdn;
+
+sdn = slapi_sdn_new_dn_byval(ipa_realm_dn);
+if (!sdn) return NULL;
+be = slapi_be_select(sdn);
+slapi_sdn_free(sdn);
+return be;
+}
+
+static const char *get_realm_base_dn(void)
+{
+const Slapi_DN *bsdn;
+Slapi_Backend *be;
+
+/* Find ancestor base DN */
+be = get_realm_backend();
+if (!be) return NULL;
+
+bsdn = slapi_be_getsuffix(be, 0);
+if (!bsdn) return NULL;
+
+return slapi_sdn_get_dn(bsdn);
+}
+
+static Slapi_Entry *get_entry_by_principal(const char *principal)
+{
+const char *bdn;
+char *filter = NULL;
+Slapi_PBlock *pb = NULL;
+char *attrlist[] = { krbPrincipalKey, krbLastPwdChange,
+ userPassword, krbPrincipalName,
+ enrolledBy, NULL };
+Slapi_Entry **es = NULL;
+int res, ret, i;
+Slapi_Entry *entry = NULL;
+
+/* Find ancestor base DN */
+bdn = get_realm_base_dn();
+if (!bdn) {
+LOG_TRACE(Search for Base DN failed\n);
+goto free_and_return;
+}
+
+filter = slapi_ch_smprintf((krbPrincipalName=%s), principal);
+if (!filter) {
+LOG_TRACE(Building filter failed\n);
+goto free_and_return;
+}
+
+pb = slapi_pblock_new();
+slapi_search_internal_set_pb(pb, bdn, LDAP_SCOPE_SUBTREE, filter,
+ attrlist, 0,
+ NULL, /* Controls */ NULL, /* UniqueID */
+ ipapwd_plugin_id, 0); /* Flags */
+
+/* do search the tree */
+ret = slapi_search_internal_pb(pb);
+slapi_pblock_get(pb, SLAPI_PLUGIN_INTOP_RESULT, res);
+if (ret == -1 || res != LDAP_SUCCESS) {
+LOG_TRACE(Search for Principal failed, err (%d)\n, res ? res : ret);
+goto free_and_return;
+}
+
+/* get entries */
+slapi_pblock_get(pb, SLAPI_PLUGIN_INTOP_SEARCH_ENTRIES, es);
+if (!es) {
+LOG_TRACE(No entries ?!);
+goto free_and_return;
+}
+
+/* count entries */
+for (i = 0; es[i]; i++) /* count */ ;
+
+/* if there is none or more than one, freak out */
+if (i != 1) {
+LOG_TRACE(Too many entries, or entry no found (%d), i);
+goto free_and_return;
+}
+entry = slapi_entry_dup(es[0]);
+
+free_and_return:
+if (pb) {
+slapi_free_search_results_internal(pb);
+slapi_pblock_destroy(pb);
+
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Fri, 2014-06-20 at 19:55 -0400, Simo Sorce wrote: On Fri, 2014-06-20 at 16:50 -0400, Nathaniel McCallum wrote: On Fri, 2014-06-20 at 16:05 -0400, Simo Sorce wrote: On Fri, 2014-06-20 at 14:47 -0400, Nathaniel McCallum wrote: This change would have very small impact on your patch set, but would be much clearer for the future consumers of this protocol. Code can be changed; protocols can't. Ok this new patchset implements the requested change. Initial testing seem to indicate it all works as expected. 0001: Line 555 has very wrong indentation. Other than that, I have looked over 0001 and 0002 very closely and built and tested them. Everything works. So conditional (indent fix) ACK on both of these. I don't see any reason to avoid merging these as soon as the indent fix is completed. It should substantially reduce your differential from master. In the new ASN.1, Newkeys has the wrong capitalization. This affects patches 0003 and 0005. I think patch 0003 may still have a permissions problem. For instance, this works for me with no error: $ ipa user-find --whoami -- 1 user matched -- User login: admin Last name: Administrator Home directory: /home/admin Login shell: /bin/bash UID: 156960 GID: 156960 Account disabled: False Password: True Kerberos keys available: True Number of entries returned 1 $ ipa-getkeytab -s ipa.example.com -p foo/ipa.example.com -r -k bar Keytab successfully retrieved and stored in: bar Is that really correct behavior or did I screw something up? I thought we had restricted the admin user from reading keys without changing them... Can you check if ipaProtectedOperation is in the aci attribute in the base tree object ? It should be there as excluded, and that should cause admin to not be able to retrieve keytabs. It was not. While running ipa-ldap-updater I got the following: InvalidSyntax: ACL Syntax Error(-5):(targetattr= \22ipaProtectedOperation;write_keys\22)(version 3.0; acl \22Admins are allowed to rekey any entity\22; allow(write) groupdn = \22ldap:///cn=admins: Invalid syntax. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Fri, 2014-06-20 at 19:55 -0400, Simo Sorce wrote: On Fri, 2014-06-20 at 16:50 -0400, Nathaniel McCallum wrote: On Fri, 2014-06-20 at 16:05 -0400, Simo Sorce wrote: On Fri, 2014-06-20 at 14:47 -0400, Nathaniel McCallum wrote: This change would have very small impact on your patch set, but would be much clearer for the future consumers of this protocol. Code can be changed; protocols can't. Ok this new patchset implements the requested change. Initial testing seem to indicate it all works as expected. 0001: Line 555 has very wrong indentation. Other than that, I have looked over 0001 and 0002 very closely and built and tested them. Everything works. So conditional (indent fix) ACK on both of these. I don't see any reason to avoid merging these as soon as the indent fix is completed. It should substantially reduce your differential from master. In the new ASN.1, Newkeys has the wrong capitalization. This affects patches 0003 and 0005. I think patch 0003 may still have a permissions problem. For instance, this works for me with no error: $ ipa user-find --whoami -- 1 user matched -- User login: admin Last name: Administrator Home directory: /home/admin Login shell: /bin/bash UID: 156960 GID: 156960 Account disabled: False Password: True Kerberos keys available: True Number of entries returned 1 $ ipa-getkeytab -s ipa.example.com -p foo/ipa.example.com -r -k bar Keytab successfully retrieved and stored in: bar Is that really correct behavior or did I screw something up? I thought we had restricted the admin user from reading keys without changing them... Can you check if ipaProtectedOperation is in the aci attribute in the base tree object ? It should be there as excluded, and that should cause admin to not be able to retrieve keytabs. Also, patch 0003 no longer applies cleanly to master. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
- Original Message - Can you check if ipaProtectedOperation is in the aci attribute in the base tree object ? It should be there as excluded, and that should cause admin to not be able to retrieve keytabs. It was not. While running ipa-ldap-updater I got the following: InvalidSyntax: ACL Syntax Error(-5):(targetattr= \22ipaProtectedOperation;write_keys\22)(version 3.0; acl \22Admins are allowed to rekey any entity\22; allow(write) groupdn = \22ldap:///cn=admins: Invalid syntax. Uhmm I do not see anything obviously wrong with ACI instruction, it looks just like the one I replace, Ideas ? Do you have ipaProtectedOperation in the schema ? (I rebased patch 3 but will wait to send a patchset until we understand (and fix) why this is failing to update. Simo. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Mon, 2014-06-23 at 14:35 -0400, Simo Sorce wrote: - Original Message - - Original Message - Can you check if ipaProtectedOperation is in the aci attribute in the base tree object ? It should be there as excluded, and that should cause admin to not be able to retrieve keytabs. It was not. While running ipa-ldap-updater I got the following: InvalidSyntax: ACL Syntax Error(-5):(targetattr= \22ipaProtectedOperation;write_keys\22)(version 3.0; acl \22Admins are allowed to rekey any entity\22; allow(write) groupdn = \22ldap:///cn=admins: Invalid syntax. Uhmm I do not see anything obviously wrong with ACI instruction, it looks just like the one I replace, Ideas ? Do you have ipaProtectedOperation in the schema ? (I rebased patch 3 but will wait to send a patchset until we understand (and fix) why this is failing to update. Ok, apparently it was a quoting issue in the .update files, hopefully that's the only issue (I am at a conference today and do not have my test env. handy). The attached patches are rebased on the latest master. 0001: Line 555 has very wrong indentation. I don't see anything else wrong in the other patches. I've tested everything and it works as designed. I have CC'd everyone who was involved with review at any point on these patches. This serves as my public notice that I'd like to ACK the next round of patches. If anyone has anything else to add, please do it before tomorrow evening. Thanks! Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote: Although the code is all done it would be nice to have a review of the feature, to see if it has all been captured: http://www.freeipa.org/page/V4/Keytab_Retrieval I'm a bit confused about the behavior of enctypes in the Request. A list of enctypes is always necessary in input when a new keytab is requested. However the list is filtered though the allowable enctypes list and if nothing is left the operation is refused. +1. However, the generated keys should be the set of allowed enctypes, not the intersection between allowed and requested enctypes. This would permit the later requesting of enctypes that were allowed at the time of creation, but not requested. If the getNew attribute is false, then the existing key is being requested. In this case password and enctypes MUST NOT be set. I don't get this. Shouldn't the return value of this request include only the intersection between allowed and requested enctypes? There is no point in responding with enctypes the client has not requested. And indeed, this provides extra data points to attack. Having this proposed behavior also means you can remove OPTIONAL from enctypes. So as it stands, enctypes currently controls what keys are generated. I would prefer that enctypes controls what keys are returned. Am I missing something? Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Fri, 2014-06-20 at 14:05 -0400, Nathaniel McCallum wrote: On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote: Although the code is all done it would be nice to have a review of the feature, to see if it has all been captured: http://www.freeipa.org/page/V4/Keytab_Retrieval I'm a bit confused about the behavior of enctypes in the Request. A list of enctypes is always necessary in input when a new keytab is requested. However the list is filtered though the allowable enctypes list and if nothing is left the operation is refused. +1. However, the generated keys should be the set of allowed enctypes, not the intersection between allowed and requested enctypes. This would permit the later requesting of enctypes that were allowed at the time of creation, but not requested. Which you absolutely do not want. If your service understands only RC4-HMAC and you provide it with AES keys then communication from a third party client will fail because the KDC will give that client an AES encrypted ticket that the service does not know how to decrypt. This is particularly important for old NFS Servers (like RHEL5) that understand only DES (sigh!) If the getNew attribute is false, then the existing key is being requested. In this case password and enctypes MUST NOT be set. I don't get this. Shouldn't the return value of this request include only the intersection between allowed and requested enctypes? Not when you are retrieving existing keys. Only when you are creating new keys. There is no point in responding with enctypes the client has not requested. And indeed, this provides extra data points to attack. ?? Having this proposed behavior also means you can remove OPTIONAL from enctypes. OPTIONAL is there because when you request existing keys it makes no sense to send a lit of enctypes, you get what the KDC has, the whole package, because you must be able to decrypt any ticket you get, getting a subset of keys would make no sense. So as it stands, enctypes currently controls what keys are generated. I would prefer that enctypes controls what keys are returned. Am I missing something? I most definitely think you are :-) Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Fri, 2014-06-20 at 14:10 -0400, Simo Sorce wrote:
On Fri, 2014-06-20 at 14:05 -0400, Nathaniel McCallum wrote:
On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote:
Although the code is all done it would be nice to have a review of the
feature, to see if it has all been captured:
http://www.freeipa.org/page/V4/Keytab_Retrieval
I'm a bit confused about the behavior of enctypes in the Request.
A list of enctypes is always necessary in input when a new keytab is
requested. However the list is filtered though the allowable enctypes
list and if nothing is left the operation is refused.
+1. However, the generated keys should be the set of allowed enctypes,
not the intersection between allowed and requested enctypes. This would
permit the later requesting of enctypes that were allowed at the time of
creation, but not requested.
Which you absolutely do not want.
If your service understands only RC4-HMAC and you provide it with AES
keys then communication from a third party client will fail because the
KDC will give that client an AES encrypted ticket that the service does
not know how to decrypt.
This is particularly important for old NFS Servers (like RHEL5) that
understand only DES (sigh!)
If the getNew attribute is false, then the existing key is being
requested. In this case password and enctypes MUST NOT be set.
I don't get this. Shouldn't the return value of this request include
only the intersection between allowed and requested enctypes?
Not when you are retrieving existing keys. Only when you are creating
new keys.
There is no point in responding with enctypes the client has not requested.
And indeed, this provides extra data points to attack.
??
Having this proposed behavior also means you can remove OPTIONAL from
enctypes.
OPTIONAL is there because when you request existing keys it makes no
sense to send a lit of enctypes, you get what the KDC has, the whole
package, because you must be able to decrypt any ticket you get, getting
a subset of keys would make no sense.
So as it stands, enctypes currently controls what keys are generated. I
would prefer that enctypes controls what keys are returned. Am I missing
something?
I most definitely think you are :-)
Ah, right. This boils down to the KDC not having any way to know what
enctypes the destination principal supports. Thanks for clarifying that.
In this case, the ASN.1 provided is confusing. I think something like
this would be clearer and less error prone:
KeytabGetMessage ::= CHOICE {
fetch[0] Fetch,
create [1] Create
reply[2] Reply
}
Fetch ::= SEQUENCE {
serviceIdentity [0] OCTET STRING
}
Create ::= SEQUENCE {
serviceIdentity [0] OCTET STRING,
enctypes[1] SEQUENCE OF Int16,
password[2] OCTET STRING OPTIONAL
}
This would also, I think, result in a cleaner implementation where the
type of operation is logically built into the decoding step.
Nathaniel
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Fri, 2014-06-20 at 14:30 -0400, Nathaniel McCallum wrote:
On Fri, 2014-06-20 at 14:10 -0400, Simo Sorce wrote:
On Fri, 2014-06-20 at 14:05 -0400, Nathaniel McCallum wrote:
On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote:
Although the code is all done it would be nice to have a review of the
feature, to see if it has all been captured:
http://www.freeipa.org/page/V4/Keytab_Retrieval
I'm a bit confused about the behavior of enctypes in the Request.
A list of enctypes is always necessary in input when a new keytab is
requested. However the list is filtered though the allowable enctypes
list and if nothing is left the operation is refused.
+1. However, the generated keys should be the set of allowed enctypes,
not the intersection between allowed and requested enctypes. This would
permit the later requesting of enctypes that were allowed at the time of
creation, but not requested.
Which you absolutely do not want.
If your service understands only RC4-HMAC and you provide it with AES
keys then communication from a third party client will fail because the
KDC will give that client an AES encrypted ticket that the service does
not know how to decrypt.
This is particularly important for old NFS Servers (like RHEL5) that
understand only DES (sigh!)
If the getNew attribute is false, then the existing key is being
requested. In this case password and enctypes MUST NOT be set.
I don't get this. Shouldn't the return value of this request include
only the intersection between allowed and requested enctypes?
Not when you are retrieving existing keys. Only when you are creating
new keys.
There is no point in responding with enctypes the client has not
requested.
And indeed, this provides extra data points to attack.
??
Having this proposed behavior also means you can remove OPTIONAL from
enctypes.
OPTIONAL is there because when you request existing keys it makes no
sense to send a lit of enctypes, you get what the KDC has, the whole
package, because you must be able to decrypt any ticket you get, getting
a subset of keys would make no sense.
So as it stands, enctypes currently controls what keys are generated. I
would prefer that enctypes controls what keys are returned. Am I missing
something?
I most definitely think you are :-)
Ah, right. This boils down to the KDC not having any way to know what
enctypes the destination principal supports. Thanks for clarifying that.
In this case, the ASN.1 provided is confusing. I think something like
this would be clearer and less error prone:
KeytabGetMessage ::= CHOICE {
fetch[0] Fetch,
create [1] Create
reply[2] Reply
}
Fetch ::= SEQUENCE {
serviceIdentity [0] OCTET STRING
}
Create ::= SEQUENCE {
serviceIdentity [0] OCTET STRING,
enctypes[1] SEQUENCE OF Int16,
password[2] OCTET STRING OPTIONAL
}
This would also, I think, result in a cleaner implementation where the
type of operation is logically built into the decoding step.
Well yes this looks better, but honestly, I think it is a bit late for
this kind of feedback, given I have a working patchset, and asked months
ago if people wanted to review the logical design.
How strongly do you feel we absolutely need to change this and go
through a new review cycle ?
Simo.
--
Simo Sorce * Red Hat, Inc * New York
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Fri, 2014-06-20 at 14:38 -0400, Simo Sorce wrote:
On Fri, 2014-06-20 at 14:30 -0400, Nathaniel McCallum wrote:
On Fri, 2014-06-20 at 14:10 -0400, Simo Sorce wrote:
On Fri, 2014-06-20 at 14:05 -0400, Nathaniel McCallum wrote:
On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote:
Although the code is all done it would be nice to have a review of the
feature, to see if it has all been captured:
http://www.freeipa.org/page/V4/Keytab_Retrieval
I'm a bit confused about the behavior of enctypes in the Request.
A list of enctypes is always necessary in input when a new keytab is
requested. However the list is filtered though the allowable enctypes
list and if nothing is left the operation is refused.
+1. However, the generated keys should be the set of allowed enctypes,
not the intersection between allowed and requested enctypes. This would
permit the later requesting of enctypes that were allowed at the time of
creation, but not requested.
Which you absolutely do not want.
If your service understands only RC4-HMAC and you provide it with AES
keys then communication from a third party client will fail because the
KDC will give that client an AES encrypted ticket that the service does
not know how to decrypt.
This is particularly important for old NFS Servers (like RHEL5) that
understand only DES (sigh!)
If the getNew attribute is false, then the existing key is being
requested. In this case password and enctypes MUST NOT be set.
I don't get this. Shouldn't the return value of this request include
only the intersection between allowed and requested enctypes?
Not when you are retrieving existing keys. Only when you are creating
new keys.
There is no point in responding with enctypes the client has not
requested.
And indeed, this provides extra data points to attack.
??
Having this proposed behavior also means you can remove OPTIONAL from
enctypes.
OPTIONAL is there because when you request existing keys it makes no
sense to send a lit of enctypes, you get what the KDC has, the whole
package, because you must be able to decrypt any ticket you get, getting
a subset of keys would make no sense.
So as it stands, enctypes currently controls what keys are generated. I
would prefer that enctypes controls what keys are returned. Am I missing
something?
I most definitely think you are :-)
Ah, right. This boils down to the KDC not having any way to know what
enctypes the destination principal supports. Thanks for clarifying that.
In this case, the ASN.1 provided is confusing. I think something like
this would be clearer and less error prone:
KeytabGetMessage ::= CHOICE {
fetch[0] Fetch,
create [1] Create
reply[2] Reply
}
Fetch ::= SEQUENCE {
serviceIdentity [0] OCTET STRING
}
Create ::= SEQUENCE {
serviceIdentity [0] OCTET STRING,
enctypes[1] SEQUENCE OF Int16,
password[2] OCTET STRING OPTIONAL
}
This would also, I think, result in a cleaner implementation where the
type of operation is logically built into the decoding step.
Well yes this looks better, but honestly, I think it is a bit late for
this kind of feedback, given I have a working patchset, and asked months
ago if people wanted to review the logical design.
You asked for a design review on Monday of this week. I am providing
that review. Do you want it or not?
How strongly do you feel we absolutely need to change this and go
through a new review cycle ?
This change would have very small impact on your patch set, but would be
much clearer for the future consumers of this protocol. Code can be
changed; protocols can't.
I am finishing the design doc now and will move to your patch set
shortly. I'd be happy to provide a quick code review for this change.
Nathaniel
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote: Although the code is all done it would be nice to have a review of the feature, to see if it has all been captured: http://www.freeipa.org/page/V4/Keytab_Retrieval Is there any need to create different permissions for password generation vs random generation? I think the answer is no because the only risk is a substandard password which you can control with password policy. I just wanted to bring it up. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Fri, 2014-06-20 at 15:50 -0400, Nathaniel McCallum wrote: On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote: Although the code is all done it would be nice to have a review of the feature, to see if it has all been captured: http://www.freeipa.org/page/V4/Keytab_Retrieval Is there any need to create different permissions for password generation vs random generation? I think the answer is no because the only risk is a substandard password which you can control with password policy. I just wanted to bring it up. Other than this question and the ASN.1 issue, I am very happy with the design. The new permissions ACI style looks very useful. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Fri, 2014-06-20 at 15:50 -0400, Nathaniel McCallum wrote: On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote: Although the code is all done it would be nice to have a review of the feature, to see if it has all been captured: http://www.freeipa.org/page/V4/Keytab_Retrieval Is there any need to create different permissions for password generation vs random generation? I think the answer is no because the only risk is a substandard password which you can control with password policy. I just wanted to bring it up. Indeed we do not differentiate, password policy is what you want to use if you want minimum guarantees, for now. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Fri, 2014-06-20 at 15:55 -0400, Nathaniel McCallum wrote: On Fri, 2014-06-20 at 15:50 -0400, Nathaniel McCallum wrote: On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote: Although the code is all done it would be nice to have a review of the feature, to see if it has all been captured: http://www.freeipa.org/page/V4/Keytab_Retrieval Is there any need to create different permissions for password generation vs random generation? I think the answer is no because the only risk is a substandard password which you can control with password policy. I just wanted to bring it up. Other than this question and the ASN.1 issue, I am very happy with the design. The new permissions ACI style looks very useful. Thanks, I changed the page to reflect the new controls too. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Fri, 2014-06-20 at 16:05 -0400, Simo Sorce wrote: On Fri, 2014-06-20 at 14:47 -0400, Nathaniel McCallum wrote: This change would have very small impact on your patch set, but would be much clearer for the future consumers of this protocol. Code can be changed; protocols can't. Ok this new patchset implements the requested change. Initial testing seem to indicate it all works as expected. 0001: Line 555 has very wrong indentation. Other than that, I have looked over 0001 and 0002 very closely and built and tested them. Everything works. So conditional (indent fix) ACK on both of these. I don't see any reason to avoid merging these as soon as the indent fix is completed. It should substantially reduce your differential from master. In the new ASN.1, Newkeys has the wrong capitalization. This affects patches 0003 and 0005. I think patch 0003 may still have a permissions problem. For instance, this works for me with no error: $ ipa user-find --whoami -- 1 user matched -- User login: admin Last name: Administrator Home directory: /home/admin Login shell: /bin/bash UID: 156960 GID: 156960 Account disabled: False Password: True Kerberos keys available: True Number of entries returned 1 $ ipa-getkeytab -s ipa.example.com -p foo/ipa.example.com -r -k bar Keytab successfully retrieved and stored in: bar Is that really correct behavior or did I screw something up? I thought we had restricted the admin user from reading keys without changing them... Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Fri, 2014-06-20 at 16:50 -0400, Nathaniel McCallum wrote: On Fri, 2014-06-20 at 16:05 -0400, Simo Sorce wrote: On Fri, 2014-06-20 at 14:47 -0400, Nathaniel McCallum wrote: This change would have very small impact on your patch set, but would be much clearer for the future consumers of this protocol. Code can be changed; protocols can't. Ok this new patchset implements the requested change. Initial testing seem to indicate it all works as expected. 0001: Line 555 has very wrong indentation. Other than that, I have looked over 0001 and 0002 very closely and built and tested them. Everything works. So conditional (indent fix) ACK on both of these. I don't see any reason to avoid merging these as soon as the indent fix is completed. It should substantially reduce your differential from master. In the new ASN.1, Newkeys has the wrong capitalization. This affects patches 0003 and 0005. I think patch 0003 may still have a permissions problem. For instance, this works for me with no error: $ ipa user-find --whoami -- 1 user matched -- User login: admin Last name: Administrator Home directory: /home/admin Login shell: /bin/bash UID: 156960 GID: 156960 Account disabled: False Password: True Kerberos keys available: True Number of entries returned 1 $ ipa-getkeytab -s ipa.example.com -p foo/ipa.example.com -r -k bar Keytab successfully retrieved and stored in: bar Is that really correct behavior or did I screw something up? I thought we had restricted the admin user from reading keys without changing them... Can you check if ipaProtectedOperation is in the aci attribute in the base tree object ? It should be there as excluded, and that should cause admin to not be able to retrieve keytabs. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On 06/16/2014 05:34 PM, Simo Sorce wrote: Although the code is all done it would be nice to have a review of the feature, to see if it has all been captured: http://www.freeipa.org/page/V4/Keytab_Retrieval Simo. Thanks! I was not deeply involved in the review, but from the high level it looks good to me. I at least fixed typos and improved formatting. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Tue, 2014-06-17 at 09:18 +0200, Martin Kosek wrote: On 06/16/2014 05:34 PM, Simo Sorce wrote: Although the code is all done it would be nice to have a review of the feature, to see if it has all been captured: http://www.freeipa.org/page/V4/Keytab_Retrieval Simo. Thanks! I was not deeply involved in the review, but from the high level it looks good to me. I at least fixed typos and improved formatting. Thanks a lot for the fixes and the improved links throughout the page :) Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
