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

[Jan Cholasta]

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

-- 
Jan Cholasta