You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by zhilhuan <gi...@git.apache.org> on 2017/03/14 07:27:37 UTC
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
GitHub user zhilhuan opened a pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360
[TC-187] fix HTTPs bugs
use ds id as key to store ssl certificate in riak server;
update ssl certificate in keystore if delivery service modified xml-id or subdomain;
repose decoded certificate in restful api.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zhilhuan/incubator-trafficcontrol TC-187
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafficcontrol/pull/360.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #360
----
commit b95456a8274aabdf04a6385c067a4f896e8c8b1c
Author: Zhilin Huang <zh...@cisco.com>
Date: 2017-03-14T06:29:44Z
use ds id as key to store ssl certificate in riak server;
update ssl certificate in keystore if delivery service modified xml-id or subdomain;
repose decoded certificate in restful api.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #360: [TC-187] fix HTTPs bugs
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/360
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/26/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by rawlinp <gi...@git.apache.org>.
Github user rawlinp commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r133741257
--- Diff: traffic_ops/app/lib/UI/SslKeys.pm ---
@@ -34,7 +34,7 @@ sub add {
&stash_role($self);
#get key data from keystore
- my $response_container = $self->riak_get( 'ssl', "$xml_id-latest");
+ my $response_container = $self->riak_get( 'ssl', "ds_$ds_id-latest");
--- End diff --
a preexisting key won't be found and won't get stashed. I'm not sure how much of an issue that might be.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by zhilhuan <gi...@git.apache.org>.
Github user zhilhuan commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r134435019
--- Diff: traffic_ops/app/lib/UI/SslKeys.pm ---
@@ -75,6 +75,34 @@ sub add {
}
}
+sub update_sslkey {
+ my $self = shift;
+ my $ds_id = shift;
+ my $xml_id = shift;
+ my $hostname = shift;
+ my $response_container = $self->riak_get( 'ssl', "ds_$ds_id-latest");
--- End diff --
will use xml_id instead in TC-547.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by rawlinp <gi...@git.apache.org>.
Github user rawlinp commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r133740708
--- Diff: traffic_ops/app/script/update_riak_for_search.pl ---
@@ -46,9 +46,6 @@
}
$record->{deliveryservice} = $xml_id;
$record->{cdn} = $cdn;
- $record->{certificate}->{crt} = decode_base64($record->{certificate}->{crt});
--- End diff --
This is an example of a client that would break, if this particular API is being used by a client outside of our codebase.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by rawlinp <gi...@git.apache.org>.
Github user rawlinp commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r133737928
--- Diff: traffic_ops/app/lib/API/DeliveryService/SslKeys.pm ---
@@ -120,12 +120,23 @@ sub view_by_xml_id {
if ( !$version ) {
$version = 'latest';
}
- $key = "$key-$version";
+ my $ds = $self->db->resultset('Deliveryservice')->search( { xml_id => $xml_id })->single();
+ if (!$ds) {
+ return $self->alert( { Error => " - Could not found delivery service with xml_id=$xml_id!" } );
+ }
+ my $ds_id = $ds->id;
+ my $key = "ds_$ds_id-$version";
--- End diff --
This has the effect of not being able to find any preexisting keys for delivery services. Will all existing keys need to be re-added because of this change?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by rawlinp <gi...@git.apache.org>.
Github user rawlinp commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r133738122
--- Diff: traffic_ops/app/lib/API/DeliveryService/SslKeys.pm ---
@@ -161,30 +172,42 @@ sub view_by_hostname {
return $self->alert( { Error => " - A delivery service does not exist for a host with hostanme of $key" } );
}
- my $xml_id = $ds->xml_id;
+ my $ds_id = $ds->id;
if ( !$version ) {
$version = 'latest';
}
- $key = "$xml_id-$version";
+ $key = "ds_$ds_id-$version";
--- End diff --
see comment above about preexisting keys
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by rawlinp <gi...@git.apache.org>.
Github user rawlinp commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r133741608
--- Diff: traffic_ops/app/lib/UI/DeliveryService.pm ---
@@ -921,6 +919,10 @@ sub update {
}
}
+ my $new_hostname = UI::SslKeys::get_hostname($self, $id, $update);
+ $upd_ssl = 1 if $old_hostname ne $new_hostname;
--- End diff --
This is good, but shouldn't the API also perform the same check and update SSL keys if the hostname changed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by rawlinp <gi...@git.apache.org>.
Github user rawlinp commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r133738664
--- Diff: traffic_ops/app/lib/API/DeliveryService/SslKeys.pm ---
@@ -161,30 +172,42 @@ sub view_by_hostname {
return $self->alert( { Error => " - A delivery service does not exist for a host with hostanme of $key" } );
}
- my $xml_id = $ds->xml_id;
+ my $ds_id = $ds->id;
if ( !$version ) {
$version = 'latest';
}
- $key = "$xml_id-$version";
+ $key = "ds_$ds_id-$version";
my $response_container = $self->riak_get( "ssl", $key );
my $response = $response_container->{"response"};
- $response->is_success()
- ? $self->success( decode_json( $response->content ) )
- : $self->alert( { Error => " - A record for ssl key $key could not be found. Response was: " . $response->content } );
+ if ($response->is_success()) {
+ my $ssl_keys = decode_json( $response->content );
+ $ssl_keys->{certificate}->{csr} = decode_base64($ssl_keys->{certificate}->{csr}),
+ $ssl_keys->{certificate}->{crt} = decode_base64($ssl_keys->{certificate}->{crt}),
+ $ssl_keys->{certificate}->{key} = decode_base64($ssl_keys->{certificate}->{key}),
+ $self->success( $ssl_keys )
+ } else {
+ $self->alert( { Error => " - A record for ssl key $key could not be found. Response was: " . $response->content } );
+ }
}
}
sub delete {
my $self = shift;
- my $key = $self->param('xmlid');
+ my $xml_id = $self->param('xmlid');
my $version = $self->param('version');
my $response_container;
my $response;
if ( !&is_admin($self) ) {
return $self->alert( { Error => " - You must be an ADMIN to perform this operation!" } );
}
else {
+ my $ds = $self->db->resultset('Deliveryservice')->search( { xml_id => $xml_id })->single();
+ if (!$ds) {
+ return $self->alert( { Error => " - Could not found delivery service with xml_id=$xml_id!" } );
+ }
+ my $ds_id = $ds->id;
+ my $key = "ds_$ds_id";
--- End diff --
preexisting keys won't be deletable?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by zhilhuan <gi...@git.apache.org>.
Github user zhilhuan commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r134430403
--- Diff: traffic_ops/app/lib/API/DeliveryService/SslKeys.pm ---
@@ -161,30 +172,42 @@ sub view_by_hostname {
return $self->alert( { Error => " - A delivery service does not exist for a host with hostanme of $key" } );
}
- my $xml_id = $ds->xml_id;
+ my $ds_id = $ds->id;
if ( !$version ) {
$version = 'latest';
}
- $key = "$xml_id-$version";
+ $key = "ds_$ds_id-$version";
my $response_container = $self->riak_get( "ssl", $key );
my $response = $response_container->{"response"};
- $response->is_success()
- ? $self->success( decode_json( $response->content ) )
- : $self->alert( { Error => " - A record for ssl key $key could not be found. Response was: " . $response->content } );
+ if ($response->is_success()) {
+ my $ssl_keys = decode_json( $response->content );
+ $ssl_keys->{certificate}->{csr} = decode_base64($ssl_keys->{certificate}->{csr}),
+ $ssl_keys->{certificate}->{crt} = decode_base64($ssl_keys->{certificate}->{crt}),
+ $ssl_keys->{certificate}->{key} = decode_base64($ssl_keys->{certificate}->{key}),
+ $self->success( $ssl_keys )
+ } else {
+ $self->alert( { Error => " - A record for ssl key $key could not be found. Response was: " . $response->content } );
+ }
}
}
sub delete {
my $self = shift;
- my $key = $self->param('xmlid');
+ my $xml_id = $self->param('xmlid');
my $version = $self->param('version');
my $response_container;
my $response;
if ( !&is_admin($self) ) {
return $self->alert( { Error => " - You must be an ADMIN to perform this operation!" } );
}
else {
+ my $ds = $self->db->resultset('Deliveryservice')->search( { xml_id => $xml_id })->single();
+ if (!$ds) {
+ return $self->alert( { Error => " - Could not found delivery service with xml_id=$xml_id!" } );
+ }
+ my $ds_id = $ds->id;
+ my $key = "ds_$ds_id";
--- End diff --
will revert this change in TC-547.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by zhilhuan <gi...@git.apache.org>.
Github user zhilhuan commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r132073405
--- Diff: traffic_ops/app/lib/UI/DeliveryService.pm ---
@@ -846,8 +842,11 @@ sub update {
$hash{http_bypass_fqdn} = $self->param('ds.http_bypass_fqdn');
}
+ my $upd_ssl = 0;
#print Dumper( \%hash );
my $update = $self->db->resultset('Deliveryservice')->find( { id => $id } );
+ $upd_ssl = 1 if $update->xml_id ne $hash{xml_id};
--- End diff --
OK, will remove this line
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by zhilhuan <gi...@git.apache.org>.
Github user zhilhuan commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r134429710
--- Diff: traffic_ops/app/lib/API/DeliveryService/SslKeys.pm ---
@@ -120,12 +120,23 @@ sub view_by_xml_id {
if ( !$version ) {
$version = 'latest';
}
- $key = "$key-$version";
+ my $ds = $self->db->resultset('Deliveryservice')->search( { xml_id => $xml_id })->single();
+ if (!$ds) {
+ return $self->alert( { Error => " - Could not found delivery service with xml_id=$xml_id!" } );
+ }
+ my $ds_id = $ds->id;
+ my $key = "ds_$ds_id-$version";
--- End diff --
As discussed in Slack, xml_id is immutable now, will revert this change in TC-547.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by DylanVolz <gi...@git.apache.org>.
Github user DylanVolz commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r131963435
--- Diff: traffic_ops/app/lib/UI/DeliveryService.pm ---
@@ -846,8 +842,11 @@ sub update {
$hash{http_bypass_fqdn} = $self->param('ds.http_bypass_fqdn');
}
+ my $upd_ssl = 0;
#print Dumper( \%hash );
my $update = $self->db->resultset('Deliveryservice')->find( { id => $id } );
+ $upd_ssl = 1 if $update->xml_id ne $hash{xml_id};
--- End diff --
This line is may be unnecessary now that [TC-402] was merged.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafficcontrol/pull/360
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #360: [TC-187] fix HTTPs bugs
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/360
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by zhilhuan <gi...@git.apache.org>.
Github user zhilhuan commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r134430100
--- Diff: traffic_ops/app/lib/API/DeliveryService/SslKeys.pm ---
@@ -120,12 +120,23 @@ sub view_by_xml_id {
if ( !$version ) {
$version = 'latest';
}
- $key = "$key-$version";
+ my $ds = $self->db->resultset('Deliveryservice')->search( { xml_id => $xml_id })->single();
+ if (!$ds) {
+ return $self->alert( { Error => " - Could not found delivery service with xml_id=$xml_id!" } );
+ }
+ my $ds_id = $ds->id;
+ my $key = "ds_$ds_id-$version";
my $response_container = $self->riak_get( "ssl", $key );
my $response = $response_container->{"response"};
- $response->is_success()
- ? $self->success( decode_json( $response->content ) )
- : $self->alert( { Error => " - A record for ssl key $key could not be found. Response was: " . $response->content } );
+ if ($response->is_success()) {
+ my $ssl_keys = decode_json( $response->content );
+ $ssl_keys->{certificate}->{csr} = decode_base64($ssl_keys->{certificate}->{csr}),
--- End diff --
We made this change because it is not compatible with GUI interface. Since this may case regress issue, will revert it back in TC-547.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by rawlinp <gi...@git.apache.org>.
Github user rawlinp commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r133736688
--- Diff: traffic_ops/app/lib/API/DeliveryService/SslKeys.pm ---
@@ -161,30 +172,42 @@ sub view_by_hostname {
return $self->alert( { Error => " - A delivery service does not exist for a host with hostanme of $key" } );
}
- my $xml_id = $ds->xml_id;
+ my $ds_id = $ds->id;
if ( !$version ) {
$version = 'latest';
}
- $key = "$xml_id-$version";
+ $key = "ds_$ds_id-$version";
my $response_container = $self->riak_get( "ssl", $key );
my $response = $response_container->{"response"};
- $response->is_success()
- ? $self->success( decode_json( $response->content ) )
- : $self->alert( { Error => " - A record for ssl key $key could not be found. Response was: " . $response->content } );
+ if ($response->is_success()) {
+ my $ssl_keys = decode_json( $response->content );
+ $ssl_keys->{certificate}->{csr} = decode_base64($ssl_keys->{certificate}->{csr}),
--- End diff --
See comment above about base64 decoding
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #360: [TC-187] fix HTTPs bugs
Posted by zhilhuan <gi...@git.apache.org>.
Github user zhilhuan commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/360
@rawlinp rebased.
Yes, we found several SSL cert issues related to update delivery service xml_id or sub-domain. And fixed those issue in this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by zhilhuan <gi...@git.apache.org>.
Github user zhilhuan commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r134434791
--- Diff: traffic_ops/app/lib/UI/SslKeys.pm ---
@@ -34,7 +34,7 @@ sub add {
&stash_role($self);
#get key data from keystore
- my $response_container = $self->riak_get( 'ssl', "$xml_id-latest");
+ my $response_container = $self->riak_get( 'ssl', "ds_$ds_id-latest");
--- End diff --
will revert this change in TC-547.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by zhilhuan <gi...@git.apache.org>.
Github user zhilhuan commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r134430236
--- Diff: traffic_ops/app/lib/API/DeliveryService/SslKeys.pm ---
@@ -161,30 +172,42 @@ sub view_by_hostname {
return $self->alert( { Error => " - A delivery service does not exist for a host with hostanme of $key" } );
}
- my $xml_id = $ds->xml_id;
+ my $ds_id = $ds->id;
if ( !$version ) {
$version = 'latest';
}
- $key = "$xml_id-$version";
+ $key = "ds_$ds_id-$version";
my $response_container = $self->riak_get( "ssl", $key );
my $response = $response_container->{"response"};
- $response->is_success()
- ? $self->success( decode_json( $response->content ) )
- : $self->alert( { Error => " - A record for ssl key $key could not be found. Response was: " . $response->content } );
+ if ($response->is_success()) {
+ my $ssl_keys = decode_json( $response->content );
+ $ssl_keys->{certificate}->{csr} = decode_base64($ssl_keys->{certificate}->{csr}),
--- End diff --
will revert this change in TC-547.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by rawlinp <gi...@git.apache.org>.
Github user rawlinp commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r133734785
--- Diff: traffic_ops/app/lib/API/DeliveryService/SslKeys.pm ---
@@ -120,12 +120,23 @@ sub view_by_xml_id {
if ( !$version ) {
$version = 'latest';
}
- $key = "$key-$version";
+ my $ds = $self->db->resultset('Deliveryservice')->search( { xml_id => $xml_id })->single();
+ if (!$ds) {
+ return $self->alert( { Error => " - Could not found delivery service with xml_id=$xml_id!" } );
+ }
+ my $ds_id = $ds->id;
+ my $key = "ds_$ds_id-$version";
my $response_container = $self->riak_get( "ssl", $key );
my $response = $response_container->{"response"};
- $response->is_success()
- ? $self->success( decode_json( $response->content ) )
- : $self->alert( { Error => " - A record for ssl key $key could not be found. Response was: " . $response->content } );
+ if ($response->is_success()) {
+ my $ssl_keys = decode_json( $response->content );
+ $ssl_keys->{certificate}->{csr} = decode_base64($ssl_keys->{certificate}->{csr}),
--- End diff --
Is it necessary for these to be decoded within the API? Won't this break any clients that expect these to be base64 encoded? Also, the API reference still states that these are base64 encoded.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by zhilhuan <gi...@git.apache.org>.
Github user zhilhuan commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r134434310
--- Diff: traffic_ops/app/lib/UI/DeliveryService.pm ---
@@ -921,6 +919,10 @@ sub update {
}
}
+ my $new_hostname = UI::SslKeys::get_hostname($self, $id, $update);
+ $upd_ssl = 1 if $old_hostname ne $new_hostname;
--- End diff --
Will also add it to TC-547
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by zhilhuan <gi...@git.apache.org>.
Github user zhilhuan commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r134435236
--- Diff: traffic_ops/app/script/update_riak_for_search.pl ---
@@ -46,9 +46,6 @@
}
$record->{deliveryservice} = $xml_id;
$record->{cdn} = $cdn;
- $record->{certificate}->{crt} = decode_base64($record->{certificate}->{crt});
--- End diff --
will revert this change in TC-547.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by zhilhuan <gi...@git.apache.org>.
Github user zhilhuan commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r134430135
--- Diff: traffic_ops/app/lib/API/DeliveryService/SslKeys.pm ---
@@ -161,30 +172,42 @@ sub view_by_hostname {
return $self->alert( { Error => " - A delivery service does not exist for a host with hostanme of $key" } );
}
- my $xml_id = $ds->xml_id;
+ my $ds_id = $ds->id;
if ( !$version ) {
$version = 'latest';
}
- $key = "$xml_id-$version";
+ $key = "ds_$ds_id-$version";
--- End diff --
As discussed in Slack, xml_id is immutable now, will revert this change in TC-547.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by rawlinp <gi...@git.apache.org>.
Github user rawlinp commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r133740451
--- Diff: traffic_ops/app/lib/UI/DeliveryService.pm ---
@@ -152,12 +153,7 @@ sub get_example_urls {
}
}
elsif ( $re->{type} eq 'PATH_REGEXP' ) {
- if ( defined( $example_urls[ $re->{set_number} ] ) ) {
- $example_urls[ $re->{set_number} ] .= $re->{pattern};
- }
- else {
- $example_urls[ $re->{set_number} ] = $re->{pattern};
- }
+ push(@example_urls, $re->{pattern});
--- End diff --
What made this change necessary? Doesn't the old code concatenate patterns if they use the same `set_number`? This looks like it would just add multiple different patterns using the same `set_number`s.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by zhilhuan <gi...@git.apache.org>.
Github user zhilhuan commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r134432509
--- Diff: traffic_ops/app/lib/UI/DeliveryService.pm ---
@@ -152,12 +153,7 @@ sub get_example_urls {
}
}
elsif ( $re->{type} eq 'PATH_REGEXP' ) {
- if ( defined( $example_urls[ $re->{set_number} ] ) ) {
- $example_urls[ $re->{set_number} ] .= $re->{pattern};
- }
- else {
- $example_urls[ $re->{set_number} ] = $re->{pattern};
- }
+ push(@example_urls, $re->{pattern});
--- End diff --
It messed up when both "HOST_REGEXP" and "PATH_REGEXP" configured. Root cause:
1. Duplicated "set_number" is not allowed in DS configuration in GUI. So not "PATH_REGEXP" will share the same "set_number" with "HOST_REGEXP".
2. "$re->{set_number}" is not the index of "@example_urls". See the "push( @example_urls, $https_url );" code above. So "$example_urls[ $re->{set_number} ] .= $re->{pattern};" may mess up with any other "HOST_REGEXP".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #360: [TC-187] fix HTTPs bugs
Posted by rawlinp <gi...@git.apache.org>.
Github user rawlinp commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/360
@zhilhuan can you please rebase this PR?
This PR also seems to fix an issue we'll have once per-DeliveryService routing names are implemented soon. Basically, if a DNS DS starts using the DNS routing name "my-edge" rather than the default "edge", the new hostname will mismatch the hostname used in the SSL cert. Your PR seems to address that issue, which is helpful in addition to the issue of changing the xml_id.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by zhilhuan <gi...@git.apache.org>.
Github user zhilhuan commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r134434211
--- Diff: traffic_ops/app/lib/UI/DeliveryService.pm ---
@@ -921,6 +919,10 @@ sub update {
}
}
+ my $new_hostname = UI::SslKeys::get_hostname($self, $id, $update);
+ $upd_ssl = 1 if $old_hostname ne $new_hostname;
--- End diff --
good catch! I think the code change is missing when doing merge.
The original change is made in TC 1.7. When picked up to master, looks like the code is lost :-(
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #360: [TC-187] fix HTTPs bugs
Posted by rawlinp <gi...@git.apache.org>.
Github user rawlinp commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/360#discussion_r133740943
--- Diff: traffic_ops/app/lib/UI/SslKeys.pm ---
@@ -75,6 +75,34 @@ sub add {
}
}
+sub update_sslkey {
+ my $self = shift;
+ my $ds_id = shift;
+ my $xml_id = shift;
+ my $hostname = shift;
+ my $response_container = $self->riak_get( 'ssl', "ds_$ds_id-latest");
--- End diff --
preexisting keys can't be updated?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---