Client side SSL related variables

Bug #920386 reported by Stuart McLaren on 2012-01-23
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Glance
Medium
Stuart McLaren

Bug Description

A couple of queries on the use of SSL related variables (I'm not an expert on this
stuff).

1) Currently the glance client must supply both a key and a cert when
using https (via GLANCE_CLIENT_CERT_FILE/GLANCE_CLIENT_CERT_FILE).

2) In contrast the GLANCE_CLIENT_CA_FILE is optional.

In relation to (1): Is there a case for making these optional? Ie use them if they are
supplied, otherwise proceed without them. If glance is on a public interface I'm not sure
that we will require users to supply values for these -- eg this seems to be the case for Swift.

In relation to (2), both curl (curl -k/--insecure) and wget (wget --no-check-certificate)
require you to explicity state that you don't want to check the server's cert if you don't
supply a ca_file. Would it be worth changing to match that behaviour? ie When glance is
using https you must either supply a GLANCE_CLIENT_CA_FILE or specify an option such
as --no-check-certificate?

Dan Prince (dan-prince) wrote :

Hi Stewart,

Regarding #1:

This possibly sounds like a bug. If the glance server is using SSL client authentication should be optional. If client auth is off on the server side then glance clients would not need to have keys and certs when trying to connect.

Regarding #2:

I'd buy adding an option like this 'glance --no-cert-check'. This would allow you to use glance client for encryption but ignore/skip verification of the glance servers certificate. This flag would certainly be handy for development but you probably wouldn't want it in production as you'd be at risk for man-in-the-middle.

While these issues are related it may be best to file two separate tickets on these?

Dan Prince (dan-prince) wrote :

s/Stewart/Stuart/g

Stuart McLaren (stuart-mclaren) wrote :
Download full text (3.2 KiB)

Hi Dan,

Thanks for your feedback. Yes, I think its a good idea to have a bug per issue. (See end of this note).

For my own sanity I've attempted to note the various legal combinations of parameters for
both the server and client. (Hopefully the formatting will get preserved).

I've also noted a third potential change:

Server side parameters to ssl.wrap_socket:

------------------------------------------------------------------
| key_file | cert_file | ca_certs | cert_reqs | valid | note |
------------------------------------------------------------------
| 0 | 0 | 0 | NONE | 1 | (1) |
| 1 | 0 | 0 | NONE | 0 | (2) |
| 0 | 1 | 0 | NONE | 0 | (2) |
| 1 | 1 | 0 | NONE | 1 | (3) |
| 1 | 1 | 1 | NONE | 0 | (4) |
| 0 | 0 | 1 | NONE | 0 | (5) |
| 1 | 1 | 0 | REQUIRED | 0 | (6) |
| 1 | 1 | 1 | REQUIRED | 1 | (7) |
------------------------------------------------------------------

(1) Plain http connection
(2) Both key and cert needed
(3) https connection, no checking of client cert
(4) If ca_cert is present cert_req must not be NONE
(5) If not using ssl a ca_certs parameter is meaningless.
(6) If ca_certs is not present cert_req must be NONE
(7) client cert will be verified using specified ca_certs file. The
    current server code doesn't seem to do this (only cert_file and key_file
    arguments are passed)
    *Change #3*: This functionality should be added, ie a new 'server_ca_cert'
                option.

Note: may want registry to have cert_reqs REQUIRED, but API have
cert_reqs NONE?

client side parameters to ssl.wrap_socket:

------------------------------------------------------------------
| key_file | cert_file | ca_certs | cert_reqs | valid | note |
------------------------------------------------------------------
| 0 | 0 | 0 | NONE | 1 | (8) |
| 1 | 0 | 0 | NONE | 0 | (9) |
| 0 | 1 | 0 | NONE | 0 | (9) |
| 1 | 1 | 0 | NONE | 1 | (10) |
| 1 | 1 | 1 | NONE | 0 | (11) |
| 0 | 0 | 1 | NONE | 1 | (12) |
| 1 | 1 | 0 | REQUIRED | 0 | (13) |
| 1 | 1 | 1 | REQUIRED | 1 | (14) |
------------------------------------------------------------------

(8) plain http connection.
(9) Both key and cert needed
(10) https connection, no checking of server cert
    *Change #2*: allow this, but only if '--no-cert-check' option is provided.
(11) If ca_cert is present cert_req must not be NONE
(12) If using ssl server client does not need to supply key_file/cert_file.
    *Change #1: Currently this is not supported, add support.
(13) If ca_certs is not present cert_req must be NONE
(14) server cert will be verified using specified ca_certs file. The
     current code allows this.

If the above is sensible...

Read more...

Stuart McLaren (stuart-mclaren) wrote :

Lost some formatting -- attaching original table as text (easier to read).

Brian Waldon (bcwaldon) on 2012-01-25
Changed in glance:
status: New → Triaged
Jay Pipes (jaypipes) on 2012-01-25
Changed in glance:
importance: Undecided → Medium
milestone: none → essex-4
assignee: nobody → Stuart McLaren (stuart-mclaren)
Stuart McLaren (stuart-mclaren) wrote :

Some keystone cert related comments.

A glance client who accesses a https keystone server to get a token
should be able to specify a "CA" file argument to confirm the CS/keystone
server's id. This would be change #4.
(Is this also the case for swift/nova?)

A glance server (when acting as a keystone client) should similarly be able
to specify a keystone "CA" file config option to verify the keystone server's
id. change #5.

In addition a glance server should be able to specify a keystone
client-side cert and key, these would be used when generating the SSL
connection to verify an existing token, and allow the keystone server
to validate the glance server/keystone client. change #6.

Jay Pipes (jaypipes) wrote :

Stu, what's the deal with this one? I could have sworn we got a patch that addressed this into trunk already...?

Stuart McLaren (stuart-mclaren) wrote :

Jay,

Yes, there were two changes relating to this went in. I have other stuff listed here, but they may
be better suited to a blueprint rather than a bug. I'd be inclined to close this.

Thierry Carrez (ttx) wrote :

Marking FixReleased/E4 based on last comment -- please reopen if you disagree.

Changed in glance:
status: Triaged → Fix Released
Jay Pipes (jaypipes) on 2012-03-01
Changed in glance:
milestone: essex-4 → essex-rc1
Thierry Carrez (ttx) wrote :

Was actually fixed in E4 iiuc

Changed in glance:
milestone: essex-rc1 → essex-4
Thierry Carrez (ttx) on 2012-04-05
Changed in glance:
milestone: essex-4 → 2012.1
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Bug attachments