Skip to content
This repository was archived by the owner on Jul 16, 2020. It is now read-only.

Controller fqdn from cert#1139

Closed
tpepper wants to merge 3 commits into
ciao-project:masterfrom
tpepper:controller_fqdn
Closed

Controller fqdn from cert#1139
tpepper wants to merge 3 commits into
ciao-project:masterfrom
tpepper:controller_fqdn

Conversation

@tpepper

@tpepper tpepper commented Feb 10, 2017

Copy link
Copy Markdown

This PR reverts the two patches which allow the FQDN to come in via a user configuration touch point with fallback to os.Hostname(). Instead it pulls the FQDN from the controller certificate, which ciao-cert populates. If not present, it still falls back to os.Hostname(). Passes BAT and singlevm test automation.

Tim Pepper added 3 commits February 10, 2017 11:52
This reverts commit dc7fe67.  To be
superceded by autoconfiguration method.

Signed-off-by: Tim Pepper <timothy.c.pepper@linux.intel.com>
This reverts commit 37968b5.  To be
superceded by autoconfiguration method.

Signed-off-by: Tim Pepper <timothy.c.pepper@linux.intel.com>
Rather than having the user tell us in the ansible setup or when
creating certificates which host is their controller, when the
controller starts up and uses its certificate, it can find its
configured fully qualified domain name in the certificate.

If for some reason the certificate includes multiple names, the first is
used.  If none is given, an attempt is made to us the non-FQDN available
in os.Hostname().

This should give the intended hostname, without an additional human
touch point on configuration.

Fixes: ciao-project#1138

Signed-off-by: Tim Pepper <timothy.c.pepper@linux.intel.com>
@coveralls

coveralls commented Feb 10, 2017

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.08%) to 66.207% when pulling ae486fb on tpepper:controller_fqdn into 46665f3 on 01org:master.

Comment thread ciao-controller/main.go

role := ssntp.GetRoleFromOIDs(parsedCert.UnknownExtKeyUsage)
if !role.IsController() {
return "", fmt.Errorf("expected certificate role ssntp.Controller, got role %s", role.String())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your code is lgtm.

But i'm surprised you're taking it from the SSNTP cert. I guess that's fine but I would expected it to come from the HTTP cert that the server is actually served with. I think that should be present on clusterConfig.Configure.Controller. The only change should be for the path and to stop looking for the controller OID.

What do you think?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd looked at a sample https cert from the controller web and was seeing slightly different data in it. Initially it was the one I was going to go for. But I liked that the ssntp one enabled the sanity check of the role. Now that I think about it though, the ssntp one is possibly...or even probably most desirably...on a different network and with a different name than the visible end-point name for the user access API's. I think now this MUST use the https api web cert.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or not. For that side of the house we have a CACert and a key.

The CA cert is only optionally created by us and in production I expect it would come from the cluster operator's IT certificate authority, not us.

The key is not a x509 struct. I need to look into whether we could embed metadata in that...in general I suspect it's possible given my gpg key has a hunk of metadata that even includes a jpeg of me!

But even if we did a pivot to embed the desired information in this key, we'd still need to tweek the deploy automation to do this for that key in addition to having ciao-cert do it for the ssntp cert's. AND we'd need to know that the embed works as an after the fact add-on, in case an operator's IT org hands them the key to use which was signed by the IT org's CAcert, not the operator creates the cert and gets it signed by the org's CAcert. I think is something we must expect for a real production deployment.

Soooo...I think the ssntp cert is an easier route to go for now. We're really just looking for a name and there is a name in the ssntp cert. As long as the ssntp cert was created with reference to the external name of the controller, it can be a reliable source of the needed name. Then we don't have to try to interact with the operator to coordinate modifications to a key that is signed by their org.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not super convinced that pulling it from the SSNTP cert is a good idea. The SSNTP communication could be on it's own private network with "internal" dns entries like cn01.ciao.internal, controller.ciao.internal, scheduler.ciao.internal and whereas controller's HTTP service would be on a more exposed network and name.

If we can't extract the hostname from the HTTP cert then i'm leaning towards:

  1. Keeping the configuration entry or..
  2. Removing any api where we need to expose absolute URIs to the client

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the added config option, but I think it's a required piece of data and symmetric to the CA and cert for compute in the config.

@tpepper tpepper closed this Feb 13, 2017
@tpepper

tpepper commented Feb 13, 2017

Copy link
Copy Markdown
Author

Superseded by PR 1142 (a minor documentation update).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants