[Freeipa-devel] [PATCH] 383 Check subject name encoding in ipa-cacert-manage renew

[Jan Cholasta]

Dne 10.12.2014 v 17:53 Martin Basti napsal(a):
> On 10/12/14 16:02, Martin Kosek wrote:
>> On 12/10/2014 02:35 PM, Jan Cholasta wrote:
>>> Dne 10.12.2014 v 11:53 Martin Kosek napsal(a):
>>>> On 12/09/2014 01:56 PM, Jan Cholasta wrote:
>>>>> Dne 5.12.2014 v 12:01 Jan Cholasta napsal(a):
>>>>>> Dne 5.12.2014 v 11:43 Martin Kosek napsal(a):
>>>>>>> On 12/05/2014 11:34 AM, Jan Cholasta wrote:
>>>>>>>> Dne 5.12.2014 v 09:03 Martin Kosek napsal(a):
>>>>>>>>> On 12/04/2014 09:36 AM, Jan Cholasta wrote:
>>>>>>>>>> +            if x509.get_der_subject(cert, x509.DER) !=
>>>>>>>>>> der_subject:
>>>>>>>>>> +                raise admintool.ScriptError("Subject name
>>>>>>>>>> encoding
>>>>>>>>>> mismatch")
>>>>>>>>> I think we can expect this to be a pretty common error, given
>>>>>>>>> this is
>>>>>>>>> the default behavior of Microsoft Certificate Services. I would
>>>>>>>>> thus
>>>>>>>>> like to make the error message more juicy.
>>>>>>>>>
>>>>>>>>> We need to make sure we offer some pointers for these users or
>>>>>>>>> they
>>>>>>>>> will
>>>>>>>>> just blame IPA for screwing up. So, the information I wrote
>>>>>>>>>
>>>>>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1129558#c11
>>>>>>>>>
>>>>>>>>> need to somehow get to the error message as a potential/likely
>>>>>>>>> root
>>>>>>>>> cause of the problem. Whether you write it in the error message
>>>>>>>>> itself
>>>>>>>>> or update the design page and just insert a link is up to you.
>>>>>>>>>
>>>>>>>>> Martin
>>>>>>>> I would rather document this and have users read the documentation,
>>>>>>>> which they
>>>>>>>> should do anyway when something goes wrong. There are many
>>>>>>>> errors in
>>>>>>>> IPA which
>>>>>>>> are common and users may blame IPA for them and I don't see what
>>>>>>>> makes
>>>>>>>> this one
>>>>>>>> so special that it should require a special treatment.
>>>>>>> I saw several reasons:
>>>>>>> - Certificate&installation error are more common than the others and
>>>>>>> users are usually quite lost in what to do with them.
>>>>>>> - In this case, we know by 90% probability what is the root cause
>>>>>>> - It will block one of the main use cases for the new CA renewal
>>>>>>> tool
>>>>>>> and people will likely hit it as MS CAs is one of the most common
>>>>>>> CAs
>>>>>>> and this is it's default behavior.
>>>>>>>
>>>>>>> Giving more details in this case will not hurt us, but benefit
>>>>>>> users. So
>>>>>>> I still do not see the harm.
>>>>>> I do not see a harm either, my point is that we should probably point
>>>>>> the user to documentation when *anything* in *any* script goes wrong,
>>>>>> not just when some arbitrarily cherry-picked error occurs.
>>>>>>
>>>>>>>> Anyway, I have created
>>>>>>>> <http://www.freeipa.org/page/Troubleshooting#External_CA_renewal_with_ipa-cacert-manage_fails>.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>> Good. Do you plan to reference the section or enhance the error
>>>>>>> message?
>>>>>> I plan to reference <http://www.freeipa.org/page/Troubleshooting>.
>>>>> See the attached patch (385).
>>>> I think the reference for the Troubleshooting page should be more
>>>> narrow so
>>>> that people only see the URL only for the cases we give specific
>>>> advise for.
>>>> Otherwise I assume they will just ignore the page if they do not
>>>> find the
>>>> advise for other errors.
>>> Right, makes sense.
>>>
>>>> Other idea would be to give reference to the article when the actual
>>>> CSR is
>>>> generated - as a heads up.
>>> I think referring to troubleshooting before there actually is some
>>> trouble is
>>> not very good for publicity.
>> Ah, that's a good point - in this purpose it would be better to have it
>> somewhere else or only refer to the MS article.
>>
>>> Anyway, updated patch attached, it implements what you suggested
>>> originally -
>>> link to the troubleshooting guide is added to relevant error
>>> messages. Let's
>>> think about this in more broad terms when the time comes for the
>>> installer
>>> refactoring.
>> Ok. I am fine with the patch conceptually. So now just someone
>> (David?) needs
>> to make sure it did not break anything :-)
>>
>> Martin
>>
> ACK, seems it doesnt break anything.
>

Thanks for the review.

Pushed to:
master: 8f9c5988e2f370cef66a4cd7cf3d363f061a439c
ipa-4-1: 3cb2f5e841f5bac6a8cc02bc9467846b35f7aab8

-- 
Jan Cholasta