Re: [Freeipa-devel] [PATCH] 260-262 Global trust configuration page

[Endi Sukma Dewata]

On 3/12/2013 11:35 AM, Petr Vobornik wrote:

Another problem is that hidden 'Default SMB Group' is not listed.
Hence it couldn't be set again after a modification. I made the
combobox
editable (first usage, so it revealed a bug) to avoid this problem.
User can enter garbage, but the framework should handle that.


This is a little difficult to use. You'll need to know that you have to
type 'Default SMB Group' to go back to the default and the UI doesn't
show that as an acceptable value. Possible solutions:

1. Add the 'Default SMB Group' as the first entry in the drop down list
so you can reselect it again. The drop down list doesn't need to be
editable.

2. Use radio buttons to separate the default value from other values:

   Fallback primary group: (o) Default SMB Group
   ( ) POSIX group: [ drop down list ]

Regardless, I think the server API needs to be changed to accept an
empty value to go back to the default value instead of taking 'Default
SMB Group'. A default value should be simple and not something that will
potentially conflict with a non-default value that happens to have the
same name.


I agree. Martin is it feasible?


 From other emails: Apparently its not worth the time. So I implemented #1.


OK.


3. Found an issue: trusctconfig-show will raise 'not found' error when
trusts are not configured on a system. It's fixed separately in attached
patch #268. It has similar behavior as dns pages.

Also I added data .json files to patch 262 and 268, so the static
version can be used.


The static page and the code looks good, but do you have a live server 
that I could take a look at?


There are some other issues with the static page:

When loading the UI, cert entity initialization fails saying "entity is 
undefined".


Also, the trust details page has 2 sections, but the second section has 
a blank title and 'undefined' fields. Did I miss a patch?


--
Endi S. Dewata

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

Re: [Freeipa-devel] [PATCH] 267 Filter groups by type (normal, posix, external)

[Endi Sukma Dewata]

On 3/12/2013 11:28 AM, Petr Vobornik wrote:

Here's a patch for filtering groups by type.
Design page: http://www.freeipa.org/page/V3/Filtering_groups_by_type

The interface is:

StrEnum('type?',
cli_name='type',
label=_('Type'),
doc=_('Group type'),
values=(u'posix', u'normal', u'external'),
),


I have two design questions.
1. Is --type the right option name?


Fine by me, it matches the label and description.


2. Is `normal` the right name for non-posix, non-external group? The
default group type (when adding group) is posix. Should the name be
something else: `simple`, `plain`, `ordinary`?


We also use 'normal' in the group adder dialog, so it's consistent. 
Other options are 'basic', 'standard', 'regular'.



I didn't want to create an option for each type. IMO it brings more
complexity.


Maybe the group-add/mod command should use the same --type option?


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


ACK from me, but maybe others might have some comments.

--
Endi S. Dewata

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

Re: [Freeipa-devel] [PATCH] 266 Fixed Web UI build error caused by rhino changes in F19

[Endi Sukma Dewata]

On 3/11/2013 8:27 AM, Petr Vobornik wrote:

rhino-1.7R4-2.fc19.noarch dropped -main flag which made the build fail
in rawhide (F19).

We can't use the same command for rhino-1.7R3-6 (F18) and rhino-1.7R4-2
(F19).
This patch adds check if rhino supports '-require' option. If so it
calls rhino with it if not it calls rhino with -main option.

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


Would it be better to check /etc/fedora-release or the actual rhino version?

Regardless, this patch is ACKed. I don't have an F19 to test it though.

--
Endi S. Dewata

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

Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE

[Endi Sukma Dewata]

On 3/7/2013 7:37 AM, Petr Vobornik wrote:

Ideally it should be generic enough to combine any widgets. This might
be a common scenario somewhere else:

Something: ( ) Option 1
   ( ) Option 2
   (o) Other: [something else   ]


This design has a flaw:
https://fedorahosted.org/freeipa/ticket/3404#comment:5


I think this one makes the most sense to me:

PAC Types: ( ) Inherited setting: ...  ...
   (o) Override inherited setting
   [ ] MS-PAC
   [ ] PAD

It looks like the NONE option is identical to not using the inherited 
values but also not selecting any new values, so we don't actually need 
a separate radio button for NONE because it can be represented by the 
above UI without redundancy. We just need better labels to explain the 
radio buttons. Maybe someone can come up with better labels than these.



I implemented following design:
https://fedorahosted.org/freeipa/ticket/3404#comment:7


It works but I can't imagine how it would look if we have more than two 
PAC types. I don't think we want to list every possible combinations. 
The above design is more future proof.



Patch attached (255-1).

I have a dilemma. I practically implemented the previous design (and
then I've found the flaw..). Patches attached as wip-fre... I wonder if
we can use it somehow or we should ditch it.


--
Endi S. Dewata

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

Re: [Freeipa-devel] [PATCH] 264-265 Web UI:Certificate pages

[Endi Sukma Dewata]

On 3/6/2013 9:51 AM, Petr Vobornik wrote:

1. As previously discussed, the cert-find is only available with Dogtag
CA, so the Certificates page should be disabled with self-signed CA. But
if self-signed CA is dropped, then it's not necessary to disable the
Certificates page. We may need to keep the ticket open until this is
resolved one way or another.


Seems that the drop will land in different release. I disabled it
similar way as in DNS. I didn't test it. Do you have installation
without dogtag?


Yes, the cert page now doesn't appear in the self-sign mode.


2. Right now the fields in the cert details page are editable although
there is no Update button. If you change the value an Undo button will
appear. If you try to leave the page it will show the Unsaved Changes
dialog. Since there's no cert-mod operation these fields should not be
editable.


The certificate field is still editable. There's an undo button and 
you'll get a 'page dirty' dialog. Could we make it read-only text field?



3. In the cert details page if you revoke the cert it will work but an
error will appear below the revocation reason field saying 'Must be an
integer'.


#2, #3: I forget to tell you, that this patch also depends on my patch
#261 - Fix handling of no_update flag in Web UI. Sorry. It should fix
it. It was pushed to master today.


Everything else is fixed except for the certificate field above.


4. I think showing an empty revocation reason field on a valid cert is
kind of weird. It might be better to create a Status section with two
fields: status and revocation reason. If status is valid the revocation
reason will be empty or hidden. If revoked then the reason will appear
below the status.


Fixed it by hiding the row.

Showing the status is separate issue. I would like to avoid computing it
in web ui - IIUIC it depends also on valid-until and similar fields.
Cert-show command doesn't include the status.


OK, let's address this separately. The search page shows the status, so 
it would make sense if the details page also shows the status.



5. In host/service details page the View link can be changed to go to
the cert details page instead of showing a dialog box.


Do we want to do it?


That's probably a question for UXD whether we should stay in the 
host/service page or move to the cert page (and probably not having a 
link back to the host/service page). Also, in self-sign mode there won't 
be a cert page to link to. Let's leave it as is for now.



6. It would be better to organize the fields in the cert details page
like the cert view dialog in host/service details page.


Switched MD5fp. with SHA1fp.

Do you also want to split subject and issuer as in the dialog and moved
the fields to different sections?

Currently the order is the same as in dialog except serial numbers. I
kept them on top because issuer and subject may be very long and they
can clash with the action panel if placed on top.


I'm OK with the current page, but maybe UXD has a different opinion. 
This can be addressed separately.



7. Certificate can be added/revoked/restored via certificate pages and
host/service details pages. We need to make sure that if you do an
operation on one page, the other pages won't show outdated information.


Fixed.


I added a cert via host details page. When I go to the cert search page 
the new cert doesn't appear until I click Refresh. The revoke/restore 
works though, the status are updated automatically in all pages.



8. The revocation reason takes an integer. Probably the search field
should change into a drop down list showing all available reasons.

9. The date options take a certain format (-MM-DD), so we should
show the format probably as greyed text in the search field.

10. The current design only allows specifying one option at a time. Some
of these options are meant to be used as a pair because they represent a
range (min & max serial number). How about creating an Advanced Search
dialog that shows all search options in separate fields so they can be
combined? The basic search field can remain simple like the search field
in other entities and it will search the cert subject only.


8-10: I agree, but I don't want to complicate the patch. Originally this
patch shouldn't really exist. Original agreement was that the search
will be fixed to 'subject' field and that it should be replaced with
proper implementation of #191 later. I made this patch to give users at
least some options. Main obstacle is that there are more pressing issues
for April release.


OK, let leave it as is for now, but some people might be reporting 
usability issue.



11. The list of search options is a drop down list, but it's surrounded
by a rounded box like the text field next to it. This might be just a
personal preference but I'm not sure if it's an appropriate look for a
drop down list.


It's just border radius. When I disable it it looks kinda inconsistent,
weirder.


OK, this is fine.

So the remaining iss

Re: [Freeipa-devel] master is broken on F18

[John Dennis]

On 03/15/2013 12:56 PM, Alexander Bokovoy wrote:

Hi!

I was investigating why installing master fails on F18 +
updates-testing and found out that install fails with
freeipa-server-3.1.99-0.20130313T1838Zgit158bf45.fc18.x86_64 from
ipa-devel repo

2013-03-15T16:17:40Z DEBUG args=/usr/bin/certutil -d /etc/httpd/alias
-R -s CN=jano.ipa.team,O=IPA.TEAM -o
/var/lib/ipa/ipa-aza7Wg/tmpcertreq -k rsa -g 2048 -z
/etc/httpd/alias/noise.txt -f /etc/httpd/alias/pwdfile.txt -a
2013-03-15T16:17:41Z DEBUG Process finished, return code=0
2013-03-15T16:17:41Z DEBUG stdout= 2013-03-15T16:17:41Z DEBUG
stderr=

Generating key.  This may take a few moments...


I believe this is a known problem in certutil where it writes data to
the wrong file descriptor. The problem was fixed upstream about 10 days
ago, I'm not sure if Fedora has the fix yet or not. Kai would know, I've
added him on the cc list.

John

--
John Dennis 

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

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

[Freeipa-devel] master is broken on F18

[Alexander Bokovoy]

Hi!

I was investigating why installing master fails on F18 + updates-testing and 
found out that install fails with 
freeipa-server-3.1.99-0.20130313T1838Zgit158bf45.fc18.x86_64 from ipa-devel repo

2013-03-15T16:17:40Z DEBUG args=/usr/bin/certutil -d /etc/httpd/alias -R -s 
CN=jano.ipa.team,O=IPA.TEAM -o /var/lib/ipa/ipa-aza7Wg/tmpcertreq -k rsa -g 
2048 -z /etc/httpd/alias/noise.txt -f /etc/httpd/alias/pwdfile.txt -a
2013-03-15T16:17:41Z DEBUG Process finished, return code=0
2013-03-15T16:17:41Z DEBUG stdout=
2013-03-15T16:17:41Z DEBUG stderr=

Generating key.  This may take a few moments...


2013-03-15T16:17:41Z DEBUG request 
'https://jano.ipa.team:8443/ca/ee/ca/profileSubmitSSLClient'
2013-03-15T16:17:41Z DEBUG request body 
'profileId=caIPAserviceCert&requestor_name=IPA+Installer&cert_request=MIICcDCCAVgCAQAwKzERMA8GA1UEChMISVBBLlRFQU0xFjAUBgNVBAMTDWphbm8u%0D%0AaXBhLnRlYW0wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDOaJLU2s6L%0D%0A5Dxvmmc8fNsYDV760P1zyK69NjcNgx1oeSLY97AgWHdLh6hihAChgHN5ZFI0paQl%0D%0A%2B2J9tED41JnUHrHjBaqivBzUpYNGeyjquGeqk8cB7owGR5Rylu%2FKeaCqR8r3Kzc5%0D%0AUuCyG%2FLjFlD%2FGCqjxuqmjyBfcZxXGz6L72DaB9IZq0uX6Q4rYbK7DzP3va1%2B4UnZ%0D%0AqqgHZwDe73TTjuw9PiKvSvI2ocHCk6ui4YT4qr1YOol7Z18woYIwnukHQygD1iAT%0D%0ApZzHZJ191XD0k5vD3wCaOGJsYGF4q1kPeFewBIB6fMuydWX09kVNgqiWmGcsz83o%0D%0ASQrLQNU72H5VAgMBAAGgADANBgkqhkiG9w0BAQUFAAOCAQEAFyT4t5oH894tDfuP%0D%0Abf0dqkDA%2F%2Fk738MI98GyyciRDhFFr18YizBtWNOx8Bl8c28O4J2Y9N%2BkmA%2BzPsKm%0D%0AC1W3Np%2Bh%2BqY5lNgK9XfQU5RXmlqPcqz588mHQgzjxePQP7od4fOVbFV03CCXjb9G%0D%0AtovS6wRrq909IxWSQ%2BNFM4S0OogCClihCy0%2FyDylpn4pYOvJCFl5xfGO3o4vrZC9%0D%0AM5tGhUIeD7H2dvMApRFKu6N7xI%2BmHfYiCKWGl8i2Mo%2FmQRX5zaHAyzrNMgqO2lNm%0D%0AjR%2B8U0H!
 
Jc%2B8ujeJq9JYCGFDpi3SW93U4E15pBHfwr31UWjtOiqgypBtUlVwdEKFu%0D%0A6QIHIA%3D%3D%0A&cert_request_type=pkcs10&xmlOutput=true'
2013-03-15T16:17:41Z DEBUG NSSConnection init jano.ipa.team
2013-03-15T16:17:41Z INFO   File 
"/usr/lib/python2.7/site-packages/ipaserver/install/installutils.py", line 612, 
in run_script
return_value = main_function()

  File "/usr/sbin/ipa-server-install", line 1088, in main
http.create_instance(realm_name, host_name, domain_name, dm_password, 
autoconfig=True, self_signed_ca=options.selfsign, subject_base=options.subject, 
auto_redirect=options.ui_redirect)

  File "/usr/lib/python2.7/site-packages/ipaserver/install/httpinstance.py", 
line 101, in create_instance
self.start_creation(runtime=60)

  File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py", line 
359, in start_creation
method()

  File "/usr/lib/python2.7/site-packages/ipaserver/install/httpinstance.py", 
line 259, in __setup_ssl
self.dercert = db.create_server_cert("Server-Cert", self.fqdn, ca_db)

  File "/usr/lib/python2.7/site-packages/ipaserver/install/certs.py", line 565, 
in create_server_cert
cdb.issue_server_cert(self.certreq_fname, self.certder_fname)

  File "/usr/lib/python2.7/site-packages/ipaserver/install/certs.py", line 661, 
in issue_server_cert
self.secdir, password, "ipaCert", **params)

  File "/usr/lib/python2.7/site-packages/ipapython/dogtag.py", line 236, in 
https_request
'https', host, port, url, connection_factory, body)

  File "/usr/lib/python2.7/site-packages/ipapython/dogtag.py", line 295, in 
_httplib_request
raise NetworkError(uri=uri, error=str(e))

2013-03-15T16:17:41Z INFO The ipa-server-install command failed, exception: 
NetworkError: cannot connect to 
'https://jano.ipa.team:8443/ca/ee/ca/profileSubmitSSLClient': (SEC_ERROR_BUSY) 
NSS could not shutdown. Objects are still in use.

while installing a build before 4152c36 does not fail.

So, something from following patches fails:
$ git log --oneline 4152c36^..HEAD
8de6c3f Remove check for alphabetic only characters from domain name validation
c8846ba Improve some error handling in ipa-replica-manage
66356f0 Improve error messages for external group members
c4ab8da Do not force named connections on upgrades
7a2d380 Use tkey-gssapi-keytab in named.conf
ca6f7f2 Update named.conf parser
04a17f0 Enforce exact SID match when adding or modifying a ID range
354a5db Avoid multiple client discovery with fixed server list
452ffa1 Preserve order of servers in ipa-client-install
158bf45 Do not hide idrange-add errors when adding trust
dcc6f13 Use new 389-ds-base cleartext password API
99b62aa Remove implicit Str to DN conversion using *-attr
ade4aae Make sure uninstall script prompts for reboot as last
9005b9b Extend ipa-replica-manage to be able to manage DNA ranges.
63407ed Don't download the schema in ipadiscovery
cf4b521 Remove unneeded python-ldap imports
664248d Use IPAdmin rather than raw python-ldap in migration.py and 
ipadiscovery.py
a024233 Use IPAdmin rather than raw python-ldap in ipa-client-install
91a63cc Remove ipaserver/ipaldap.py
4e6a2a9 Move ipaldap to ipapython
a38d93f Add support for re-enrolling hosts using keytab
91606e6 Change DNA magic value to -1 to make UID 999 usable
8d43235 Perform secondary rid range overla

Re: [Freeipa-devel] [PATCH] 1092 Fix LDAP lockout plugin

[Rob Crittenden]

Rob Crittenden wrote:

Martin Kosek wrote:

On 03/11/2013 10:07 PM, Rob Crittenden wrote:

Fixed a number of issues applying password policy against LDAP binds.
See patch
for details.

rob



I see some issues with this fix:

1) Shouldn't group password policy serve only as an override to the main
policy? I.e. if I have this policy:

# ipa pwpolicy-show test
   Group: test
   Priority: 10
   Max failures: 2

We should still follow settings other than "Max failures" configured in
global policy, right? At least the Kerberos seem to do it. I think we
should be consistent in this case. Now, other values just seem to be
zero.


There should be only one policy. It isn't supposed to merge policies
together (there is only one krbPwdPolicyReference per principal).

How is the KDC acting differently?


I think we will need to fix both the pre-op and the post-op to make this
working really consistently.

2) The lockout post-op still counts failed logins even though we are in
lockout time, is this expected? It is another point if inconsistency
with Kerberos auth. It leaves user's krbloginfailedcount stay on "Max
failures".


Ok.



3) Sometimes, I get into a state when I lockout a new user with Kerberos
and then wait some time until the lockout time passes (no admin unlock),
I am able to run as many LDAP binds as I want.


Can you clarify? Successful or unsuccessful binds?


This is all I found so far. Honza is also reviewing it, so I will let
him post hist findings too.

Martin


Here is an updated patch to not increment past the max failures on LDAP 
binds.


I couldn't reproduce your 3rd point.

rob

>From 196e3e4158bcefbc8cd417c0b6410b931472 Mon Sep 17 00:00:00 2001
From: Rob Crittenden 
Date: Fri, 15 Feb 2013 11:51:59 -0500
Subject: [PATCH] Fix lockout of LDAP bind.

There were several problems:

- A cut-n-paste error where the wrong value was being considered when
  an account was administratively unlocked.
- An off-by-one error where LDAP got one extra bind attempt.
- krbPwdPolicyReference wasn't being retrieved as a virtual attribute so
  only the global_policy was used.
- The lockout duration wasn't examined in the context of too many failed
  logins so wasn't being applied properly.
- The failedcount is no longer updated past max failures.

https://fedorahosted.org/freeipa/ticket/3433
---
 .../ipa-slapi-plugins/ipa-lockout/ipa_lockout.c| 149 +
 1 file changed, 94 insertions(+), 55 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c b/daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c
index fea997d79825e05a6ffe4bf65e22821948794ff3..9e8920af96aae757be54f98836dd14dfdd7d3c13 100644
--- a/daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c
+++ b/daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c
@@ -226,6 +226,50 @@ done:
 return ret;
 }
 
+int ipalockout_getpolicy(Slapi_Entry *target_entry, Slapi_Entry **policy_entry,
+ Slapi_ValueSet** values, char **actual_type_name,
+ const char **policy_dn, int *attr_free_flags,
+ char **errstr)
+{
+int ldrc = 0;
+int flags = 0;
+int type_name_disposition = 0;
+Slapi_DN *pdn = NULL;
+
+/* Only continue if there is a password policy */
+ldrc = slapi_vattr_values_get(target_entry, "krbPwdPolicyReference",
+values,
+&type_name_disposition, actual_type_name,
+SLAPI_VIRTUALATTRS_REQUEST_POINTERS,
+attr_free_flags);
+if (ldrc == 0) {
+Slapi_Value *sv = NULL;
+
+slapi_valueset_first_value(*values, &sv);
+
+if (values != NULL) {
+*policy_dn = slapi_value_get_string(sv);
+}
+}
+
+if (*policy_dn == NULL) {
+LOG_TRACE("No kerberos password policy\n");
+return LDAP_SUCCESS;
+} else {
+pdn = slapi_sdn_new_dn_byref(*policy_dn);
+ldrc = slapi_search_internal_get_entry(pdn, NULL, policy_entry,
+getPluginID());
+slapi_sdn_free(&pdn);
+if (ldrc != LDAP_SUCCESS) {
+LOG_FATAL("Failed to retrieve entry \"%s\": %d\n", *policy_dn, ldrc);
+*errstr = "Failed to retrieve account policy.";
+return LDAP_OPERATIONS_ERROR;
+}
+}
+
+return LDAP_SUCCESS;
+}
+
 int
 ipalockout_init(Slapi_PBlock *pb)
 {
@@ -346,7 +390,7 @@ ipalockout_close(Slapi_PBlock * pb)
 static int ipalockout_postop(Slapi_PBlock *pb)
 {
 char *dn = NULL;
-char *policy_dn = NULL;
+const char *policy_dn = NULL;
 Slapi_Entry *target_entry = NULL;
 Slapi_Entry *policy_entry = NULL;
 Slapi_DN *sdn = NULL;
@@ -360,6 +404,7 @@ static int ipalockout_postop(Slapi_PBlock *pb)
 unsigned long failedcount = 0;
 char failedcountstr[32];
 int failed_bind = 0;
+unsigned int max_fail = 0;
 struct tm utctime;
 time_t time_now;
 char timestr[GENERALIZ

Re: [Freeipa-devel] [PATCH] 1092 Fix LDAP lockout plugin

[Rob Crittenden]

Martin Kosek wrote:

On 03/11/2013 10:07 PM, Rob Crittenden wrote:

Fixed a number of issues applying password policy against LDAP binds.
See patch
for details.

rob



I see some issues with this fix:

1) Shouldn't group password policy serve only as an override to the main
policy? I.e. if I have this policy:

# ipa pwpolicy-show test
   Group: test
   Priority: 10
   Max failures: 2

We should still follow settings other than "Max failures" configured in
global policy, right? At least the Kerberos seem to do it. I think we
should be consistent in this case. Now, other values just seem to be zero.


There should be only one policy. It isn't supposed to merge policies 
together (there is only one krbPwdPolicyReference per principal).


How is the KDC acting differently?


I think we will need to fix both the pre-op and the post-op to make this
working really consistently.

2) The lockout post-op still counts failed logins even though we are in
lockout time, is this expected? It is another point if inconsistency
with Kerberos auth. It leaves user's krbloginfailedcount stay on "Max
failures".


Ok.



3) Sometimes, I get into a state when I lockout a new user with Kerberos
and then wait some time until the lockout time passes (no admin unlock),
I am able to run as many LDAP binds as I want.


Can you clarify? Successful or unsuccessful binds?


This is all I found so far. Honza is also reviewing it, so I will let
him post hist findings too.

Martin


rob

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

Re: [Freeipa-devel] [PATCH] 0006 Remove check for alphabetic only characters from domain name validation

[Martin Kosek]

On 03/15/2013 12:11 PM, Martin Kosek wrote:

On 03/15/2013 12:08 PM, Petr Spacek wrote:

On 14.3.2013 15:08, Martin Kosek wrote:

On 03/11/2013 01:02 PM, Ana Krivokapic wrote:

On 02/27/2013 10:58 AM, Martin Kosek wrote:

On 02/22/2013 04:02 PM, Ana Krivokapic wrote:

On 02/22/2013 10:19 AM, Petr Spacek wrote:

On 20.2.2013 11:03, Ana Krivokapic wrote:

On 02/18/2013 01:08 PM, Martin Kosek wrote:

On 02/18/2013 12:47 PM, Sumit Bose wrote:

On Mon, Feb 18, 2013 at 12:27:35PM +0100, Petr Spacek wrote:

On 15.2.2013 15:22, Ana Krivokapic wrote:

Hello,

The .isalpha() check in validate_domain_name() was too strict,
causing some commands like ipa dnsrecord-add to fail.

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

I would add --force option rather than removing whole check, if
it's possible.

Would it be possible to mention RFC in the error message? Something
like _('top level domain label must be alphabetic (RFC 1123 section
2.1)')
?

IMHO it is handy, because it educates users.

The problem is that this check is always done on the last component of
the domain_name even if it is just a sub-domain of the FreeIPA domain,
where e.g. numbers are valid characters.

At the beginning of validate_domain_name() a trailing '.' is stripped
away. iirc the trailing '.' is an indication for a complete, fully
qualified name. Would it work if the presence of the trailing '.' is
saved and the check is only done if there was a '.'?

bye,
Sumit


Sure. Though I am now not 100% sure that some IPA functions do not
use this
validator with a fqdn hostname without trailing dot. If not, I am
for fixing
this function as Sumit and Petr suggested.

Martin

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

After some thought, I decided to change the approach.

As pointed out by Sumit, the problem was that the validate_domain_name()
function did not distinguish between fqdn and non-fqdn domains
(subdomains of the IPA domain). The trailing dot is not a clear
indication either, because some IPA functions use this validator with an
fqdn without the trailing dot.

To fix this, I introduced an additional parameter to this function - a
flag which indicates whether the domain name is an fqdn or not. The is
.isalpha() check is then performed only in the case of an fqdn.

I also improved the error message to mention the relevant RFC, as
suggested by Petr.

Please don't forget to add --force switch. It could be handy.


I added the --force switch to ipa dnsrecord-add and opened a new ticket
to handle the rest of the ipa commands that use domain name validation:
https://fedorahosted.org/freeipa/ticket/3455

Updated patch is attached.


This patch fixed validation only partially. The --force flag you made
available
will not allow admin to for example add a zone "example.zone1" which
technically will be resolvable, it is just not a good practice:

# ipa dnszone-add example.zone1 --name-server `hostname`. --force
ipa: ERROR: invalid 'name': top level domain label must be alphabetic (RFC
1123
section 2.1)

To enable this, I think you would need to not postpone the validation to DNS
zone pre_callback as you could not check --force flag presence right in the
idnsname parameter validator.

We may also want to change --force flag label, it now talks only about NS
record validation, but we now expanded it a bit, so the label would need
to be
more general.

Martin


I added the fix for dnszone-add and edited the label of the --force flag
to make it more general.



The zone with this name can be created even without the --force flag.

# ipa dnszone-add example.zone1 --name-server `hostname`.
Administrator e-mail address [hostmaster.example.zone1.]:

Administrator e-mail address: top level domain label must be alphabetic

(RFC 1123 section 2.1)
Administrator e-mail address [hostmaster.example.zone1.]:
hostmaster.example.zone.
   Zone name: example.zone1
   Authoritative nameserver: vm-037.idm.lab.bos.redhat.com.
   Administrator e-mail address: hostmaster.example.zone.
   SOA serial: 1363268183
   SOA refresh: 3600
   SOA retry: 900
   SOA expire: 1209600
   SOA minimum: 3600
   BIND update policy: grant IDM.LAB.BOS.REDHAT.COM krb5-self * A; grant
IDM.LAB.BOS.REDHAT.COM krb5-self
   * ; grant IDM.LAB.BOS.REDHAT.COM krb5-self * SSHFP;
   Active zone: TRUE
   Dynamic update: FALSE
   Allow query: any;
   Allow transfer: none;

I thought that the check would only be surpassed with the --force flag.

All this struggle with this patch makes me thinking if we are not making it too
complicated. I am rather inclined to drop this particular check altogether. I
do not see that much harm if user creates an IPA managed zone "example.zone1".
Petr, what is your opinion on dropping this check in IPA? Is it OK to drop it?


IMHO we can drop the whole check, it should be safe. I did some basic tests
with latest bind-dyndb-ldap and it worked for me.

Re: [Freeipa-devel] [PATCH] 1092 Fix LDAP lockout plugin

[Jan Cholasta]

On 15.3.2013 11:19, Martin Kosek wrote:

I see some issues with this fix:

1) Shouldn't group password policy serve only as an override to the main
policy? I.e. if I have this policy:

# ipa pwpolicy-show test
   Group: test
   Priority: 10
   Max failures: 2

We should still follow settings other than "Max failures" configured in
global policy, right? At least the Kerberos seem to do it. I think we
should be consistent in this case. Now, other values just seem to be zero.

I think we will need to fix both the pre-op and the post-op to make this
working really consistently.


+1, noticed this as well.



2) The lockout post-op still counts failed logins even though we are in
lockout time, is this expected? It is another point if inconsistency
with Kerberos auth. It leaves user's krbloginfailedcount stay on "Max
failures".

3) Sometimes, I get into a state when I lockout a new user with Kerberos
and then wait some time until the lockout time passes (no admin unlock),
I am able to run as many LDAP binds as I want.

This is all I found so far. Honza is also reviewing it, so I will let
him post hist findings too.


The commit message says "was being applied properly", when it should say 
"was being applied improperly".


I have added steps to reproduce the issues the patch fixes to the 
ticket: 


Honza

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH] 0006 Remove check for alphabetic only characters from domain name validation

[Martin Kosek]

On 03/15/2013 12:08 PM, Petr Spacek wrote:

On 14.3.2013 15:08, Martin Kosek wrote:

On 03/11/2013 01:02 PM, Ana Krivokapic wrote:

On 02/27/2013 10:58 AM, Martin Kosek wrote:

On 02/22/2013 04:02 PM, Ana Krivokapic wrote:

On 02/22/2013 10:19 AM, Petr Spacek wrote:

On 20.2.2013 11:03, Ana Krivokapic wrote:

On 02/18/2013 01:08 PM, Martin Kosek wrote:

On 02/18/2013 12:47 PM, Sumit Bose wrote:

On Mon, Feb 18, 2013 at 12:27:35PM +0100, Petr Spacek wrote:

On 15.2.2013 15:22, Ana Krivokapic wrote:

Hello,

The .isalpha() check in validate_domain_name() was too strict,
causing some commands like ipa dnsrecord-add to fail.

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

I would add --force option rather than removing whole check, if
it's possible.

Would it be possible to mention RFC in the error message? Something
like _('top level domain label must be alphabetic (RFC 1123 section
2.1)')
?

IMHO it is handy, because it educates users.

The problem is that this check is always done on the last component of
the domain_name even if it is just a sub-domain of the FreeIPA domain,
where e.g. numbers are valid characters.

At the beginning of validate_domain_name() a trailing '.' is stripped
away. iirc the trailing '.' is an indication for a complete, fully
qualified name. Would it work if the presence of the trailing '.' is
saved and the check is only done if there was a '.'?

bye,
Sumit


Sure. Though I am now not 100% sure that some IPA functions do not
use this
validator with a fqdn hostname without trailing dot. If not, I am
for fixing
this function as Sumit and Petr suggested.

Martin

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

After some thought, I decided to change the approach.

As pointed out by Sumit, the problem was that the validate_domain_name()
function did not distinguish between fqdn and non-fqdn domains
(subdomains of the IPA domain). The trailing dot is not a clear
indication either, because some IPA functions use this validator with an
fqdn without the trailing dot.

To fix this, I introduced an additional parameter to this function - a
flag which indicates whether the domain name is an fqdn or not. The is
.isalpha() check is then performed only in the case of an fqdn.

I also improved the error message to mention the relevant RFC, as
suggested by Petr.

Please don't forget to add --force switch. It could be handy.


I added the --force switch to ipa dnsrecord-add and opened a new ticket
to handle the rest of the ipa commands that use domain name validation:
https://fedorahosted.org/freeipa/ticket/3455

Updated patch is attached.


This patch fixed validation only partially. The --force flag you made
available
will not allow admin to for example add a zone "example.zone1" which
technically will be resolvable, it is just not a good practice:

# ipa dnszone-add example.zone1 --name-server `hostname`. --force
ipa: ERROR: invalid 'name': top level domain label must be alphabetic (RFC
1123
section 2.1)

To enable this, I think you would need to not postpone the validation to DNS
zone pre_callback as you could not check --force flag presence right in the
idnsname parameter validator.

We may also want to change --force flag label, it now talks only about NS
record validation, but we now expanded it a bit, so the label would need to be
more general.

Martin


I added the fix for dnszone-add and edited the label of the --force flag
to make it more general.



The zone with this name can be created even without the --force flag.

# ipa dnszone-add example.zone1 --name-server `hostname`.
Administrator e-mail address [hostmaster.example.zone1.]:

Administrator e-mail address: top level domain label must be alphabetic

(RFC 1123 section 2.1)
Administrator e-mail address [hostmaster.example.zone1.]:
hostmaster.example.zone.
   Zone name: example.zone1
   Authoritative nameserver: vm-037.idm.lab.bos.redhat.com.
   Administrator e-mail address: hostmaster.example.zone.
   SOA serial: 1363268183
   SOA refresh: 3600
   SOA retry: 900
   SOA expire: 1209600
   SOA minimum: 3600
   BIND update policy: grant IDM.LAB.BOS.REDHAT.COM krb5-self * A; grant
IDM.LAB.BOS.REDHAT.COM krb5-self
   * ; grant IDM.LAB.BOS.REDHAT.COM krb5-self * SSHFP;
   Active zone: TRUE
   Dynamic update: FALSE
   Allow query: any;
   Allow transfer: none;

I thought that the check would only be surpassed with the --force flag.

All this struggle with this patch makes me thinking if we are not making it too
complicated. I am rather inclined to drop this particular check altogether. I
do not see that much harm if user creates an IPA managed zone "example.zone1".
Petr, what is your opinion on dropping this check in IPA? Is it OK to drop it?


IMHO we can drop the whole check, it should be safe. I did some basic tests
with latest bind-dyndb-ldap and it worked for me.



Ok, I agree. Ana, can you please send a r

Re: [Freeipa-devel] [PATCH] 0006 Remove check for alphabetic only characters from domain name validation

[Petr Spacek]

On 14.3.2013 15:08, Martin Kosek wrote:

On 03/11/2013 01:02 PM, Ana Krivokapic wrote:

On 02/27/2013 10:58 AM, Martin Kosek wrote:

On 02/22/2013 04:02 PM, Ana Krivokapic wrote:

On 02/22/2013 10:19 AM, Petr Spacek wrote:

On 20.2.2013 11:03, Ana Krivokapic wrote:

On 02/18/2013 01:08 PM, Martin Kosek wrote:

On 02/18/2013 12:47 PM, Sumit Bose wrote:

On Mon, Feb 18, 2013 at 12:27:35PM +0100, Petr Spacek wrote:

On 15.2.2013 15:22, Ana Krivokapic wrote:

Hello,

The .isalpha() check in validate_domain_name() was too strict,
causing some commands like ipa dnsrecord-add to fail.

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

I would add --force option rather than removing whole check, if
it's possible.

Would it be possible to mention RFC in the error message? Something
like _('top level domain label must be alphabetic (RFC 1123 section
2.1)')
?

IMHO it is handy, because it educates users.

The problem is that this check is always done on the last component of
the domain_name even if it is just a sub-domain of the FreeIPA domain,
where e.g. numbers are valid characters.

At the beginning of validate_domain_name() a trailing '.' is stripped
away. iirc the trailing '.' is an indication for a complete, fully
qualified name. Would it work if the presence of the trailing '.' is
saved and the check is only done if there was a '.'?

bye,
Sumit


Sure. Though I am now not 100% sure that some IPA functions do not
use this
validator with a fqdn hostname without trailing dot. If not, I am
for fixing
this function as Sumit and Petr suggested.

Martin

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

After some thought, I decided to change the approach.

As pointed out by Sumit, the problem was that the validate_domain_name()
function did not distinguish between fqdn and non-fqdn domains
(subdomains of the IPA domain). The trailing dot is not a clear
indication either, because some IPA functions use this validator with an
fqdn without the trailing dot.

To fix this, I introduced an additional parameter to this function - a
flag which indicates whether the domain name is an fqdn or not. The is
.isalpha() check is then performed only in the case of an fqdn.

I also improved the error message to mention the relevant RFC, as
suggested by Petr.

Please don't forget to add --force switch. It could be handy.


I added the --force switch to ipa dnsrecord-add and opened a new ticket
to handle the rest of the ipa commands that use domain name validation:
https://fedorahosted.org/freeipa/ticket/3455

Updated patch is attached.


This patch fixed validation only partially. The --force flag you made available
will not allow admin to for example add a zone "example.zone1" which
technically will be resolvable, it is just not a good practice:

# ipa dnszone-add example.zone1 --name-server `hostname`. --force
ipa: ERROR: invalid 'name': top level domain label must be alphabetic (RFC 1123
section 2.1)

To enable this, I think you would need to not postpone the validation to DNS
zone pre_callback as you could not check --force flag presence right in the
idnsname parameter validator.

We may also want to change --force flag label, it now talks only about NS
record validation, but we now expanded it a bit, so the label would need to be
more general.

Martin


I added the fix for dnszone-add and edited the label of the --force flag
to make it more general.



The zone with this name can be created even without the --force flag.

# ipa dnszone-add example.zone1 --name-server `hostname`.
Administrator e-mail address [hostmaster.example.zone1.]:

Administrator e-mail address: top level domain label must be alphabetic

(RFC 1123 section 2.1)
Administrator e-mail address [hostmaster.example.zone1.]: 
hostmaster.example.zone.
   Zone name: example.zone1
   Authoritative nameserver: vm-037.idm.lab.bos.redhat.com.
   Administrator e-mail address: hostmaster.example.zone.
   SOA serial: 1363268183
   SOA refresh: 3600
   SOA retry: 900
   SOA expire: 1209600
   SOA minimum: 3600
   BIND update policy: grant IDM.LAB.BOS.REDHAT.COM krb5-self * A; grant
IDM.LAB.BOS.REDHAT.COM krb5-self
   * ; grant IDM.LAB.BOS.REDHAT.COM krb5-self * SSHFP;
   Active zone: TRUE
   Dynamic update: FALSE
   Allow query: any;
   Allow transfer: none;

I thought that the check would only be surpassed with the --force flag.

All this struggle with this patch makes me thinking if we are not making it too
complicated. I am rather inclined to drop this particular check altogether. I
do not see that much harm if user creates an IPA managed zone "example.zone1".
Petr, what is your opinion on dropping this check in IPA? Is it OK to drop it?


IMHO we can drop the whole check, it should be safe. I did some basic tests 
with latest bind-dyndb-ldap and it worked for me.


--
Petr^2 Spacek

___
Freeipa-devel mail

Re: [Freeipa-devel] [PATCH] 1092 Fix LDAP lockout plugin

[Martin Kosek]

On 03/11/2013 10:07 PM, Rob Crittenden wrote:

Fixed a number of issues applying password policy against LDAP binds. See patch
for details.

rob



I see some issues with this fix:

1) Shouldn't group password policy serve only as an override to the main 
policy? I.e. if I have this policy:


# ipa pwpolicy-show test
  Group: test
  Priority: 10
  Max failures: 2

We should still follow settings other than "Max failures" configured in global 
policy, right? At least the Kerberos seem to do it. I think we should be 
consistent in this case. Now, other values just seem to be zero.


I think we will need to fix both the pre-op and the post-op to make this 
working really consistently.


2) The lockout post-op still counts failed logins even though we are in lockout 
time, is this expected? It is another point if inconsistency with Kerberos 
auth. It leaves user's krbloginfailedcount stay on "Max failures".


3) Sometimes, I get into a state when I lockout a new user with Kerberos and 
then wait some time until the lockout time passes (no admin unlock), I am able 
to run as many LDAP binds as I want.


This is all I found so far. Honza is also reviewing it, so I will let him post 
hist findings too.


Martin

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