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

Ticket31684 #1328

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions changes/ticket31684
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
o Minor features (onion services):
- Implement a new GETINFO command to fetch microdescriptor consensus. Closes ticket 31684.

Copy link
Contributor

Choose a reason for hiding this comment

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

Spacing

Copy link
Contributor

Choose a reason for hiding this comment

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

Typos

o Code simplification and refactoring (onion services):
- Create a helper function that can fetch network status or microdesc consensuses.

63 changes: 48 additions & 15 deletions src/feature/control/control_getinfo.c
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,41 @@ getinfo_helper_current_time(control_connection_t *control_conn,
return 0;
}

/** GETINFO helper for dumping different consensus flavors
* returns: 0 on success -1 on error. */
STATIC int
getinfo_helper_current_consensus(consensus_flavor_t flavor,
char** answer,
const char** errmsg)
{
asn-d6 marked this conversation as resolved.
Show resolved Hide resolved
const char *flavor_name = networkstatus_get_flavor_name(flavor);
if (!strcmp(flavor_name, "??")) {
*errmsg = "Could not open cached consensus. "
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong error message

"Make sure FetchUselessDescriptors is set to 1.";
return -1;
}
if (we_want_to_fetch_flavor(get_options(), flavor)) {
/** Check from the cache */
const cached_dir_t *consensus = dirserv_get_consensus(flavor_name);
if (consensus) {
*answer = tor_strdup(consensus->dir);
}
}
if (!*answer) { /* try loading it from disk */
tor_mmap_t *mapped = networkstatus_map_cached_consensus(flavor_name);
if (mapped) {
*answer = tor_memdup_nulterm(mapped->data, mapped->size);
tor_munmap_file(mapped);
}
if (!*answer) { /* generate an error */
*errmsg = "Could not open cached consensus. "
"Make sure FetchUselessDescriptors is set to 1.";
return -1;
}
}
return 0;
}

/** Implementation helper for GETINFO: knows the answers for questions about
* directory information. */
STATIC int
Expand Down Expand Up @@ -362,6 +397,7 @@ getinfo_helper_dir(control_connection_t *control_conn,
if (body)
*answer = tor_strndup(body, ri->cache_info.signed_descriptor_len);
} else if (! we_fetch_router_descriptors(get_options())) {

asn-d6 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Spacing

/* Descriptors won't be available, provide proper error */
*errmsg = "We fetch microdescriptors, not router "
"descriptors. You'll need to use md/name/* "
Expand Down Expand Up @@ -576,24 +612,19 @@ getinfo_helper_dir(control_connection_t *control_conn,
smartlist_free(descs);
} else if (!strcmpstart(question, "dir/status/")) {
*answer = tor_strdup("");
} else if (!strcmp(question, "dir/status-vote/current/consensus")) { /* v3 */
if (we_want_to_fetch_flavor(get_options(), FLAV_NS)) {
const cached_dir_t *consensus = dirserv_get_consensus("ns");
if (consensus)
*answer = tor_strdup(consensus->dir);
} else if (!strcmp(question, "dir/status-vote/current/consensus")) {
int consensus_result = getinfo_helper_current_consensus(FLAV_NS,
answer, errmsg);
if (consensus_result == -1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We use < 0

return -1;
}
if (!*answer) { /* try loading it from disk */
tor_mmap_t *mapped = networkstatus_map_cached_consensus("ns");
if (mapped) {
*answer = tor_memdup_nulterm(mapped->data, mapped->size);
tor_munmap_file(mapped);
}
if (!*answer) { /* generate an error */
*errmsg = "Could not open cached consensus. "
"Make sure FetchUselessDescriptors is set to 1.";
} else if (!strcmp(question,
"dir/status-vote/current/consensus-microdesc")) {
int consensus_result = getinfo_helper_current_consensus(FLAV_MICRODESC,
answer, errmsg);
if (consensus_result == -1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We use < 0

return -1;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Weird spacing?

} else if (!strcmp(question, "network-status")) { /* v1 */
static int network_status_warned = 0;
if (!network_status_warned) {
Expand Down Expand Up @@ -1513,6 +1544,8 @@ static const getinfo_item_t getinfo_items[] = {
"v2 networkstatus docs as retrieved from a DirPort."),
ITEM("dir/status-vote/current/consensus", dir,
"v3 Networkstatus consensus as retrieved from a DirPort."),
ITEM("dir/status-vote/current/consensus-microdesc", dir,
ltbringer marked this conversation as resolved.
Show resolved Hide resolved
"v3 Microdescriptors consensus as retrieved from a DirPort."),
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo

ITEM("exit-policy/default", policies,
"The default value appended to the configured exit policy."),
ITEM("exit-policy/reject-private/default", policies,
Expand Down
4 changes: 4 additions & 0 deletions src/feature/control/control_getinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ STATIC int getinfo_helper_downloads(
control_connection_t *control_conn,
const char *question, char **answer,
const char **errmsg);
STATIC int getinfo_helper_current_consensus(
consensus_flavor_t flavor,
char **answer,
const char **errmsg);
STATIC int getinfo_helper_dir(
control_connection_t *control_conn,
const char *question, char **answer,
Expand Down
5 changes: 3 additions & 2 deletions src/feature/dircache/dirserv.c
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,9 @@ dirserv_set_cached_consensus_networkstatus(const char *networkstatus,

/** Return the latest downloaded consensus networkstatus in encoded, signed,
* optionally compressed format, suitable for sending to clients. */
cached_dir_t *
dirserv_get_consensus(const char *flavor_name)
MOCK_IMPL(cached_dir_t *,
dirserv_get_consensus,
(const char *flavor_name))
Copy link
Contributor

Choose a reason for hiding this comment

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

Spacing

{
if (!cached_consensuses)
return NULL;
Expand Down
2 changes: 1 addition & 1 deletion src/feature/dircache/dirserv.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ int directory_permits_begindir_requests(const or_options_t *options);
int directory_too_idle_to_fetch_descriptors(const or_options_t *options,
time_t now);

cached_dir_t *dirserv_get_consensus(const char *flavor_name);
MOCK_DECL(cached_dir_t *, dirserv_get_consensus, (const char *flavor_name));
void dirserv_set_cached_consensus_networkstatus(const char *consensus,
size_t consensus_len,
const char *flavor_name,
Expand Down
10 changes: 6 additions & 4 deletions src/feature/nodelist/networkstatus.c
Original file line number Diff line number Diff line change
Expand Up @@ -216,10 +216,12 @@ networkstatus_reset_download_failures(void)
}

/** Return the filename used to cache the consensus of a given flavor */
static char *
networkstatus_get_cache_fname(int flav,
const char *flavorname,
int unverified_consensus)

Copy link
Contributor

Choose a reason for hiding this comment

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

Spacing

MOCK_IMPL(char *,
networkstatus_get_cache_fname,
(int flav,
const char *flavorname,
int unverified_consensus))
{
char buf[128];
const char *prefix;
Expand Down
7 changes: 6 additions & 1 deletion src/feature/nodelist/networkstatus.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@

void networkstatus_reset_warnings(void);
void networkstatus_reset_download_failures(void);
tor_mmap_t *networkstatus_map_cached_consensus(const char *flavorname);
MOCK_DECL(char *,
networkstatus_get_cache_fname,
Copy link
Contributor

Choose a reason for hiding this comment

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

Spacing

(int flav,
const char *flavorname,
int unverified_consensus));
tor_mmap_t * networkstatus_map_cached_consensus(const char *flavorname);
Copy link
Contributor

Choose a reason for hiding this comment

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

Extra space

int router_reload_consensus_networkstatus(void);
void routerstatus_free_(routerstatus_t *rs);
#define routerstatus_free(rs) \
Expand Down
13 changes: 5 additions & 8 deletions src/lib/fs/mmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@
* failure, return NULL. Sets errno properly, using ERANGE to mean
* "empty file". Must only be called on trusted Tor-owned files, as changing
* the underlying file's size causes unspecified behavior. */
tor_mmap_t *
tor_mmap_file(const char *filename)
MOCK_IMPL(tor_mmap_t *, tor_mmap_file, (const char *filename))
Copy link
Contributor

Choose a reason for hiding this comment

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

Spacing

{
int fd; /* router file */
char *string;
Expand Down Expand Up @@ -111,8 +110,7 @@ tor_mmap_file(const char *filename)
}
/** Release storage held for a memory mapping; returns 0 on success,
* or -1 on failure (and logs a warning). */
int
tor_munmap_file(tor_mmap_t *handle)
MOCK_IMPL(int, tor_munmap_file, (tor_mmap_t *handle))
Copy link
Contributor

Choose a reason for hiding this comment

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

Spacing

{
int res;

Expand All @@ -132,8 +130,8 @@ tor_munmap_file(tor_mmap_t *handle)
return res;
}
#elif defined(_WIN32)
tor_mmap_t *
tor_mmap_file(const char *filename)

MOCK_IMPL(tor_mmap_t *, tor_mmap_file, (const char *filename))
Copy link
Contributor

Choose a reason for hiding this comment

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

Spacing

{
TCHAR tfilename[MAX_PATH]= {0};
tor_mmap_t *res = tor_malloc_zero(sizeof(tor_mmap_t));
Expand Down Expand Up @@ -213,8 +211,7 @@ tor_mmap_file(const char *filename)
}

/* Unmap the file, and return 0 for success or -1 for failure */
int
tor_munmap_file(tor_mmap_t *handle)
MOCK_IMPL(int, tor_munmap_file, (tor_mmap_t *handle))
Copy link
Contributor

Choose a reason for hiding this comment

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

Spacing

{
if (handle == NULL)
return 0;
Expand Down
5 changes: 3 additions & 2 deletions src/lib/fs/mmap.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#define TOR_MMAP_H

#include "lib/cc/compat_compiler.h"
#include "lib/testsupport/testsupport.h"
#include <stddef.h>

#ifdef _WIN32
Expand All @@ -35,7 +36,7 @@ typedef struct tor_mmap_t {

} tor_mmap_t;

tor_mmap_t *tor_mmap_file(const char *filename);
int tor_munmap_file(tor_mmap_t *handle);
MOCK_DECL(tor_mmap_t *, tor_mmap_file, (const char *filename));
MOCK_DECL(int, tor_munmap_file, (tor_mmap_t *handle));

#endif /* !defined(TOR_MMAP_H) */