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

[Rob Crittenden]

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