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

Jump to bottom

gate calling of evp_method_id on having a non-zero name id #23110

Closed
wants to merge 4 commits into from
Closed

gate calling of evp_method_id on having a non-zero name id #23110

wants to merge 4 commits into from

Conversation

nhorman
Copy link
Contributor

@nhorman nhorman commented Dec 20, 2023

If a name is passed to EVP__fetch of the form: name1:name2:name3

The names are parsed on the separator ':' and added to the store, but during the lookup in inner_evp_generic_fetch, the subsequent search of the store uses the full name1:name2:name3 string, which fails lookup, and causes subsequent assertion failures in evp_method_id.

instead catch the failure in inner_evp_generic_fetch and return an error code if the name_id against a colon separated list of names fails. This provides a graceful error return path without asserts, and leaves room for a future feature in which such formatted names can be parsed and searched for iteratively

Add a simple test to verify that providing a colon separated name results in an error indicating an invalid lookup.

Checklist
  • documentation is added or updated
  • tests are added or updated

@nhorman
gate calling of evp_method_id on having a non-zero name id
5838422 
If a name is passed to EVP_<OBJ>_fetch of the form:
name1:name2:name3

The names are parsed on the separator ':' and added to the store, but
during the lookup in inner_evp_generic_fetch, the subsequent search of
the store uses the full name1:name2:name3 string, which fails lookup,
and causes subsequent assertion failures in evp_method_id.

instead catch the failure in inner_evp_generic_fetch and return an error
code if the name_id against a colon separated list of names fails.  This
provides a graceful error return path without asserts, and leaves room
for a future feature in which such formatted names can be parsed and
searched for iteratively

Add a simple test to verify that providing a colon separated name
results in an error indicating an invalid lookup.
@nhorman nhorman self-assigned this Dec 20, 2023
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Dec 20, 2023
@t8m t8m added branch: master Merge to master branch approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member triaged: bug The issue/pr is/fixes a bug branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 branch: 3.2 Merge to openssl-3.2 labels Dec 20, 2023
t8m
t8m reviewed Dec 20, 2023
View reviewed changes
test/evp_test.c Outdated Show resolved Hide resolved
@t8m t8m added the tests: present The PR has suitable tests present label Dec 20, 2023
t8m
t8m requested changes Dec 22, 2023
View reviewed changes
test/evp_extra_test2.c Outdated Show resolved Hide resolved
t8m
t8m reviewed Dec 22, 2023
View reviewed changes
test/evp_extra_test2.c Outdated Show resolved Hide resolved
@nhorman nhorman requested a review from t8m December 22, 2023 18:02
t8m
t8m requested changes Dec 27, 2023
View reviewed changes
test/evp_extra_test2.c Outdated Show resolved Hide resolved
@nhorman nhorman requested a review from t8m December 27, 2023 15:52
@t8m t8m removed the approval: otc review pending This pull request needs review by an OTC member label Dec 27, 2023
@tmshort tmshort added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Dec 29, 2023
@openssl-machine openssl-machine removed the approval: done This pull request has the required number of approvals label Dec 30, 2023
@openssl-machine openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Dec 30, 2023
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

openssl-machine pushed a commit that referenced this pull request Jan 1, 2024
@nhorman
gate calling of evp_method_id on having a non-zero name id
94be985 
If a name is passed to EVP_<OBJ>_fetch of the form:
name1:name2:name3

The names are parsed on the separator ':' and added to the store, but
during the lookup in inner_evp_generic_fetch, the subsequent search of
the store uses the full name1:name2:name3 string, which fails lookup,
and causes subsequent assertion failures in evp_method_id.

instead catch the failure in inner_evp_generic_fetch and return an error
code if the name_id against a colon separated list of names fails.  This
provides a graceful error return path without asserts, and leaves room
for a future feature in which such formatted names can be parsed and
searched for iteratively

Add a simple test to verify that providing a colon separated name
results in an error indicating an invalid lookup.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Todd Short <todd.short@me.com>
(Merged from #23110)
openssl-machine pushed a commit that referenced this pull request Jan 1, 2024
@nhorman
gate calling of evp_method_id on having a non-zero name id
ac22ad6 
If a name is passed to EVP_<OBJ>_fetch of the form:
name1:name2:name3

The names are parsed on the separator ':' and added to the store, but
during the lookup in inner_evp_generic_fetch, the subsequent search of
the store uses the full name1:name2:name3 string, which fails lookup,
and causes subsequent assertion failures in evp_method_id.

instead catch the failure in inner_evp_generic_fetch and return an error
code if the name_id against a colon separated list of names fails.  This
provides a graceful error return path without asserts, and leaves room
for a future feature in which such formatted names can be parsed and
searched for iteratively

Add a simple test to verify that providing a colon separated name
results in an error indicating an invalid lookup.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Todd Short <todd.short@me.com>
(Merged from #23110)

(cherry picked from commit 94be985)
@nhorman nhorman closed this Jan 1, 2024
openssl-machine pushed a commit that referenced this pull request Jan 1, 2024
@nhorman
gate calling of evp_method_id on having a non-zero name id
ec29062 
If a name is passed to EVP_<OBJ>_fetch of the form:
name1:name2:name3

The names are parsed on the separator ':' and added to the store, but
during the lookup in inner_evp_generic_fetch, the subsequent search of
the store uses the full name1:name2:name3 string, which fails lookup,
and causes subsequent assertion failures in evp_method_id.

instead catch the failure in inner_evp_generic_fetch and return an error
code if the name_id against a colon separated list of names fails.  This
provides a graceful error return path without asserts, and leaves room
for a future feature in which such formatted names can be parsed and
searched for iteratively

Add a simple test to verify that providing a colon separated name
results in an error indicating an invalid lookup.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Todd Short <todd.short@me.com>
(Merged from #23110)

(cherry picked from commit 94be985)
openssl-machine pushed a commit that referenced this pull request Jan 1, 2024
@nhorman
gate calling of evp_method_id on having a non-zero name id
0f973b9 
If a name is passed to EVP_<OBJ>_fetch of the form:
name1:name2:name3

The names are parsed on the separator ':' and added to the store, but
during the lookup in inner_evp_generic_fetch, the subsequent search of
the store uses the full name1:name2:name3 string, which fails lookup,
and causes subsequent assertion failures in evp_method_id.

instead catch the failure in inner_evp_generic_fetch and return an error
code if the name_id against a colon separated list of names fails.  This
provides a graceful error return path without asserts, and leaves room
for a future feature in which such formatted names can be parsed and
searched for iteratively

Add a simple test to verify that providing a colon separated name
results in an error indicating an invalid lookup.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Todd Short <todd.short@me.com>
(Merged from #23110)

(cherry picked from commit 94be985)
wbeck10 pushed a commit to wbeck10/openssl that referenced this pull request Jan 8, 2024
@nhorman @wbeck10p
gate calling of evp_method_id on having a non-zero name id
4598f13 
If a name is passed to EVP_<OBJ>_fetch of the form:
name1:name2:name3

The names are parsed on the separator ':' and added to the store, but
during the lookup in inner_evp_generic_fetch, the subsequent search of
the store uses the full name1:name2:name3 string, which fails lookup,
and causes subsequent assertion failures in evp_method_id.

instead catch the failure in inner_evp_generic_fetch and return an error
code if the name_id against a colon separated list of names fails.  This
provides a graceful error return path without asserts, and leaves room
for a future feature in which such formatted names can be parsed and
searched for iteratively

Add a simple test to verify that providing a colon separated name
results in an error indicating an invalid lookup.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Todd Short <todd.short@me.com>
(Merged from openssl#23110)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t8m t8m t8m approved these changes

@tmshort tmshort tmshort approved these changes

@hlandau hlandau Awaiting requested review from hlandau

@jamuir jamuir Awaiting requested review from jamuir

@mattcaswell mattcaswell Awaiting requested review from mattcaswell

Assignees

@nhorman nhorman

Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 branch: 3.2 Merge to openssl-3.2 severity: fips change The pull request changes FIPS provider sources tests: present The PR has suitable tests present triaged: bug The issue/pr is/fixes a bug
Projects
Project Board
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nhorman @openssl-machine @t8m @tmshort