Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nope-ip removal - Phase 1 #8714

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+27 −22
Open

nope-ip removal - Phase 1 #8714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/rpc/rpc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
const _ = require('lodash');
const util = require('util');
const assert = require('assert');
// const ip_module = require('ip');
const EventEmitter = require('events').EventEmitter;

const P = require('../util/promise');
Expand Down
7 changes: 3 additions & 4 deletions src/server/common_services/auth_server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

const _ = require('lodash');
const bcrypt = require('bcrypt');
const ip_module = require('ip');

const P = require('../../util/promise');
const dbg = require('../../util/debug_module')(__filename);
Expand Down Expand Up @@ -578,10 +577,10 @@ function _prepare_auth_request(req) {
const client_ip = net_utils.unwrap_ipv6(req.auth.client_ip);
if (client_ip) {
let is_allowed = false;
const client_ip_val = ip_module.toLong(client_ip);
const client_ip_val = net_utils.ip_toLong(client_ip);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if we still need allowed_ips in accounts. maybe we should get rid of that? then we don't need most of the code here

for (const ip_range of req.account.allowed_ips) {
const start = ip_module.toLong(ip_range.start);
const end = ip_module.toLong(ip_range.end);
const start = net_utils.ip_toLong(ip_range.start);
const end = net_utils.ip_toLong(ip_range.end);
if (client_ip_val >= start && client_ip_val <= end) {
is_allowed = true;
break;
Expand Down
21 changes: 11 additions & 10 deletions src/util/http_utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
/* eslint-disable no-control-regex */

const _ = require('lodash');
const ip = require('ip');
const ip_module = require('ip');
const net = require('net');
const url = require('url');
const http = require('http');
const https = require('https');
Expand Down Expand Up @@ -44,15 +45,15 @@ const unsecured_https_proxy_agent = HTTPS_PROXY ?

const no_proxy_list =
(NO_PROXY ? NO_PROXY.split(',') : []).map(addr => {
if (ip.isV4Format(addr) || ip.isV6Format(addr)) {
if (net.isIPv4(addr) || net.isIPv6(addr)) {
return {
kind: 'IP',
addr
};
}

try {
ip.cidr(addr);
ip_module.cidr(addr);
return {
kind: 'CIDR',
addr
Expand Down Expand Up @@ -383,16 +384,16 @@ function send_reply(req, res, reply, options) {
* Check if a hostname should be proxied or not
*/
function should_proxy(hostname) {
const isIp = ip.isV4Format(hostname) || ip.isV6Format(hostname);
const isIp = net.isIPv4(hostname) || net.isIPv6(hostname);
dbg.log2(`should_proxy: hostname ${hostname} isIp ${isIp}`);

for (const { kind, addr } of no_proxy_list) {
dbg.log3(`should_proxy: an item from no_proxy_list: kind ${kind} addr ${addr}`);
if (isIp) {
if (kind === 'IP' && ip.isEqual(addr, hostname)) {
if (kind === 'IP' && ip_module.isEqual(addr, hostname)) {
return false;
}
if (kind === 'CIDR' && ip.cidrSubnet(addr).contains(hostname)) {
if (kind === 'CIDR' && ip_module.cidrSubnet(addr).contains(hostname)) {
return false;
}

Expand Down Expand Up @@ -756,7 +757,7 @@ function http_get(uri, options) {
* @param {number} https_port
* @param {('S3'|'IAM'|'STS'|'METRICS')} server_type
* @param {Object} request_handler
*/
*/
async function start_https_server(https_port, server_type, request_handler, nsfs_config_root) {
const ssl_cert_info = await ssl_utils.get_ssl_cert_info(server_type, nsfs_config_root);
const https_server = await ssl_utils.create_https_server(ssl_cert_info, true, request_handler);
Expand All @@ -775,7 +776,7 @@ async function start_https_server(https_port, server_type, request_handler, nsfs
* @param {number} http_port
* @param {('S3'|'IAM'|'STS'|'METRICS')} server_type
* @param {Object} request_handler
*/
*/
async function start_http_server(http_port, server_type, request_handler) {
const http_server = http.createServer(request_handler);
if (http_port > 0) {
Expand All @@ -790,7 +791,7 @@ async function start_http_server(http_port, server_type, request_handler) {
* @param {number} port
* @param {http.Server} server
* @param {('S3'|'IAM'|'STS'|'METRICS')} server_type
*/
*/
function listen_port(port, server, server_type) {
return new Promise((resolve, reject) => {
if (server_type !== 'METRICS') {
Expand All @@ -810,7 +811,7 @@ function listen_port(port, server, server_type) {
/**
* Setup endpoint socket and server, Setup is not used for non-endpoint servers.
* @param {http.Server} server
*/
*/
function setup_endpoint_server(server) {
// Handle 'Expect' header different than 100-continue to conform with AWS.
// Consider any expect value as if the client is expecting 100-continue.
Expand Down
20 changes: 13 additions & 7 deletions src/util/net_utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

const _ = require('lodash');
const os = require('os');
const url = require('url');
const net = require('net');
const dns = require('dns');
const ip_module = require('ip');
Expand All @@ -26,7 +25,7 @@ async function ping(target, options) {

options = options || DEFAULT_PING_OPTIONS;
_.defaults(options, DEFAULT_PING_OPTIONS);
const candidate_ip = url.parse(target).hostname || target;
const candidate_ip = new URL(target).hostname || target;

if (net.isIP(candidate_ip)) {
await _ping_ip(candidate_ip);
Expand All @@ -49,7 +48,7 @@ function _ping_ip(session, ip) {
}

async function dns_resolve(target, options) {
const modified_target = url.parse(target).hostname || target;
const modified_target = new URL(target).hostname || target;
await os_utils.get_dns_config(); // unused? needed?
const res = await dns.promises.resolve(modified_target, (options && options.rrtype) || 'A');
return res;
Expand Down Expand Up @@ -90,18 +89,24 @@ function unwrap_ipv6(ip) {
return ip;
}

//the name ip_toLong consist of camel case and underscore, to indicate that toLong is the function we had in node-ip
function ip_toLong(ip) {
// eslint-disable-next-line no-bitwise
return ip.split('.').reduce((acc, octet) => (acc << 8) + parseInt(octet, 10), 0) >>> 0;
}

function ip_to_long(ip) {
return ip_module.toLong(unwrap_ipv6(ip));
return ip_toLong(unwrap_ipv6(ip));
}

function is_ip(address) {
return ip_module.isV4Format(address) || ip_module.isV6Format(address);
return net.isIPv4(address) || net.isIPv6(address);
}

function find_ifc_containing_address(address) {
const family =
(ip_module.isV4Format(address) && 'IPv4') ||
(ip_module.isV6Format(address) && 'IPv6') ||
(net.isIPv4(address) && 'IPv4') ||
(net.isIPv6(address) && 'IPv6') ||
'';
if (!family) return;
for (const [ifc, arr] of Object.entries(os.networkInterfaces())) {
Expand All @@ -120,5 +125,6 @@ exports.is_ip = is_ip;
exports.is_fqdn = is_fqdn;
exports.is_localhost = is_localhost;
exports.unwrap_ipv6 = unwrap_ipv6;
exports.ip_toLong = ip_toLong;
exports.ip_to_long = ip_to_long;
exports.find_ifc_containing_address = find_ifc_containing_address;
Loading