HTTP && HTTPS Transport Review

Greg Hudson ghudson at MIT.EDU
Mon Aug 12 12:36:47 EDT 2013


On 08/08/2013 05:41 PM, Robbie Harwood wrote:
> I have written code to add support for HTTP && HTTPS transport of
> Kerberos traffic to the KDC which I would like to submit for review.  It
> can be found on my github[0].

Thanks for this.  Please take a look at:

    http://k5wiki.kerberos.org/wiki/Contributing_code

We will need a few things in particular:

* Please create a project page with a design writeup.  It is difficult
for us to review code without an accompanying design document, as we
have to intuit the design from the code changes.  If you don't have a
wiki account, you'll need to talk to Tom Yu (tlyu on irc) about
registering one; we unfortunately had to disable open registration
because of spammers.

* Please make sure to review the C style checker output before asking
for a round of review.  Sometimes it is appropriate to ignore output,
but currently the checker finds a few no-brainer things like overly long
lines in the first commit.  Skimming the code, it looks like you did a
reasonably good job of matching our style, but see:
http://k5wiki.kerberos.org/wiki/Coding_style/Practices#Local_variables

Without looking at the code in too much detail, I have some design comments:

* What Kerberos-over-HTTP protocol does this implement?  We've discussed
a couple of different pieces of prior art here, and it isn't obvious
whether you've tried to match either of them.  This is one of the things
I'd expect to find in a design writeup.

* The first commit (~6) adds support for "tcp://hostname:port" and
"udp://hostname:port".  I don't believe we had discussed this, and I'm
not fond of the idea.  "tcp:" and "udp:" are not real URI schemes, and
our parsing rules for hostname:port combinations do not follow the
common URI syntax.  I am still fine with having http: and https: URLs in
that field, overloaded with the hostname:port syntax; it's easy to tell
the difference by recognizing the "http://" and "https://" prefixes.

* The next commit (~5) adds an enumeration type which contains platform
socket type values as well as protocol designators we define ourselves.
 This idea is inherently non-portable, and we'll need to find an
alternative design.  Most likely, we'll want an enumeration containing
our own protocol designators (UDP/TCP, UDP, TCP, HTTP, and HTTPS) and we
can map that onto platform socket types when required.

* The third commit (~4) appears to contain a lot of cut-and-paste from
service_tcp_fd to service_http_fd.  We'll need more careful design.



More information about the krbdev mailing list