[Freeipa-devel] [PATCH] 0090, 0092..0094 cert-show: show subject alternative names

[Fraser Tweedale]

On Mon, Aug 15, 2016 at 07:48:22AM +0200, Jan Cholasta wrote:
> On 12.8.2016 18:57, Petr Spacek wrote:
> > 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!"
> > 
> > :-)
> 
> Care to give a concrete example of when this actually happened? Because IIRC
> this happened once or twice, not "usually".
> 
> Anyway, I'm fine with whatever, my point was that additional effort needs to
> be put in to support "everything" and even then, we wouldn't actually
> support everything (two months later: "we need to support extension XYZ!").
> 
Sure, but in this case it was minimal additional effort to support
even the esoteric name types - it is only for display after all; if
an uncommon altname is (somehow) there we can display it.

> (FWIW, I think it would be more useful to add support for Basic constraints
> rather than X.400 SANs.)
> 
I can only agree, and say: another RFE for another day :)

> > 
> > 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).
> > 
> 
> 
> -- 
> Jan Cholasta
> 
> -- 
> 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