Wip rgw openssl 7 #11776
Wip rgw openssl 7 #11776
Conversation
| Civetweb requires that the server key, server certificate, and any other | ||
| CA or intermediate certificates be supplied in one file. Each of these | ||
| items must be in `pem` form. Because the combined file contains the | ||
| secret key, it should be protected from unauthorized access. |
There was a problem hiding this comment.
Should we clarify "unauthorized access" here? Explicitly "Only the UID that runs radosgw (the "ceph" UID) should have read access to this key file on the RGW server" ?
There was a problem hiding this comment.
I'm more worried about people publishing them via the web, making them accessible company wide via puppet, or throwing printouts in the waste basket. All those "careless" things people do. So I'd rather not be too specific.
mdw-at-linuxbox
force-pushed
the
wip-rgw-openssl-7
branch
from
16db4e4
to
90a356c
Compare
|
I've pushed an updated copy of this - based on the civetweb1.8 that's in master. I've also updated git@github.com:mdw-at-linuxbox/civetweb-1 wip-listen2 aka ceph/civetweb#15 - that change is required for this. |
|
This branch appears to be failing due to some unrelated problem with the version of master I based on - causes TEST_rados_get_bad_size_shard_1 to fail in test-erasure-eio.sh, as best I can tell. (The logic jenkins uses to describe the problem is tagging unrelated "normal" output instead of the problem.) |
|
jenkins test this please (no log) |
| @@ -870,7 +870,7 @@ endif(WITH_KVS) | |||
| if(WITH_RADOSGW) | |||
| set(civetweb_common_files civetweb/src/civetweb.c) | |||
| add_library(civetweb_common_objs OBJECT ${civetweb_common_files}) | |||
| target_include_directories(civetweb_common_objs PUBLIC | |||
| target_include_directories(civetweb_common_objs PRIVATE | |||
There was a problem hiding this comment.
@mdw-at-linuxbox why is this needed? Also, please change the title and description of this commit
| @@ -3,7 +3,8 @@ | |||
| url = https://github.com/ceph/ceph-object-corpus.git | |||
| [submodule "src/civetweb"] | |||
| path = src/civetweb | |||
| url = https://github.com/ceph/civetweb | |||
| url = https://github.com/mdw-at-linuxbox/civetweb | |||
| branch = wip-listen2 | |||
There was a problem hiding this comment.
@mdw-at-linuxbox please drop this change. Messing with submodule urls is one of the better ways to annoy everyone. We can create a branch on ceph/civetweb instead
| if (secure_port != "443") | ||
| token_value = token_value + ":" + secure_port; | ||
| } else if (!port.empty()) { | ||
| if (secure_port != "80") |
There was a problem hiding this comment.
@mdw-at-linuxbox should it be port?
mdw-at-linuxbox
force-pushed
the
wip-rgw-openssl-7
branch
from
90a356c
to
cbf0533
Compare
|
I've pushed a new version. This is based on wip-rgw-civetweb-1.8-2. I may have overwritten somebody else's stuff too - hadn't realized other people were pushing stuff to this branch. Arg. So I pushed: Probably I should have corrected the commit message for 6e6d7c2; it's now pointing to |
We previously rebased to a pre-1.8 version (accidentally) Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
mdw-at-linuxbox
force-pushed
the
wip-rgw-openssl-7
branch
from
cbf0533
to
91c41ae
Compare
For 'target_include_directories" for the cmake object library 'civetweb_common_objs', change from PUBLIC to PRIVATE. This doesn't break anything, so it wasn't doing anything useful. If it has it any effect, it would be to cause everything that linked against this "library" to also use the indictated include path. Which would be great except everything in ceph wants to include "civetweb/civetweb.h" and not "civetweb.h". We already make separate arrangements elsewhere for that to work. Additionally, static object libraries in cmake aren't really libraries, so I'm not entirely sure this even does anything. So: making this public is not useful, and could be harmful. Making it private makes this only take effect for building civetweb.c itself, exactly the effect we we require, and no more. Signed-off-by: Marcus Watts <mwatts@redhat.com>
If building with radosgw, always look for openssl library (even when building with nss). Then, use objdump to fetch SONAME from the copies of libssl and libcrypto that were found. When building civetweb; pass the library soname values in as the libraries to load with "dlopen". This is a problem that went away for a bit, but came back with some changes for http://tracker.ceph.com/issues/16535 Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1341775 Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1258961 Fixes: http://tracker.ceph.com/issues/11239 Signed-off-by: Marcus Watts <mwatts@redhat.com>
This includes information on file format and configuration file syntax. Signed-off-by: Marcus Watts <mwatts@redhat.com>
… mg_get_local_addr The logic inside of radosgw that computes aws v4 signatures wants to know what server port the client connected. The patch to civetweb patch adds a call mg_get_local_addr() which will permit that code to actually find out on what address a connection was received, rather than merely guessing based on configuration as it previously did. Signed-off-by: Marcus Watts <mwatts@redhat.com>
Without https, only port is set. With https, secure_port and port are both set to the same value. The previous logic looked at port first and had overly simplified conditional logic which was liable to try to apply both non-default cases. The correct behavior is: look secure_port first, and if secure_port is set, then only check to see if it's a non-default port. Signed-off-by: Marcus Watts <mwatts@redhat.com>
…ith civetweb. For civetweb: accept a range of port numbers joined with '+'. Port numbers may include an ipaddress: prefix and 's' suffix. Additionally, use "mg_get_local_addr" to correctly deduce host port per incoming connection. civetweb can accept connections on multiple ports, some of which might have SSL turned on and some not. Both s3 and swift have various authorization protocols in which the port number matters. In the generic radosgw frontend process, each frontend only has one port number, but we should want to have both ssl and non-ssl connections managed within one rgw frontend, because the thread pool is also per front-end, and that *is* a scarce resource. So, this patch enables the use of multiple ports with a single civetweb frontend. To indicate https: append an 's' to portno. To use multiple ports, use +. So 80+443s indicates use of the usual default http ports. The parsed port is not stored in the frontend structure, So instead, this patch adds logic to use the results of mg_get_local_addr() on a per-connection basis insetad of the generic front-end port number. This will affect "v4" s3 authorization, and also affect swift pre-signed URLs. mg_get_local_addr() is a new customization to civetweb; that submodule was updated (in a temporary repository) by the previous commit to this. Signed-off-by: Marcus Watts <mwatts@redhat.com>
mdw-at-linuxbox
force-pushed
the
wip-rgw-openssl-7
branch
from
91c41ae
to
8bc6dec
Compare
|
I've fixed the conflict I had with master, and improved commit wording. 2826aaa rgw: really rebase rgw to civetweb 1.8 now 2826aaa - this is the same as the commit in #12510 - and it will be overridden by the commit in 46ced9d. a113cf5 - this fixes the code that had been in conflict, basically the logic was not quite right for https. |
ghost
mentioned this pull request
So this is similar to "wip-rgw-openssl-4" except using civetweb 1.8.
f5f8c99 use civetweb-1.8. (this should be similar to wip-civetweb-1.8...)
5380caf load by soname same as pr#11571.
7da0ed4 document same as pr#11571.
33c1367 now uses "wip-listen1". - note, uses mdw-at-linuxbox fork (+ see ceph/civetweb#15 )
16db4e4 same as last push I did for pr#11571.