Skip to content

Commit 274dc14

Browse files
committed
Certificates: Fuse meta and level stacks
These stacks always had the same size, and their corresponding elements always referred to the same certificate. This was pending work from #55, which I think is now properly solved. Also refactors x509stack_push(); was messy. Patched an unlikely memory leak in the chaos.
1 parent b6dbf0e commit 274dc14

File tree

2 files changed

+99
-83
lines changed

2 files changed

+99
-83
lines changed

Diff for: src/cert_stack.c

+96-83
Original file line numberDiff line numberDiff line change
@@ -56,24 +56,19 @@ struct metadata_node {
5656
struct serial_numbers serials;
5757
struct subjects subjects;
5858

59+
/*
60+
* Certificate repository "level". This aims to identify if the
61+
* certificate is located at a distinct server than its father (common
62+
* case when the RIRs delegate RPKI repositories).
63+
*/
64+
unsigned int level;
65+
5966
/** Used by certstack. Points to the next stacked certificate. */
6067
SLIST_ENTRY(metadata_node) next;
6168
};
6269

6370
SLIST_HEAD(metadata_stack, metadata_node);
6471

65-
/**
66-
* Certificate repository "level". This aims to identify if the
67-
* certificate is located at a distinct server than its father (common
68-
* case when the RIRs delegate RPKI repositories).
69-
*/
70-
struct repo_level_node {
71-
unsigned int level;
72-
SLIST_ENTRY(repo_level_node) next;
73-
};
74-
75-
SLIST_HEAD(repo_level_stack, repo_level_node);
76-
7772
/**
7873
* This is the foundation through which we pull off our iterative traversal,
7974
* as opposed to a stack-threatening recursive one.
@@ -109,12 +104,6 @@ struct cert_stack {
109104
* seemingly not intended to be used outside of its library.)
110105
*/
111106
struct metadata_stack metas;
112-
113-
/**
114-
* Stacked data to store the repository "levels" (each level is a
115-
* delegation of an RPKI server).
116-
*/
117-
struct repo_level_stack levels;
118107
};
119108

120109
int
@@ -134,7 +123,6 @@ certstack_create(struct cert_stack **result)
134123

135124
SLIST_INIT(&stack->defers);
136125
SLIST_INIT(&stack->metas);
137-
SLIST_INIT(&stack->levels);
138126

139127
*result = stack;
140128
return 0;
@@ -185,7 +173,6 @@ certstack_destroy(struct cert_stack *stack)
185173
unsigned int stack_size;
186174
struct metadata_node *meta;
187175
struct defer_node *post;
188-
struct repo_level_node *level;
189176

190177
stack_size = 0;
191178
while (!SLIST_EMPTY(&stack->defers)) {
@@ -208,15 +195,6 @@ certstack_destroy(struct cert_stack *stack)
208195
}
209196
pr_val_debug("Deleted %u metadatas.", stack_size);
210197

211-
stack_size = 0;
212-
while (!SLIST_EMPTY(&stack->levels)) {
213-
level = SLIST_FIRST(&stack->levels);
214-
SLIST_REMOVE_HEAD(&stack->levels, next);
215-
free(level);
216-
stack_size++;
217-
}
218-
pr_val_debug("Deleted %u stacked levels.", stack_size);
219-
220198
free(stack);
221199
}
222200

@@ -242,7 +220,6 @@ x509stack_pop(struct cert_stack *stack)
242220
{
243221
X509 *cert;
244222
struct metadata_node *meta;
245-
struct repo_level_node *repo;
246223

247224
cert = sk_X509_pop(stack->x509s);
248225
if (cert == NULL)
@@ -254,12 +231,6 @@ x509stack_pop(struct cert_stack *stack)
254231
pr_crit("Attempted to pop empty metadata stack");
255232
SLIST_REMOVE_HEAD(&stack->metas, next);
256233
meta_destroy(meta);
257-
258-
repo = SLIST_FIRST(&stack->levels);
259-
if (repo == NULL)
260-
pr_crit("Attempted to pop empty repo level stack");
261-
SLIST_REMOVE_HEAD(&stack->levels, next);
262-
free(repo);
263234
}
264235

265236
/**
@@ -297,89 +268,131 @@ deferstack_is_empty(struct cert_stack *stack)
297268
return SLIST_EMPTY(&stack->defers);
298269
}
299270

271+
static int
272+
init_resources(X509 *x509, enum rpki_policy policy, enum cert_type type,
273+
struct resources **_result)
274+
{
275+
struct resources *result;
276+
int error;
277+
278+
result = resources_create(false);
279+
if (result == NULL)
280+
return pr_enomem();
281+
282+
resources_set_policy(result, policy);
283+
error = certificate_get_resources(x509, result, type);
284+
if (error)
285+
goto fail;
286+
287+
/*
288+
* rfc8630#section-2.3
289+
* "The INR extension(s) of this TA MUST contain a non-empty set of
290+
* number resources."
291+
* The "It MUST NOT use the "inherit" form of the INR extension(s)"
292+
* part is already handled in certificate_get_resources().
293+
*/
294+
if (type == TA && resources_empty(result)) {
295+
error = pr_val_err("Trust Anchor certificate does not define any number resources.");
296+
goto fail;
297+
}
298+
299+
*_result = result;
300+
return 0;
301+
302+
fail:
303+
resources_destroy(result);
304+
return error;
305+
}
306+
307+
static int
308+
init_level(struct cert_stack *stack, unsigned int *_result)
309+
{
310+
struct metadata_node *head_meta;
311+
unsigned int work_repo_level;
312+
unsigned int result;
313+
314+
/*
315+
* Bruh, I don't understand the point of this block.
316+
* Why can't it just be `result = working_repo_peek_level();`?
317+
*/
318+
319+
result = 0;
320+
work_repo_level = working_repo_peek_level();
321+
head_meta = SLIST_FIRST(&stack->metas);
322+
if (head_meta != NULL && work_repo_level > head_meta->level)
323+
result = work_repo_level;
324+
325+
*_result = result;
326+
return 0;
327+
}
328+
329+
static struct defer_node *
330+
create_separator(void)
331+
{
332+
struct defer_node *result;
333+
334+
result = malloc(sizeof(struct defer_node));
335+
if (result == NULL)
336+
return NULL;
337+
338+
result->type = DNT_SEPARATOR;
339+
return result;
340+
}
341+
300342
/** Steals ownership of @x509 on success. */
301343
int
302344
x509stack_push(struct cert_stack *stack, struct rpki_uri *uri, X509 *x509,
303345
enum rpki_policy policy, enum cert_type type)
304346
{
305347
struct metadata_node *meta;
306-
struct repo_level_node *repo, *head_repo;
307348
struct defer_node *defer_separator;
308-
unsigned int work_repo_level;
309349
int ok;
310350
int error;
311351

312-
repo = malloc(sizeof(struct repo_level_node));
313-
if (repo == NULL)
314-
return pr_enomem();
315-
316-
repo->level = 0;
317-
work_repo_level = working_repo_peek_level();
318-
head_repo = SLIST_FIRST(&stack->levels);
319-
if (head_repo != NULL && work_repo_level > head_repo->level)
320-
repo->level = work_repo_level;
321-
322-
SLIST_INSERT_HEAD(&stack->levels, repo, next);
323-
324352
meta = malloc(sizeof(struct metadata_node));
325-
if (meta == NULL) {
326-
error = pr_enomem();
327-
goto end3;
328-
}
353+
if (meta == NULL)
354+
return pr_enomem();
329355

330356
meta->uri = uri;
331357
uri_refget(uri);
332358
serial_numbers_init(&meta->serials);
333359
subjects_init(&meta->subjects);
334360

335-
meta->resources = resources_create(false);
336-
if (meta->resources == NULL) {
337-
error = pr_enomem();
338-
goto end4;
339-
}
340-
resources_set_policy(meta->resources, policy);
341-
error = certificate_get_resources(x509, meta->resources, type);
361+
error = init_resources(x509, policy, type, &meta->resources);
342362
if (error)
343-
goto end5;
363+
goto cleanup_subjects;
344364

345-
/*
346-
* rfc8630#section-2.3
347-
* "The INR extension(s) of this TA MUST contain a non-empty set of
348-
* number resources."
349-
* The "It MUST NOT use the "inherit" form of the INR extension(s)"
350-
* part is already handled in certificate_get_resources().
351-
*/
352-
if (type == TA && resources_empty(meta->resources)) {
353-
error = pr_val_err("Trust Anchor certificate does not define any number resources.");
354-
goto end5;
355-
}
365+
error = init_level(stack, &meta->level); /* Does not need a revert */
366+
if (error)
367+
goto destroy_resources;
356368

357-
defer_separator = malloc(sizeof(struct defer_node));
369+
defer_separator = create_separator();
358370
if (defer_separator == NULL) {
359371
error = pr_enomem();
360-
goto end5;
372+
goto destroy_resources;
361373
}
362-
defer_separator->type = DNT_SEPARATOR;
363374

364375
ok = sk_X509_push(stack->x509s, x509);
365376
if (ok <= 0) {
366377
error = val_crypto_err(
367378
"Could not add certificate to trusted stack: %d", ok);
368-
goto end5;
379+
goto destroy_separator;
369380
}
370381

371382
SLIST_INSERT_HEAD(&stack->defers, defer_separator, next);
372383
SLIST_INSERT_HEAD(&stack->metas, meta, next);
373384

374385
return 0;
375386

376-
end5: resources_destroy(meta->resources);
377-
end4: subjects_cleanup(&meta->subjects, subject_cleanup);
387+
destroy_separator:
388+
free(defer_separator);
389+
destroy_resources:
390+
resources_destroy(meta->resources);
391+
cleanup_subjects:
392+
subjects_cleanup(&meta->subjects, subject_cleanup);
378393
serial_numbers_cleanup(&meta->serials, serial_cleanup);
379394
uri_refput(meta->uri);
380395
free(meta);
381-
end3: SLIST_REMOVE_HEAD(&stack->levels, next);
382-
free(repo);
383396
return error;
384397
}
385398

@@ -428,8 +441,8 @@ x509stack_peek_resources(struct cert_stack *stack)
428441
unsigned int
429442
x509stack_peek_level(struct cert_stack *stack)
430443
{
431-
struct repo_level_node *repo = SLIST_FIRST(&stack->levels);
432-
return (repo != NULL) ? repo->level : 0;
444+
struct metadata_node *meta = SLIST_FIRST(&stack->metas);
445+
return (meta != NULL) ? meta->level : 0;
433446
}
434447

435448
static int

Diff for: src/object/certificate.c

+3
Original file line numberDiff line numberDiff line change
@@ -1152,6 +1152,9 @@ __certificate_get_resources(X509 *cert, struct resources *resources,
11521152
return 0;
11531153
}
11541154

1155+
/**
1156+
* Copies the resources from @cert to @resources.
1157+
*/
11551158
int
11561159
certificate_get_resources(X509 *cert, struct resources *resources,
11571160
enum cert_type type)

0 commit comments

Comments
 (0)