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.

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

Port plugin extension #2253

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
1a64007
network: Started port plugin
ZronekM Sep 24, 2018
f78a7d7
network: Implemented core port functionality
ZronekM Sep 25, 2018
49bfd25
network: Implemented port functionality
ZronekM Sep 26, 2018
1ddefa8
network: Minor Cleanup
ZronekM Sep 26, 2018
2f0a8d7
network: Minor Cleanup
ZronekM Sep 26, 2018
de603fb
Merge conflicts & Formatting
ZronekM Oct 4, 2018
fbfd1ec
Formatting
ZronekM Oct 4, 2018
2a8eab8
Formatting
ZronekM Oct 4, 2018
42892a9
Formatting
ZronekM Oct 4, 2018
7f69d5a
Formatting
ZronekM Oct 4, 2018
c1478ae
Formatting
ZronekM Oct 4, 2018
be49d6e
Implicit function declaration removed
ZronekM Oct 4, 2018
5bdb8f0
Merge remote-tracking branch 'origin/port-plugin-extension' into port…
ZronekM Oct 4, 2018
d141ec6
Formatting
ZronekM Oct 4, 2018
95d50bb
FUCKING formatting
ZronekM Oct 4, 2018
e5ebf73
Formatting
ZronekM Oct 4, 2018
3d25ff8
Network: Reformat source code
sanssecours Oct 5, 2018
5220150
testFix
ZronekM Oct 5, 2018
e6d5926
plugin: Formatting fixes for port
ZronekM Oct 4, 2018
df45c69
Merge branch 'port-plugin-extension' of github.com:Piankero/libelektr…
ZronekM Oct 13, 2018
5e43708
Merge branch 'master' of github.com:ElektraInitiative/libelektra into…
ZronekM Oct 13, 2018
6a90152
network: reduced error messages in specification, changed connect cal…
ZronekM Oct 13, 2018
02451d9
network: correct bind error return handling
ZronekM Oct 13, 2018
27b216e
network: removed confusing comment
ZronekM Oct 13, 2018
bee7be9
Merge
ZronekM Jan 11, 2019
ad43a18
network-plugin: After merge fix for elektra set error
ZronekM Jan 11, 2019
7cd5e4a
network: Removed fragile test and added comment for it
ZronekM Jan 11, 2019
aab7226
network: Clang formatting errors
ZronekM Jan 11, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions doc/METADATA.ini
Expand Up @@ -553,6 +553,21 @@ usedby/plugin= network ipaddr
description= check if value resolves correctly to an ipaddr
empty value uses either ipv4 or ipv6

[check/port]
type= string
status = implemented
usedby/plugin= network
description= check if the given port is either a service under /etc/services or
is between 0 - 65535 (both inclusive)

[check/port/listen]
type= string
status = implemented
usedby/plugin= network
description= check if the given port is either a service under /etc/services or
is between 0 - 65535 (both inclusive) and is unused so that potential
applications can start with that port

[check/format]
type= string
status= idea
Expand Down
3 changes: 3 additions & 0 deletions doc/news/_preparation_next_release.md
Expand Up @@ -120,6 +120,9 @@ This new plugin parses a subset of YAML using a parser generated by [Bison](http
The `network` plugin now also allows for non-numerical hosts (i.e. "localhost") to be set and tries to
resolve it via DNS. *(Michael Zronek)*

The `network` plugin also supports port declarations to check if a portnumber is valid
or if the port is available to use. *(Michael Zronek)*

### <<Plugin2>>

- <<TODO>>
Expand Down
24 changes: 24 additions & 0 deletions src/error/specification
Expand Up @@ -1258,3 +1258,27 @@ severity:warning
ingroup:plugin
module:gpgme
macro:GPGME_INVALID_RECIPIENT

number:201
description:The given port was not withing a valid range
severity:error
ingroup:plugin
module:network

number:202
description:Could not find service name for port declaration
severity:error
ingroup:plugin
module:network

number:203
description:Could not use port as it is already in use
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need so many errors? Aren't the descriptions and the reasons somewhat overlapping?

Copy link
Author

Choose a reason for hiding this comment

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

Should I just do an "Port error" and reference to the specific error in the reason metadata?

Copy link
Contributor

Choose a reason for hiding this comment

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

You should generally redesign the errors to be more user friendly. Think about what applications and users care about.

Only if there are situations where applications want to distinguish one error from the other, the error number should be different.

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively we could also introduce some inheritance for error messages and something like kdbContainsError (KDB *key, int whicherror) (or similar for the higher-level API).

We can discuss this tomorrow if you already want to work on that topic. But maybe you should finish the tasks you started already before.

Copy link
Author

Choose a reason for hiding this comment

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

I refactored the errors and are only using a single one which I did not see anywhere else in the specification file. I also reused another error number from the range plugin

Copy link
Author

Choose a reason for hiding this comment

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

Error messages will be dealt with another issue #1789

severity:error
ingroup:plugin
module:network

number:204
description:Connection Error occured
severity:error
ingroup:plugin
module:network
22 changes: 17 additions & 5 deletions src/plugins/network/README.md
Expand Up @@ -5,7 +5,7 @@
- infos/needs =
- infos/placements = presetstorage
- infos/status = maintained unittest nodep libc nodoc
- infos/metadata = check/ipaddr
- infos/metadata = check/ipaddr check/port check/port/listen
- infos/description = Checks keys if they contain a valid ip address

## Introduction
Expand All @@ -14,6 +14,10 @@ This plugin is a check plugin that checks if a key contains a valid ip
address. It uses the `POSIX.1-2001` interface `getaddrinfo()` in order
to check if an ip address is valid.

Furthermore `getaddrinfo()` is used in `check/port` to resolve a port by its service name
which is defined under `/etc/services`. The portname is translated to the respective portnumber.
The plugin can be used to check for valid port numbers and if the set port is free to use.

## Purpose

While, in theory, a regular expression can express if a string is a
Expand All @@ -36,7 +40,15 @@ it to implement this plugin.

Every key tagged with the metakey `check/ipaddr` will be checked
using `getaddrinfo()`. If additionally the values `ipv4` or `ipv6`
are supplied, the address family will be specified. If supplied only
numerical hosts are allowed. If left empty, the plugin will resolve
domain names and look if it is reachable (i.e. "localhost" should most
likely work on any system)
are supplied, the address family will be specified.

If `check/port` is specified on a given key, the plugin will validate if the port is a
correct number between 1 and 65535.

If `check/port/listen` is specified, the plugin will check if the application can be started
and listen on the given port.

## Future Work

`check/port/connect` to check if the port can be pinged/reached (usually for clients).
If not reachable, users receive a warning. A correct timeout setting will be problematic though.
71 changes: 70 additions & 1 deletion src/plugins/network/network.c
Expand Up @@ -50,6 +50,70 @@ int elektraNetworkAddrInfo (Key * toCheck)
return 0;
}

int elektraPortInfo(Key * toCheck, Key * parentKey) {
const Key * meta = keyGetMeta (toCheck, "check/port");
const Key * listenMeta = keyGetMeta (toCheck, "check/port/listen");
if (!meta && !listenMeta) return 0; /* No check to do */
char *endptr = NULL;
long portNumber = strtol(keyString(toCheck), &endptr, 10);
int portNumberNetworkByteOrder;

if (*endptr == '\0') {
if (portNumber < 0 || portNumber > 65535 || *endptr != 0) {
Copy link
Member

Choose a reason for hiding this comment

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

I think you can remove the check *endptr != 0 here. As far as I can tell, *endptr should always be 0 in this part of the code, since the if-statement one line above:

if (*endptr == '\0')

makes sure that this is the case.

ELEKTRA_SET_ERRORF(201, parentKey, "Port %d on key %s was not within 0 - 65535",
portNumber, keyName(toCheck));
return -1;
}
portNumberNetworkByteOrder = htons(portNumber);
} else {
struct servent* service;
service = getservbyname(keyString(toCheck), NULL); //NULL means we accept both tcp and udp
if (service == NULL) {
ELEKTRA_SET_ERRORF(202, parentKey, "Could not find service with name %s on key %s",
keyString(toCheck), keyName(toCheck));
return -1;
}
portNumberNetworkByteOrder = service->s_port;
}

//Check if we can connect to it
if (!listenMeta) return 0; /* No check to do */

char *hostname = "localhost";

int sockfd;
struct sockaddr_in serv_addr;
struct hostent *server;
sockfd = socket(AF_INET, SOCK_STREAM, 0);
if (sockfd < 0) {
ELEKTRA_SET_ERROR(204, parentKey, "Could not open a socket");
}

server = gethostbyname(hostname);
if (server == NULL) {
ELEKTRA_SET_ERRORF(204, parentKey, "Could not connect to %s: No such host",
hostname);
return -1;
}

bzero((char *) &serv_addr, sizeof(serv_addr));
serv_addr.sin_family = AF_INET;
bcopy((char *)server->h_addr,
(char *)&serv_addr.sin_addr.s_addr,
server->h_length);

serv_addr.sin_port = (in_port_t) portNumberNetworkByteOrder;
if (connect(sockfd,(struct sockaddr *) &serv_addr,sizeof(serv_addr)) == 0) {
close(sockfd);
ELEKTRA_SET_ERRORF(203, parentKey, "Port %s is already in use which was specified on key %s",
keyString(toCheck), keyName(toCheck));
return -1;
}
close(sockfd);

return 0;
}

int elektraNetworkGet (Plugin * handle ELEKTRA_UNUSED, KeySet * returned, Key * parentKey ELEKTRA_UNUSED)
{
/* configuration only */
Expand All @@ -61,6 +125,8 @@ int elektraNetworkGet (Plugin * handle ELEKTRA_UNUSED, KeySet * returned, Key *
keyNew ("system/elektra/modules/network/exports/set", KEY_FUNC, elektraNetworkSet, KEY_END),
keyNew ("system/elektra/modules/network/exports/elektraNetworkAddrInfo", KEY_FUNC, elektraNetworkAddrInfo,
KEY_END),
keyNew ("system/elektra/modules/network/exports/elektraPortInfo", KEY_FUNC, elektraNetworkAddrInfo,
KEY_END),
#include "readme_network.c"
keyNew ("system/elektra/modules/network/infos/version", KEY_VALUE, PLUGINVERSION, KEY_END), KS_END));
ksDel (n);
Expand All @@ -72,7 +138,6 @@ int elektraNetworkSet (Plugin * handle ELEKTRA_UNUSED, KeySet * returned, Key *
{
/* check all keys */
Key * cur;

ksRewind (returned);
while ((cur = ksNext (returned)) != 0)
{
Expand All @@ -92,6 +157,10 @@ int elektraNetworkSet (Plugin * handle ELEKTRA_UNUSED, KeySet * returned, Key *
elektraFree (errmsg);
return -1;
}
int p = elektraPortInfo(cur, parentKey);
if (p != 0) {
return -1;
}
}

return 1; /* success */
Expand Down
1 change: 1 addition & 0 deletions src/plugins/network/network.h
Expand Up @@ -21,6 +21,7 @@
#include <unistd.h>

int elektraNetworkAddrInfo (Key * toCheck);
int elektraPortInfo(Key * toCheck, Key * parentKey);

int elektraNetworkGet (Plugin * handle, KeySet * ks, Key * parentKey);
int elektraNetworkSet (Plugin * handle, KeySet * ks, Key * parentKey);
Expand Down
48 changes: 48 additions & 0 deletions src/plugins/network/testmod_network.c
Expand Up @@ -17,6 +17,9 @@
#include <tests.h>

#define PLUGIN_NAME "network"

static void testPorts() ;

#include "../ipaddr/test_ipaddr.h"

int main (int argc, char ** argv)
Expand All @@ -27,8 +30,53 @@ int main (int argc, char ** argv)
init (argc, argv);

testIPAll ();
testPorts ();

print_result ("testmod_network");

return nbError;
}

static void testPort (char const * const port, const int ret, char const * const version, char const * const metaName)
{
Key * parentKey = keyNew ("user/tests/port", KEY_VALUE, "", KEY_END);
KeySet * conf = ksNew (0, KS_END);
KeySet * ks = ksNew (10, keyNew ("user/test/port/totest", KEY_VALUE, port, KEY_META, metaName, version, KEY_END), KS_END);
PLUGIN_OPEN (PLUGIN_NAME);
const int pluginStatus = plugin->kdbSet (plugin, ks, parentKey);
char message[200];
(void) snprintf (message, 200, "validation of %s “%s” returned %d instead of %d", version[0] == '\0' ? "Port" : version, port,
pluginStatus, ret);
succeed_if (pluginStatus == ret, message);
ksDel (ks);
keyDel (parentKey);
PLUGIN_CLOSE ();
}

static inline void testPortAny (char const * const port, int ret)
{
testPort (port, ret, "", "check/port");
}

static inline void testListenPortAny (char const * const port, int ret)
{
testPort (port, ret, "", "check/port/listen");
}

static void testPorts() {
testPortAny("0", 1);
testPortAny("1234", 1);
testPortAny("65535", 1);
testPortAny("ssh", 1);
testPortAny("https", 1);

testPortAny("65536", -1);
testPortAny("-1", -1);
testPortAny("22d", -1);
testPortAny("myInvalidServiceName", -1);

//These tests aren't portable I guess
testListenPortAny("http", -1);
testListenPortAny("8080", 1);

}