review of auth ring spec
Vincent Ladeuil
v.ladeuil+lp at free.fr
Fri Jul 13 10:55:59 BST 2007
>>>>> "mbp" == Martin Pool <mbp at sourcefrog.net> writes:
mbp> I reviewed Vincent's spec
mbp> http://bazaar-vcs.org/DraftSpecs/AuthenticationRing That
mbp> is a very clear and readable spec and a useful feature.
mbp> Preliminaries: for many design documents we seem to have
mbp> shifted from putting them on the wiki to putting them
mbp> into the doc/developer directory. This spec was drafted
mbp> before that change. In general this seems to be working
mbp> pretty well: it means that they're tracked by Bundle
mbp> Buggy so we can make sure they do not go too long
mbp> without review (as this one has); they are not any
mbp> harder to write; and the conversation more naturally
mbp> happens on the list rather than both on the list and in
mbp> the wiki. I think we can continue doing that, and if
mbp> Vincent wants to turn this into a rest file in the
mbp> developer document that'd be good with me.
I'll turn it into a document under doc/developer.
mbp> On the spec itself:
mbp> I can imagine wanting to store other schemes here - for example
mbp> passwords to access svn servers through that plugin, or Launchpad
mbp> xmlrpc calls. So this layer should ideally not make too many
mbp> assumptions that it's being used by the Transport layer.
I'll look into that, but the idea was to create an enhanced
PasswordManager that transports can subscribe to, so that seems
to feet.
mbp> Robert dislikes the name 'auth ring'; I think it's ok.
I'll take any better name proposed but will stick with that one
in the mean time. keychain ? cached-credentials ?
>> The realm` can be ignored as long as it is still presented
>> to the user when prompting for the password (unless
>> someone found a way to declare two different realms for
>> the same path).
mbp> OK. I do wonder if it would be better to use the realm
mbp> as a clue as to what existing password to use, rather
mbp> than the path. But I guess it's probably rare to have
mbp> several independent authentication schemes on the same
mbp> http host, and hard to generalize about how they should
mbp> be treated if they do exist.
Indeed. Realms provide a way to use the same authentication for
disjoint hierarchies, but I'll wait for real use cases before
trying to address that.
>> HTTP proxy can be handled as HTTP (or HTTPS).
mbp> Of course we need to match up the proxy credentials by the proxy's
mbp> host/port, rather than that of the destination server.
Yup.
>> Note that we ignore the port (which can be provided in the
>> URL) to simplify definitions.
mbp> Is there a reason why including the port would be hard?
No. The only point being to handle default port values for protocols.
>> Also note that an optional unsecure field will be allowed
>> to force the connection to HTTPS hosts that provides a
>> self certified certificate (the default should be to
>> refuse the connection and inform the user).
mbp> Please be more explicit and call it something like
mbp> 'accept_untrusted_certificate', 'unsecure' could mean several
mbp> different things.
Ok. I didn't want to introduce fields that were protocol
dependent but I don't have a strong opinion on that point.
>> scheme matches even if decorators are used in the requested URL
mbp> Hm I wonder if we have a factored-out method to match
mbp> schemes disregarding decorators?
Unfortunately no, there is a FIXME in the redirection handling
about that.
mbp> I suppose we would just trim from the front til we get a
mbp> match.
I raised that point in another mail, we don't have a coherent
syntax for the decorators.
>> Possible values are None and base64. When the field is
>> absent, None is assumed. Additional encodings may be added
>> in future versions.
mbp> I guess 'None' means plaintext, so why not just call it
mbp> 'plaintext' and make that the default if it's not given.
Indeed.
mbp> http allows the server to ask for various authentication
mbp> methods. I guess only Basic and Digest can be served by
mbp> passwords. I wonder if they need to be stored in this
mbp> file.
Well, given that the authentication method is an implementation
detail of the authentication itself and that server should accept
whatever method the client is capable of, I can't imagine a
server requesting different passwords for different
authentication methods. Again, until a real use case occurs, I'll
tend to avoid mentioning the authentication method in that file.
>> host matches if included in the request URL. foo.net will
>> match a requested bzr.foo.net.
mbp> I'm not so sure this is a good idea; sometimes
mbp> subdomains have different security properties. Is it
mbp> really needed?
I know of several domains where sftp.domain.net and
http.domain.net use the same credentials, so yes, I think it's
needed.
mbp> Maybe we should leave this out for now, and allow for
mbp> *.foo.net later.
The 'first match wins' rule allows to declare different
credentials for different sub-domains without a need for regexps
or wildcard chars.
>> The section name is an arbitrary string, only the DEFAULT
>> value is reserved and should appear as the *last* section.
mbp> Hm, I would have expected the section name to be the url
mbp> to which this applies. In your example
mbp> [foo.net]
mbp> scheme=ftp
mbp> host=foo.net
mbp> user=joe
mbp> password=secret-pass
mbp> It seems redundant to give it twice, and I can't think of anything
mbp> else that would be a more reasonable title.
To me the title is mostly a comment, I don't think we need to
even make it unique.
It can't be the host only as different paths on the same host may
require different credentials. It can't be scheme+host either for
the same reason.
So, I chose to use it as a comment (and in that example I just
kept the host because it worked), but you can imagine :
[my private projects]
scheme=sftp
host=localhost
path=private
user=joe
password=pass
[my public projects]
scheme=sftp
host=localhost
path=public
user=jim
password=pass2
or even:
[read access]
scheme=http
host=localhost
user=joe
password=pass
[write access]
scheme=sftp
host=locahost
user=jim
password=pass2
mbp> By the way did you look at how subversion does this?
Just did it very quickly at
http://svnbook.red-bean.com/nightly/en/svn.serverconfig.netmodel.html#svn.serverconfig.netmodel.credcache
Summary:
- each credentials set is cached in a file whose name is a hash
of the target. I find it hard to the users (where should I
update ?), while being no problem for crackers. So a single
file seems more user-friendly to me.
- svn has a --no-auth-cache option that some users may want to
activate at a global level ?
- more interestingly svn is able to interface with windows and
OSX keychains to provide encrypted credentials, so a future
version can provide that too.
mbp> Maybe in a per-host section I'd want to specify a per-host proxy
mbp> server, ssh command, etc.
Hmmm, have you ever seen hosts with different proxies for
different targets ? Just curious, it may help people working with
the same laptop in various envs but I doubt it...
By 'ssh command' you mean additional parameters to the shh
command related to the credentials proposed to the server ?
>> The default content of the file will be:
mbp> So will this be created by default by Bazaar?
For compatibility, if such a file is created it should respect
the previous behavior of bzr. But I think the default version
will contains only comments.
mbp> I think I would rather not write out the user's local
mbp> username, but rather use that as a fallback every time
mbp> we are run, in case they run from different machines or
mbp> change their username or something.
Good point, I focused too much on providing a spec without any
constraint on implementations, but that point should be quite easy
to handle in any implementation.
mbp> I can see value in creating a file with comments about
mbp> what it can contain, if it does not exist.
Fully agree.
>> 600 read/write for owner only by default, if another mode
>> (more permissive) is used, no passwords are extracted from
>> it and the user will always be prompted (should be
>> mentioned as a comment in the file itself).
mbp> If the permissions are too weak, we should give a
mbp> warning every time we try to read it. I'm not so sure
mbp> we should *refuse* to read it.
Warning seems enough to me, the idea was to avoid unsuspecting
users to get tricked by a malicious file.
>> Why can't bzr update the authentication file when it
>> queried the user for a password ?
mbp> We should leave that option open for the future.
Ok. Aaron also told me that the file can be rewritten without
losing the comments which was my main fear about updating the
file.
mbp> I don't see anything in the current design that would
mbp> prohibit us asking the user if they want to save the
mbp> password, or giving them an option that updates it. If
mbp> we did that I think we should just default to base-64
mbp> encoding; the user doesn't need to care about the
mbp> details.
Ok.
mbp> So all in all really good.
Thanks.
Sorry for the delay in my answer, you raised very fine points,
I'll try to rewrite my spec asap, but I wanted to keep the ball
rolling in the mean time,
Vincent
More information about the bazaar
mailing list