[Freeipa-devel] [PATCH] 0210 frontend: fix output validation for multiple type choices
Jan Cholasta
jcholast at redhat.com
Tue Jul 19 10:29:18 UTC 2016
On 19.7.2016 11:36, Alexander Bokovoy wrote:
> On Tue, 19 Jul 2016, Jan Cholasta wrote:
>> On 19.7.2016 10:40, Alexander Bokovoy wrote:
>>> On Tue, 19 Jul 2016, Jan Cholasta wrote:
>>>> On 18.7.2016 10:12, Alexander Bokovoy wrote:
>>>>> On Mon, 18 Jul 2016, Jan Cholasta wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 16.7.2016 12:46, Alexander Bokovoy wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I had some time and was blocked by these bugs to do my tickets so I
>>>>>>> actually fixed these three problems that are assigned to Martin
>>>>>>> Babinsky. Hopefully, Martin wouldn't be offended by that. :)
>>>>>>>
>>>>>>> ------
>>>>>>>
>>>>>>> Output entry elements may have multiple types allowed. We need to
>>>>>>> check
>>>>>>> all of them to properly validate the output. Right now, thin client
>>>>>>> receives type specifications for elements as tuples of types, so
>>>>>>> what is seen as 'None' on the server side becomes (type(None),)
>>>>>>> tuple
>>>>>>> on the thin client side.
>>>>>>>
>>>>>>> Change validation to account this by processing each separate type
>>>>>>> of the element and account for both None and type(None). Raise type
>>>>>>> error only if all of the type checks failed.
>>>>>>>
>>>>>>> https://fedorahosted.org/freeipa/ticket/6061
>>>>>>
>>>>>> NACK, this only hides the real issue, which is that trustconfig-show
>>>>>> (and automember-set-default in #6037) claims to return the primary
>>>>>> key
>>>>>> of the object in the 'value' output field, but the object does not
>>>>>> have a primary key, so the client rightfully expects None.
>>>>> Why did it work before introducing thin client?
>>>>
>>>> Because I took the liberty of not putting any extra effort just to
>>>> keep old hacks working on thin client. In this case, output.PrimaryKey
>>>> was used for the 'value' output field before, which always allowed
>>>> unicode in addition to the primary key type. On thin client,
>>>> output.Output with the primary key type is used instead, which is less
>>>> forgiving and uncovers bogus command definitions. (There aren't any
>>>> besides the ones already mentioned, I remember fixing them but the
>>>> commit got lost somehow. Oh well.)
>>> This means thin client would not work against old server which would
>>> return a non-primary key value. I think it is unacceptable.
>>
>> Not true, as this only applies to servers with API schema (4.4+).
> Only because thin client does not work against servers without API
> schema at all:
>
> [root at f24-master ~]# ipa -vv -e xmlrpc_uri=https://id.vda.li/ipa/xml
> config-show
> ipa: INFO: trying https://id.vda.li/ipa/json
> ipa: INFO: Request: {
> "id": 0, "method": "ping", "params": [
> [], {}
> ]
> }
> ipa: INFO: Response: {
> "error": null, "id": 0, "principal": "admin at VDA.LI",
> "result": {
> "messages": [
> {
> "code": 13001, "message": "API Version
> number was not sent, forward compatibility not guaranteed. Assuming
> server's API version, 2.156", "name": "VersionMissing",
> "type": "warning"
> }
> ], "summary": "IPA server version 4.2.3. API version 2.156"
> }, "version": "4.2.3"
> }
> ipa: INFO: Forwarding 'config_show/1' to json server
> 'https://id.vda.li/ipa/json'
> ipa: INFO: Request: {
> "id": 0, "method": "config_show/1", "params": [
> [], {
> "version": "2.210"
> }
> ]
> }
> ipa: INFO: Response: {
> "error": {
> "code": 905, "message": "unknown command 'config_show/1'",
> "name": "CommandError"
> }, "id": 0, "principal": "admin at VDA.LI", "result": null,
> "version": "4.2.3"
> }
> ipa: ERROR: unknown command 'config_show/1'
Works fine for me:
$ rpm -q freeipa-client
freeipa-client-4.4.0.201607180602GITa2891af3-0.fc24.x86_64
$ ipa ping
-------------------------------------------
IPA server version 4.2.3. API version 2.156
-------------------------------------------
$ ipa config-show
Maximum username length: 32
Home directory base: /home
Default shell: /bin/sh
Default users group: ipausers
Default e-mail domain: abc.idm.lab.eng.brq.redhat.com
Search time limit: -1
Search size limit: -1
User search fields: uid,givenname,sn,telephonenumber,ou,title
Group search fields: cn,description
Enable migration mode: FALSE
Certificate Subject base: O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
Password Expiration Notification (days): 4
Password plugin features: AllowNThash
SELinux user map order:
guest_u:s0$xguest_u:s0$user_u:s0$staff_u:s0-s0:c0.c1023$unconfined_u:s0-s0:c0.c1023
Default SELinux user: unconfined_u:s0-s0:c0.c1023
Default PAC types: nfs:NONE, MS-PAC
Try removing ~/.cache/ipa/servers/<server hostname> - if you reinstalled
the server in the last hour (the default cache TTL), it should help.
>
> Same happens for every other command so I cannot even test the behavior.
> It works against the same server as the thin client is.
>
> [root at f24-master ~]# ipa -vv config-show
> ipa: INFO: trying https://f24-master.ipa.ad.test/ipa/json
> ipa: INFO: Forwarding 'config_show/1' to json server
> 'https://f24-master.ipa.ad.test/ipa/json'
> ipa: INFO: Request: {
> "id": 0, "method": "config_show/1", "params": [
> [], {
> "version": "2.210"
> }
> ]
> }
> ipa: INFO: Response: {
> "error": null, "id": 0, "principal": "admin at IPA.AD.TEST",
> "result": {
> "result": {
> "ca_renewal_master_server": "f24-master.ipa.ad.test",
> "ca_server_server": [
> "f24-master.ipa.ad.test"
> ], "dn":
> "cn=ipaConfig,cn=etc,dc=ipa,dc=ad,dc=test",
> "ipa_master_server": [
> "f24-master.ipa.ad.test"
> ], "ipacertificatesubjectbase": [
> "O=IPA.AD.TEST"
> ], "ipaconfigstring": [
> "AllowNThash"
> ], "ipadefaultemaildomain": [
> "ipa.ad.test"
> ], "ipadefaultloginshell": [
> "/bin/sh"
> ], "ipadefaultprimarygroup": [
> "ipausers"
> ], "ipagroupsearchfields": [
> "cn,description"
> ], "ipahomesrootdir": [
> "/home"
> ], "ipakrbauthzdata": [
> "nfs:NONE", "MS-PAC"
> ], "ipamaxusernamelength": [
> "32"
> ], "ipamigrationenabled": [
> "FALSE"
> ], "ipapwdexpadvnotify": [
> "4"
> ], "ipasearchrecordslimit": [
> "100"
> ], "ipasearchtimelimit": [
> "2"
> ], "ipaselinuxusermapdefault": [
> "unconfined_u:s0-s0:c0.c1023"
> ], "ipaselinuxusermaporder": [
>
> "guest_u:s0$xguest_u:s0$user_u:s0$staff_u:s0-s0:c0.c1023$unconfined_u:s0-s0:c0.c1023"
>
> ], "ipausersearchfields": [
> "uid,givenname,sn,telephonenumber,ou,title"
> ], "ntp_server_server": [
> "f24-master.ipa.ad.test"
> ]
> }, "summary": null, "value": null
> }, "version": "4.4.0.201607151226GIT37bfd1f"
> }
> Maximum username length: 32
> Home directory base: /home
> Default shell: /bin/sh
> Default users group: ipausers
> Default e-mail domain: ipa.ad.test
> Search time limit: 2
> Search size limit: 100
> User search fields: uid,givenname,sn,telephonenumber,ou,title
> Group search fields: cn,description
> Enable migration mode: FALSE
> Certificate Subject base: O=IPA.AD.TEST
> Password Expiration Notification (days): 4
> Password plugin features: AllowNThash
> SELinux user map order:
> guest_u:s0$xguest_u:s0$user_u:s0$staff_u:s0-s0:c0.c1023$unconfined_u:s0-s0:c0.c1023
>
> Default SELinux user: unconfined_u:s0-s0:c0.c1023
> Default PAC types: nfs:NONE, MS-PAC
> IPA masters: f24-master.ipa.ad.test
> IPA CA servers: f24-master.ipa.ad.test
> IPA NTP servers: f24-master.ipa.ad.test
> IPA CA renewal master: f24-master.ipa.ad.test
>
>
>
>>
>>>
>>>>> Aside from that, comparing "(type(None),) is None" will never give you
>>>>> True on the thin client side. At the server side we have "None is
>>>>> None"
>>>>> and that works. So the question is also why there is a change like
>>>>> that
>>>>> between client and server sides?
>>>>
>>>> I don't see how this has anything to do with anything. In output
>>>> validation, "type = None" means that value of any type is allowed,
>>>> "type = (type(None),)" means that only None is allowed. Nothing
>>>> changed with regard to that in thin client.
>>> Really? Previous code worked against corresponding server, new code
>>> doesn't work against the very same server. I consider it a breakage.
>>
>> Really, since commit b6e4972e. If something is broken, please file a
>> ticket.
> See above. How should I test?
--
Jan Cholasta
More information about the Freeipa-devel
mailing list