Re: [Freeipa-devel] [PATCH] 057 Validate MX records
Jakub Hrozek wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 02/16/2011 03:28 PM, Jakub Hrozek wrote: On Tue, Feb 15, 2011 at 03:45:12PM -0500, Rob Crittenden wrote: Jakub Hrozek wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 https://fedorahosted.org/freeipa/ticket/967 I'm wondering whether to extend the patch - if the mail server name does not end with a dot, BIND treats it as relative to the zone. So if you do: ipa dnsrecord-add example.com @ --mx-rec="10 mail.example.com" dig would then return mail.example.com.example.com The correct way of adding it is (note the trailing dot): ipa dnsrecord-add example.com @ --mx-rec="10 mail.example.com." This is in line with how nsupdate works, so should we just document it? A smarter way might be to check if the hostname ends with the zone name and append a dot, but I'm not sure if that perhaps /too/ smart.. While we're at this should we enforce that prio is>= 0 and< MAXINT ? Good suggestion, thanks. As per the MX record documentation I found it should actually be between 0 and 65535, so this is what the patch enforces. Jan's suggestion to rename the parameter is also included. Rob reminded me that the example included was actually wrong. New patch attached. ack, pushed to master ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 057 Validate MX records
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 02/16/2011 03:28 PM, Jakub Hrozek wrote: > On Tue, Feb 15, 2011 at 03:45:12PM -0500, Rob Crittenden wrote: >> Jakub Hrozek wrote: >>> -BEGIN PGP SIGNED MESSAGE- >>> Hash: SHA1 >>> >>> https://fedorahosted.org/freeipa/ticket/967 >>> >>> I'm wondering whether to extend the patch - if the mail server name does >>> not end with a dot, BIND treats it as relative to the zone. >>> >>> So if you do: >>> ipa dnsrecord-add example.com @ --mx-rec="10 mail.example.com" >>> >>> dig would then return mail.example.com.example.com >>> >>> The correct way of adding it is (note the trailing dot): >>> ipa dnsrecord-add example.com @ --mx-rec="10 mail.example.com." >>> >>> This is in line with how nsupdate works, so should we just document it? >>> A smarter way might be to check if the hostname ends with the zone name >>> and append a dot, but I'm not sure if that perhaps /too/ smart.. >> >> While we're at this should we enforce that prio is >= 0 and < MAXINT ? > > Good suggestion, thanks. As per the MX record documentation I found it > should actually be between 0 and 65535, so this is what the patch > enforces. > > Jan's suggestion to rename the parameter is also included. > > Rob reminded me that the example included was actually wrong. New patch attached. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk1b5RQACgkQHsardTLnvCVwngCfRoP9hv7lZQSwkLh5o2yt8etx m4oAoIPs6VnXpVxnmk70Y5wvfbvV9xun =05R/ -END PGP SIGNATURE- From bca6c53ac41081adfe5ec427d5b27499df85852b Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Tue, 15 Feb 2011 10:40:27 +0100 Subject: [PATCH] Validate MX records https://fedorahosted.org/freeipa/ticket/967 --- API.txt |8 ipalib/plugins/dns.py | 20 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/API.txt b/API.txt index 6f0c32f..9e34647 100644 --- a/API.txt +++ b/API.txt @@ -514,7 +514,7 @@ option: List('ipseckeyrecord?', attribute=True, cli_name='ipseckey_rec',ist('ips option: List('keyrecord?', attribute=True, cli_name='key_rec',ist('keyrecord?', attribute=True, cli_name='key_rec', doc='comma-separated list of KEY records', label='KEY record', multivalue=True) option: List('kxrecord?', attribute=True, cli_name='kx_rec',ist('kxrecord?', attribute=True, cli_name='kx_rec', doc='comma-separated list of KX records', label='KX record', multivalue=True) option: List('locrecord?', attribute=True, cli_name='loc_rec',ist('locrecord?', attribute=True, cli_name='loc_rec', doc='comma-separated list of LOC records', label='LOC record', multivalue=True) -option: List('mxrecord?', attribute=True, cli_name='mx_rec',ist('mxrecord?', attribute=True, cli_name='mx_rec', doc='comma-separated list of MX records', label='MX record', multivalue=True) +option: List('mxrecord?', _validate_mx, attribute=True, cli_name='mx_rec',ist('mxrecord?', _validate_mx, attribute=True, cli_name='mx_rec', doc='comma-separated list of MX records', label='MX record', multivalue=True) option: List('naptrrecord?', attribute=True, cli_name='naptr_rec',ist('naptrrecord?', attribute=True, cli_name='naptr_rec', doc='comma-separated list of NAPTR records', label='NAPTR record', multivalue=True) option: List('nsrecord?', attribute=True, cli_name='ns_rec',ist('nsrecord?', attribute=True, cli_name='ns_rec', doc='comma-separated list of NS records', label='NS record', multivalue=True) option: List('nsecrecord?', attribute=True, cli_name='nsec_rec',ist('nsecrecord?', attribute=True, cli_name='nsec_rec', doc='comma-separated list of NSEC records', label='NSEC record', multivalue=True) @@ -558,7 +558,7 @@ option: List('ipseckeyrecord?', attribute=True, cli_name='ipseckey_rec',ist('ips option: List('keyrecord?', attribute=True, cli_name='key_rec',ist('keyrecord?', attribute=True, cli_name='key_rec', doc='comma-separated list of KEY records', label='KEY record', multivalue=True) option: List('kxrecord?', attribute=True, cli_name='kx_rec',ist('kxrecord?', attribute=True, cli_name='kx_rec', doc='comma-separated list of KX records', label='KX record', multivalue=True) option: List('locrecord?', attribute=True, cli_name='loc_rec',ist('locrecord?', attribute=True, cli_name='loc_rec', doc='comma-separated list of LOC records', label='LOC record', multivalue=True) -option: List('mxrecord?', attribute=True, cli_name='mx_rec',ist('mxrecord?', attribute=True, cli_name='mx_rec', doc='comma-separated list of MX records', label='MX record', multivalue=True) +option: List('mxrecord?', _validate_mx, attribute=True, cli_name='mx_rec',ist('mxrecord?', _validate_mx, attribute=True, cli_name='mx_rec', doc='comma-separated list of MX records', label='MX record', multivalue=True) option: List('naptrrecord?', attribute=True, cli_name='naptr_rec',ist('naptrrecord?', attribute=True, cli_name='naptr_rec', doc='comma-separated list of NAPTR records', lab
Re: [Freeipa-devel] [PATCH] 057 Validate MX records
On Tue, Feb 15, 2011 at 03:45:12PM -0500, Rob Crittenden wrote: > Jakub Hrozek wrote: > >-BEGIN PGP SIGNED MESSAGE- > >Hash: SHA1 > > > >https://fedorahosted.org/freeipa/ticket/967 > > > >I'm wondering whether to extend the patch - if the mail server name does > >not end with a dot, BIND treats it as relative to the zone. > > > >So if you do: > >ipa dnsrecord-add example.com @ --mx-rec="10 mail.example.com" > > > >dig would then return mail.example.com.example.com > > > >The correct way of adding it is (note the trailing dot): > >ipa dnsrecord-add example.com @ --mx-rec="10 mail.example.com." > > > >This is in line with how nsupdate works, so should we just document it? > >A smarter way might be to check if the hostname ends with the zone name > >and append a dot, but I'm not sure if that perhaps /too/ smart.. > > While we're at this should we enforce that prio is >= 0 and < MAXINT ? Good suggestion, thanks. As per the MX record documentation I found it should actually be between 0 and 65535, so this is what the patch enforces. Jan's suggestion to rename the parameter is also included. >From 329beb70070748ed108ecd528cd0fee2f9b1ee36 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Tue, 15 Feb 2011 10:40:27 +0100 Subject: [PATCH] Validate MX records https://fedorahosted.org/freeipa/ticket/967 diff --git a/API.txt b/API.txt index 6f0c32f..9e34647 100644 --- a/API.txt +++ b/API.txt @@ -514,7 +514,7 @@ option: List('ipseckeyrecord?', attribute=True, cli_name='ipseckey_rec',ist('ips option: List('keyrecord?', attribute=True, cli_name='key_rec',ist('keyrecord?', attribute=True, cli_name='key_rec', doc='comma-separated list of KEY records', label='KEY record', multivalue=True) option: List('kxrecord?', attribute=True, cli_name='kx_rec',ist('kxrecord?', attribute=True, cli_name='kx_rec', doc='comma-separated list of KX records', label='KX record', multivalue=True) option: List('locrecord?', attribute=True, cli_name='loc_rec',ist('locrecord?', attribute=True, cli_name='loc_rec', doc='comma-separated list of LOC records', label='LOC record', multivalue=True) -option: List('mxrecord?', attribute=True, cli_name='mx_rec',ist('mxrecord?', attribute=True, cli_name='mx_rec', doc='comma-separated list of MX records', label='MX record', multivalue=True) +option: List('mxrecord?', _validate_mx, attribute=True, cli_name='mx_rec',ist('mxrecord?', _validate_mx, attribute=True, cli_name='mx_rec', doc='comma-separated list of MX records', label='MX record', multivalue=True) option: List('naptrrecord?', attribute=True, cli_name='naptr_rec',ist('naptrrecord?', attribute=True, cli_name='naptr_rec', doc='comma-separated list of NAPTR records', label='NAPTR record', multivalue=True) option: List('nsrecord?', attribute=True, cli_name='ns_rec',ist('nsrecord?', attribute=True, cli_name='ns_rec', doc='comma-separated list of NS records', label='NS record', multivalue=True) option: List('nsecrecord?', attribute=True, cli_name='nsec_rec',ist('nsecrecord?', attribute=True, cli_name='nsec_rec', doc='comma-separated list of NSEC records', label='NSEC record', multivalue=True) @@ -558,7 +558,7 @@ option: List('ipseckeyrecord?', attribute=True, cli_name='ipseckey_rec',ist('ips option: List('keyrecord?', attribute=True, cli_name='key_rec',ist('keyrecord?', attribute=True, cli_name='key_rec', doc='comma-separated list of KEY records', label='KEY record', multivalue=True) option: List('kxrecord?', attribute=True, cli_name='kx_rec',ist('kxrecord?', attribute=True, cli_name='kx_rec', doc='comma-separated list of KX records', label='KX record', multivalue=True) option: List('locrecord?', attribute=True, cli_name='loc_rec',ist('locrecord?', attribute=True, cli_name='loc_rec', doc='comma-separated list of LOC records', label='LOC record', multivalue=True) -option: List('mxrecord?', attribute=True, cli_name='mx_rec',ist('mxrecord?', attribute=True, cli_name='mx_rec', doc='comma-separated list of MX records', label='MX record', multivalue=True) +option: List('mxrecord?', _validate_mx, attribute=True, cli_name='mx_rec',ist('mxrecord?', _validate_mx, attribute=True, cli_name='mx_rec', doc='comma-separated list of MX records', label='MX record', multivalue=True) option: List('naptrrecord?', attribute=True, cli_name='naptr_rec',ist('naptrrecord?', attribute=True, cli_name='naptr_rec', doc='comma-separated list of NAPTR records', label='NAPTR record', multivalue=True) option: List('nsrecord?', attribute=True, cli_name='ns_rec',ist('nsrecord?', attribute=True, cli_name='ns_rec', doc='comma-separated list of NS records', label='NS record', multivalue=True) option: List('nsecrecord?', attribute=True, cli_name='nsec_rec',ist('nsecrecord?', attribute=True, cli_name='nsec_rec', doc='comma-separated list of NSEC records', label='NSEC record', multivalue=True) @@ -603,7 +603,7 @@ option: List('ipseckeyrecord?', attribute=True, cli_name='ipseckey_rec',ist('ips option: List('keyrecord?', attribute=True
Re: [Freeipa-devel] [PATCH] 057 Validate MX records
On Tue, Feb 15, 2011 at 12:09:11PM +0100, Jakub Hrozek wrote: > https://fedorahosted.org/freeipa/ticket/967 > > I'm wondering whether to extend the patch - if the mail server name does > not end with a dot, BIND treats it as relative to the zone. > > So if you do: > ipa dnsrecord-add example.com @ --mx-rec="10 mail.example.com" > > dig would then return mail.example.com.example.com > > The correct way of adding it is (note the trailing dot): > ipa dnsrecord-add example.com @ --mx-rec="10 mail.example.com." > > This is in line with how nsupdate works, so should we just document it? > A smarter way might be to check if the hostname ends with the zone name > and append a dot, but I'm not sure if that perhaps /too/ smart.. Hello, I would rather not include this logic. DNS traditionally allows such flexibility; admins must modify zones (in text form or in LDAP) carefully. Regards, Adam -- Adam Tkac, Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 057 Validate MX records
Jakub Hrozek wrote: > https://fedorahosted.org/freeipa/ticket/967 > > I'm wondering whether to extend the patch - if the mail server name does > not end with a dot, BIND treats it as relative to the zone. > > So if you do: > ipa dnsrecord-add example.com @ --mx-rec="10 mail.example.com" > > dig would then return mail.example.com.example.com > > The correct way of adding it is (note the trailing dot): > ipa dnsrecord-add example.com @ --mx-rec="10 mail.example.com." > > This is in line with how nsupdate works, so should we just document it? > A smarter way might be to check if the hostname ends with the zone name > and append a dot, but I'm not sure if that perhaps /too/ smart.. Just a nitpicking here, but shouldn't the second arg of the function be called mx or something like that? Jan ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 057 Validate MX records
Jakub Hrozek wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 https://fedorahosted.org/freeipa/ticket/967 I'm wondering whether to extend the patch - if the mail server name does not end with a dot, BIND treats it as relative to the zone. So if you do: ipa dnsrecord-add example.com @ --mx-rec="10 mail.example.com" dig would then return mail.example.com.example.com The correct way of adding it is (note the trailing dot): ipa dnsrecord-add example.com @ --mx-rec="10 mail.example.com." This is in line with how nsupdate works, so should we just document it? A smarter way might be to check if the hostname ends with the zone name and append a dot, but I'm not sure if that perhaps /too/ smart.. While we're at this should we enforce that prio is >= 0 and < MAXINT ? You can import MAXINT with: from xmlrpclib import MAXINT rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 057 Validate MX records
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 https://fedorahosted.org/freeipa/ticket/967 I'm wondering whether to extend the patch - if the mail server name does not end with a dot, BIND treats it as relative to the zone. So if you do: ipa dnsrecord-add example.com @ --mx-rec="10 mail.example.com" dig would then return mail.example.com.example.com The correct way of adding it is (note the trailing dot): ipa dnsrecord-add example.com @ --mx-rec="10 mail.example.com." This is in line with how nsupdate works, so should we just document it? A smarter way might be to check if the hostname ends with the zone name and append a dot, but I'm not sure if that perhaps /too/ smart.. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk1aXtcACgkQHsardTLnvCXY0wCgtkc0kBdPorCgd9oyh4AazDy0 8hoAn0vgX5xQYJv2D9gjjTgnu0mgUMbp =nzLT -END PGP SIGNATURE- From 9b76991ba0dae19c84a2cad2b60775f8ffa3cc9a Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Tue, 15 Feb 2011 10:40:27 +0100 Subject: [PATCH] Validate MX records https://fedorahosted.org/freeipa/ticket/967 --- API.txt |8 ipalib/plugins/dns.py | 17 + 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/API.txt b/API.txt index fab2241..2ee7fa1 100644 --- a/API.txt +++ b/API.txt @@ -514,7 +514,7 @@ option: List('ipseckeyrecord?', attribute=True, cli_name='ipseckey_rec',ist('ips option: List('keyrecord?', attribute=True, cli_name='key_rec',ist('keyrecord?', attribute=True, cli_name='key_rec', doc='comma-separated list of KEY records', label='KEY record', multivalue=True) option: List('kxrecord?', attribute=True, cli_name='kx_rec',ist('kxrecord?', attribute=True, cli_name='kx_rec', doc='comma-separated list of KX records', label='KX record', multivalue=True) option: List('locrecord?', attribute=True, cli_name='loc_rec',ist('locrecord?', attribute=True, cli_name='loc_rec', doc='comma-separated list of LOC records', label='LOC record', multivalue=True) -option: List('mxrecord?', attribute=True, cli_name='mx_rec',ist('mxrecord?', attribute=True, cli_name='mx_rec', doc='comma-separated list of MX records', label='MX record', multivalue=True) +option: List('mxrecord?', _validate_mx, attribute=True, cli_name='mx_rec',ist('mxrecord?', _validate_mx, attribute=True, cli_name='mx_rec', doc='comma-separated list of MX records', label='MX record', multivalue=True) option: List('naptrrecord?', attribute=True, cli_name='naptr_rec',ist('naptrrecord?', attribute=True, cli_name='naptr_rec', doc='comma-separated list of NAPTR records', label='NAPTR record', multivalue=True) option: List('nsrecord?', attribute=True, cli_name='ns_rec',ist('nsrecord?', attribute=True, cli_name='ns_rec', doc='comma-separated list of NS records', label='NS record', multivalue=True) option: List('nsecrecord?', attribute=True, cli_name='nsec_rec',ist('nsecrecord?', attribute=True, cli_name='nsec_rec', doc='comma-separated list of NSEC records', label='NSEC record', multivalue=True) @@ -558,7 +558,7 @@ option: List('ipseckeyrecord?', attribute=True, cli_name='ipseckey_rec',ist('ips option: List('keyrecord?', attribute=True, cli_name='key_rec',ist('keyrecord?', attribute=True, cli_name='key_rec', doc='comma-separated list of KEY records', label='KEY record', multivalue=True) option: List('kxrecord?', attribute=True, cli_name='kx_rec',ist('kxrecord?', attribute=True, cli_name='kx_rec', doc='comma-separated list of KX records', label='KX record', multivalue=True) option: List('locrecord?', attribute=True, cli_name='loc_rec',ist('locrecord?', attribute=True, cli_name='loc_rec', doc='comma-separated list of LOC records', label='LOC record', multivalue=True) -option: List('mxrecord?', attribute=True, cli_name='mx_rec',ist('mxrecord?', attribute=True, cli_name='mx_rec', doc='comma-separated list of MX records', label='MX record', multivalue=True) +option: List('mxrecord?', _validate_mx, attribute=True, cli_name='mx_rec',ist('mxrecord?', _validate_mx, attribute=True, cli_name='mx_rec', doc='comma-separated list of MX records', label='MX record', multivalue=True) option: List('naptrrecord?', attribute=True, cli_name='naptr_rec',ist('naptrrecord?', attribute=True, cli_name='naptr_rec', doc='comma-separated list of NAPTR records', label='NAPTR record', multivalue=True) option: List('nsrecord?', attribute=True, cli_name='ns_rec',ist('nsrecord?', attribute=True, cli_name='ns_rec', doc='comma-separated list of NS records', label='NS record', multivalue=True) option: List('nsecrecord?', attribute=True, cli_name='nsec_rec',ist('nsecrecord?', attribute=True, cli_name='nsec_rec', doc='comma-separated list of NSEC records', label='NSEC record', multivalue=True) @@ -603,7 +603,7 @@ option: List('ipseckeyrecord?', attribute=True, cli_name='ipseckey_rec',ist('ips option: List('keyrecord?', attribute=True, cli_name='key_rec',ist('keyrecord?', attribute=True, cli_name='ke