Skip to content

Conversation

@slontis
Copy link
Member

@slontis slontis commented May 30, 2022

Add variants of d2i_PUBKEY_fp and d2i_PUBKEY_bio that pass a library context.

This also fixes some incorrect i2d documentation.

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

@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label May 30, 2022
@t-j-h
Copy link
Member

t-j-h commented May 30, 2022

How does the test actually test that the libctx is used for anything - i.e. the purpose of the variant of the function?

@slontis
Copy link
Member Author

slontis commented May 30, 2022

How does the test actually test that the libctx is used for anything - i.e. the purpose of the variant of the function?

The test setup loads the NULL provider into the default library context. If you dont use the mainctx it will fall over.

@t8m t8m added triaged: feature The issue/pr requests/adds a feature approval: review pending This pull request needs review by a committer labels May 30, 2022
@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/otc but the last update was 30 days ago

@t8m t8m closed this Jul 7, 2022
@t8m t8m reopened this Jul 7, 2022
Copy link
Member

@t8m t8m left a comment

Choose a reason for hiding this comment

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

Approved assuming CI passes.

@github-actions github-actions bot removed the severity: fips change The pull request changes FIPS provider sources label Jul 7, 2022
@slontis slontis force-pushed the d2i_Pubkey_fp_ex branch from 1a5af7e to 66363e7 Compare July 8, 2022 01:37
@slontis
Copy link
Member Author

slontis commented Jul 8, 2022

Windows was failing due to an applink error. Only change was to test/build.info

@slontis
Copy link
Member Author

slontis commented Jul 8, 2022

@paulidale have you seen this error before in the non-caching?

@t8m
Copy link
Member

t8m commented Jul 8, 2022

@paulidale have you seen this error before in the non-caching?

This is #18631 - unfortunately we do not know what is causing the failure.

@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/committers but the last update was 30 days ago

@t8m t8m 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 Aug 16, 2022
@t8m
Copy link
Member

t8m commented Aug 16, 2022

Unfortunately there are conflicts now, can you please rebase @slontis ?

@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Sep 13, 2022
@hlandau
Copy link
Member

hlandau commented Sep 13, 2022

Still OK

These functions pass a library content and prop query.
The i2d documentation related to these functions has been corrected since the bio and fp functions always return 0 or 1.
@slontis
Copy link
Member Author

slontis commented Oct 27, 2022

Rebased to fix merge conflicts..

@t8m t8m added approval: review pending This pull request needs review by a committer and removed approval: done This pull request has the required number of approvals labels Oct 27, 2022
@t8m t8m requested a review from hlandau October 27, 2022 09:55
@hlandau hlandau 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 Oct 27, 2022
@openssl-machine openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Oct 28, 2022
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@t8m
Copy link
Member

t8m commented Nov 2, 2022

Merged to master branch. However the doc fix commit would cherry-pick cleanly to 3.0/3.1 branches. If @hlandau and @slontis agrees I'll cherry-pick it there.

Keeping this open for now.

@t8m t8m added triaged: documentation The issue/pr deals with documentation (errors) branch: 3.0 Applies to openssl-3.0 branch branch: 3.1 Applies to openssl-3.1 (EOL) labels Nov 2, 2022
openssl-machine pushed a commit that referenced this pull request Nov 2, 2022
These functions pass a library content and prop query.
The i2d documentation related to these functions has been corrected since the bio and fp functions always return 0 or 1.

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #18427)
openssl-machine pushed a commit that referenced this pull request Nov 2, 2022
i2d_XXX_bio and i2d_XXX_fp return either 0 or 1.
Other i2d_XXX functions return the number of bytes or negative on error.

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #18427)
@hlandau
Copy link
Member

hlandau commented Nov 7, 2022

@t8m re: doc fix: Sounds good, go ahead.

@t8m
Copy link
Member

t8m commented Nov 7, 2022

Cherry-picked the doc fix commit to 3.0 and 3.1 branches. Closing.

@t8m t8m closed this Nov 7, 2022
openssl-machine pushed a commit that referenced this pull request Nov 7, 2022
i2d_XXX_bio and i2d_XXX_fp return either 0 or 1.
Other i2d_XXX functions return the number of bytes or negative on error.

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #18427)

(cherry picked from commit 943051d)
openssl-machine pushed a commit that referenced this pull request Nov 7, 2022
i2d_XXX_bio and i2d_XXX_fp return either 0 or 1.
Other i2d_XXX functions return the number of bytes or negative on error.

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #18427)

(cherry picked from commit 943051d)
beldmit pushed a commit to beldmit/openssl that referenced this pull request Dec 26, 2022
These functions pass a library content and prop query.
The i2d documentation related to these functions has been corrected since the bio and fp functions always return 0 or 1.

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#18427)
beldmit pushed a commit to beldmit/openssl that referenced this pull request Dec 26, 2022
i2d_XXX_bio and i2d_XXX_fp return either 0 or 1.
Other i2d_XXX functions return the number of bytes or negative on error.

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#18427)
dengert added a commit to dengert/OpenSC that referenced this pull request Feb 21, 2023
This is in addition to "Introduce use of custom ossl libctx with OpenSSL >= 3.0"
OpenSC#2712

pkcs11-tool uses some functions found in libopensc, but does not create a sc_context
like other OpenSC tools as the pkcs11 module can be any pkscs11 module.

There is one OpenSSL function "d2i_PUBKEY_bio" that does not have an equivalent
"d2i_PUBKEY_ex_bio" in 3.0.8, It is listed in OpenSSL master

See: openssl/openssl#18427
dengert added a commit to dengert/OpenSC that referenced this pull request Feb 21, 2023
This is in addition to "Introduce use of custom ossl libctx with OpenSSL >= 3.0"
OpenSC#2712

pkcs11-tool uses some functions found in libopensc, but does not create a sc_context
like other OpenSC tools as the pkcs11 module can be any pkscs11 module.

There is one OpenSSL function "d2i_PUBKEY_bio" that does not have an equivalent
"d2i_PUBKEY_ex_bio" in 3.0.8.  It is listed in OpenSSL master.
See: openssl/openssl#18427

 On branch ossl_lib_ctx-pkcs11-tool
 Changes to be committed:
	modified:   src/tools/pkcs11-tool.c
dengert added a commit to dengert/OpenSC that referenced this pull request Mar 8, 2023
This is in addition to "Introduce use of custom ossl libctx with OpenSSL >= 3.0"
OpenSC#2712

pkcs11-tool uses some functions found in libopensc, but does not create a sc_context
like other OpenSC tools as the pkcs11 module can be any pkscs11 module.

There is one OpenSSL function "d2i_PUBKEY_bio" that does not have an equivalent
"d2i_PUBKEY_ex_bio" in 3.0.8.  It is listed in OpenSSL master.
See: openssl/openssl#18427

 On branch ossl_lib_ctx-pkcs11-tool
 Changes to be committed:
	modified:   src/tools/pkcs11-tool.c
Jakuje pushed a commit to OpenSC/OpenSC that referenced this pull request Mar 20, 2023
This is in addition to "Introduce use of custom ossl libctx with OpenSSL >= 3.0"
#2712

pkcs11-tool uses some functions found in libopensc, but does not create a sc_context
like other OpenSC tools as the pkcs11 module can be any pkscs11 module.

There is one OpenSSL function "d2i_PUBKEY_bio" that does not have an equivalent
"d2i_PUBKEY_ex_bio" in 3.0.8.  It is listed in OpenSSL master.
See: openssl/openssl#18427

 On branch ossl_lib_ctx-pkcs11-tool
 Changes to be committed:
	modified:   src/tools/pkcs11-tool.c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch branch: 3.0 Applies to openssl-3.0 branch branch: 3.1 Applies to openssl-3.1 (EOL) severity: fips change The pull request changes FIPS provider sources triaged: documentation The issue/pr deals with documentation (errors) triaged: feature The issue/pr requests/adds a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants