On 12.8.2016 11:33, Jan Cholasta wrote: > On 4.8.2016 18:18, Petr Vobornik wrote: >> On 07/22/2016 07:13 AM, Fraser Tweedale wrote: >>> On Tue, Jul 19, 2016 at 08:50:34AM +0200, Jan Cholasta wrote: >>>> Hi, >>>> >>>> On 14.7.2016 13:44, Fraser Tweedale wrote: >>>>> Hi all, >>>>> >>>>> The attached patch includes SANs in cert-show output. If you have >>>>> certs with esoteric altnames (especially any that are more than just >>>>> ASN.1 string types), please test with those certs. >>>>> >>>>> https://fedorahosted.org/freeipa/ticket/6022 >>>> >>>> I think it would be better to have a separate attribute for each supported >>>> SAN type rather than cramming everything into subject_alt_name. That way if >>>> you care only about a single specific type you won't have to go through all >>>> the values and parse them. Also it would allow you to use param types >>>> appropriate to the SAN types (DNSNameParam for DNS names, Principal for >>>> principal names, etc.) >>>> >>>> Nitpick: please don't mix moving existing stuff and adding new stuff in a >>>> single patch. >>>> >>> Updated patches attached. >>> >>> Patches 0092..0094 are refactors and bugfixes. >>> Patch 0090-2 is the main feature (depends on 0092..0094). >>> >>> Thanks, >>> Fraser >>> >> >> bump for review > > Patch 0092: ACK > > Patch 0093: ACK > > Patch 0094: ACK > > Patch 0090: > > 1) Generic otherNames (san_other) do not work correctly. The OID is not > included in the value and names with complex type other than KerberosPrincipal > are not parsed correctly. The value should include the OID and DER blob of the > name. > > 2) With --all, san_other should be included in the result for all otherNames, > even the known ones, to provide (limited) forward compatibility. > > 3) Do we have to support *all* the name types? I mean we could, for the sake > of completeness, but it might be easier to just keep the few ones we actually > care about (email, DNS name, principal name, UPN and directory name in your > patch 0095).
As far as I remember this reasoning usually comes back to bite us into butt. - "Implement only this subset, nobody else needs the other the of ... (whatever, e.g. DNS record type)." - "Yes my lord." Two months later: - "We need to support for XYZ. It was (already) late yesterday!" :-) Petr^2 Spacek > > 4) > > + obj.setdefault(attr_name, ).append(unicode(name)) > > The value should not (always) be unicode, but of the type declared by the > param (unicode or ipapython.kerberos.Principal or ipapython.dnsutil.DNSName). -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code