[Freeipa-devel] [PATCH] 924 display both hex and decimal serial numbers

[Jan Cholasta]

On 13.3.2012 22:57, Rob Crittenden wrote:
> Jan Cholasta wrote:
>> On 7.3.2012 17:12, Rob Crittenden wrote:
>>> Petr Vobornik wrote:
>>>> On 03/06/2012 09:56 PM, Rob Crittenden wrote:
>>>>> Rob Crittenden wrote:
>>>>>> Jan Cholasta wrote:
>>>>>>> Dne 18.1.2012 00:04, Rob Crittenden napsal(a):
>>>>>>>> Jan Cholasta wrote:
>>>>>>>>> Dne 16.1.2012 22:02, Rob Crittenden napsal(a):
>>>>>>>>>> Rob Crittenden wrote:
>>>>>>>>>>> Jan Cholasta wrote:
>>>>>>>>>>>> Dne 13.1.2012 20:53, Rob Crittenden napsal(a):
>>>>>>>>>>>>> When viewing a certificate it will show the serial number as
>>>>>>>>>>>>> hex
>>>>>>>>>>>>> (dec).
>>>>>>>>>>>>>
>>>>>>>>>>>>> # ipa service-show HTTP/rawhide.example.com
>>>>>>>>>>>>> Principal: HTTP/rawhide.example.com at EXAMPLE.COM
>>>>>>>>>>>>> Certificate: [snip]
>>>>>>>>>>>>> Keytab: True
>>>>>>>>>>>>> Managed by: rawhide.example.com
>>>>>>>>>>>>> Subject: CN=rawhide.example.com,O=EXAMPLE.COM
>>>>>>>>>>>>> Serial Number: 0x403 (1027)
>>>>>>>>>>>>> Issuer: CN=EXAMPLE.COM Certificate Authority
>>>>>>>>>>>>> Not Before: Fri Jan 13 15:00:44 2012 UTC
>>>>>>>>>>>>> Not After: Thu Jan 13 15:00:44 2022 UTC
>>>>>>>>>>>>> Fingerprint (MD5):
>>>>>>>>>>>>> e5:43:17:0d:8d:af:d6:69:d8:fb:eb:ca:79:fb:47:69
>>>>>>>>>>>>> Fingerprint (SHA1):
>>>>>>>>>>>>> c2:9e:8e:de:42:c9:4a:29:cc:b0:a0:de:57:c7:b7:d8:f9:b5:fe:e6
>>>>>>>>>>>>>
>>>>>>>>>>>>> rob
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> NACK
>>>>>>>>>>>>
>>>>>>>>>>>> Displaying a host or a service in the webUI fails with "IPA
>>>>>>>>>>>> error
>>>>>>>>>>>> 3009:
>>>>>>>>>>>> invalid 'serial_number': Decimal or hexadecimal number is
>>>>>>>>>>>> required
>>>>>>>>>>>> for
>>>>>>>>>>>> serial number".
>>>>>>>>>>>>
>>>>>>>>>>>> I would suggest to do the nifty formatting of serial numbers on
>>>>>>>>>>>> the
>>>>>>>>>>>> client side, that would fix the webUI issue, allow non-IPA
>>>>>>>>>>>> clients to
>>>>>>>>>>>> parse the number without dissecting the string representation
>>>>>>>>>>>> of it
>>>>>>>>>>>> and
>>>>>>>>>>>> probably also save me a hack in the type conversion overhaul.
>>>>>>>>>>>> You
>>>>>>>>>>>> could
>>>>>>>>>>>> for example add a parameter flag like "format_serial_number" to
>>>>>>>>>>>> indicate
>>>>>>>>>>>> to the client that it should format the value as a serial
>>>>>>>>>>>> number.
>>>>>>>>>>>>
>>>>>>>>>>>> Honza
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Well, we want to do as little client formatting as possible. The
>>>>>>>>>>> idea is
>>>>>>>>>>> to have a very thin client.
>>>>>>>>>
>>>>>>>>> It doesn't seem right to me to enforce this specific
>>>>>>>>> representation of
>>>>>>>>> what is really just an integer at the API level. Doing a little
>>>>>>>>> formatting on the client side won't make the client(s)
>>>>>>>>> particularly
>>>>>>>>> fat,
>>>>>>>>> will it?
>>>>>>>>
>>>>>>>> Yes. The current code just outputs labels and data. There is no
>>>>>>>> "if it
>>>>>>>> is this attribute then do that" logic.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> IMHO there is too much stuff done on server that would make more
>>>>>>>>> sense
>>>>>>>>> to do on client anyway (especially CLI-specific stuff such as CSV
>>>>>>>>> parsing). What is the reason we want such a thin client?
>>>>>>>>
>>>>>>>> To avoid double work such that every time we want a formatting
>>>>>>>> change we
>>>>>>>> have to change it in multiple places. This lesson was learned in
>>>>>>>> v1.
>>>>>>>>
>>>>>>>>> I believe there should be clear separation of presentation and
>>>>>>>>> content,
>>>>>>>>> but perhaps I'm a little bit too idealistic :-).
>>>>>>>>
>>>>>>>> You have a point, serial number is defined as an integer.
>>>>>>>> Perhaps we
>>>>>>>> should revisit this decision to display hex at all.
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I'll look into fixing the UI side.
>>>>>>>>>>
>>>>>>>>>> I don't see this error in services, it displays correctly. I'm
>>>>>>>>>> not
>>>>>>>>>> sure
>>>>>>>>>> if it is my browser or what but hosts don't display much of
>>>>>>>>>> anything
>>>>>>>>>> for
>>>>>>>>>> me.
>>>>>>>>>>
>>>>>>>>>> rob
>>>>>>>>>
>>>>>>>>> I have just checked both master and ipa-2-2 and I'm getting the
>>>>>>>>> same
>>>>>>>>> error message (tested in Firefox 9.0.1) when viewing details of a
>>>>>>>>> host
>>>>>>>>> or a service with the usercertificate attribute set.
>>>>>>>>>
>>>>>>>>> BTW, wouldn't it make sense to format serial numbers in the cert
>>>>>>>>> plugin
>>>>>>>>> in the same way?
>>>>>>>>
>>>>>>>> Perhaps. Like I said, I'm not really in favor of this change.
>>>>>>>>
>>>>>>>> rob
>>>>>>>
>>>>>>> Maybe we can do a compromise of some sort. What about allowing the
>>>>>>> client to specify with each request what representation/formatting
>>>>>>> the
>>>>>>> server should use for the resulting entries and attributes?
>>>>>>
>>>>>> That would be mighty flexible but would open a new can of worms. I
>>>>>> think
>>>>>> long term I'd like to be able to request what attributes to see (ala
>>>>>> ldapsearch) but that too is a bit out of scope.
>>>>>>
>>>>>> This comes down to Output being rather loosely defined and we already
>>>>>> have a ticket open on that. It basically just defines the broad
>>>>>> types of
>>>>>> data to be returned (string, list, dict, etc) but not the internal
>>>>>> components of complex types.
>>>>>
>>>>> Took a new approach and created a new output attribute,
>>>>> serial_number_hex, that is displayed separately.
>>>>>
>>>>> UI portion added as well.
>>>>>
>>>>
>>>> ACK for the UI part. I attached a patch which extends UI static testing
>>>> data - to keep things in solid state.
>>>>
>>>> I think this approach is still evil (as the whole ticket) but I don't
>>>> have a better solution (in CLI).
>>>
>>> We are in agreement.
>>>
>>>> Question:
>>>> Isn't the '0x' part a bit redundant? The label already says '(hex)'.
>>>> However I can buy a 'It is a convention.' argument.
>>>
>>> Yes, I did it for convention, plus to avoid confusion for the case where
>>> it looks like a decimal number but isn't, e.g. 10. If you saw:
>>>
>>> Serial number: 16
>>> Serial number (hex): 10
>>>
>>> It might be confusing. 0x10 would be clearer.
>>>
>>> rob
>>
>> This patch works for me, but let me repeat myself:
>>
>> > BTW, wouldn't it make sense to format serial numbers in the cert
>> > plugin in the same way?
>>
>> Honza
>>
>
> Updated.
>
> rob
>

Thanks, ACK.

Honza

-- 
Jan Cholasta