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

apps: silent warning when loading CSR files with vfyopt option #20799

Closed
wants to merge 1 commit into from
Closed

apps: silent warning when loading CSR files with vfyopt option #20799

wants to merge 1 commit into from

Conversation

uudiin
Copy link
Contributor

@uudiin uudiin commented Apr 21, 2023 •
edited

When verifying or signing a CSR file with the -vfyopt option, a warning message similar to the following will appear:

Warning: CSR self-signature does not match the contents

This happens especially when the SM2 algorithm is used and the distid parameter is added. Pass the vfyopts parameter to the do_X509_REQ_verify() function to eliminate the warning message.

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

@t8m t8m added branch: master Merge to master branch approval: review pending This pull request needs review by a committer 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 tests: exempted The PR is exempt from requirements for testing labels Apr 21, 2023
@t8m
Copy link
Member

t8m commented Apr 21, 2023

Would it be possible to add a test?

@uudiin
Copy link
Contributor Author

uudiin commented Apr 27, 2023

Would it be possible to add a test?

Hi @t8m , I manually typed in the command to do the test. When make test, it need to add the V=1 parameter to display this warning. The output is as follows:

../../util/wrap.pl ../../apps/openssl req -config ../../test/test.cnf -new -key ../../test/certs/sm2.key -sigopt 'distid:1234567812345678' -out testreq-sm2.pem -sm3 => 0
    ok 1 - Generating SM2 certificate request
Warning: CSR self-signature does not match the contents
Certificate request self-signature verify OK

I need some help, I don't know how to add tests to detect this warning in make test.

@uudiin
apps: silent warning when loading CSR files with vfyopt option
c688d55 
When verifying or signing a CSR file with the -vfyopt option,
a warning message similar to the following will appear:

  Warning: CSR self-signature does not match the contents

This happens especially when the SM2 algorithm is used and the
distid parameter is added. Pass the vfyopts parameter to the
do_X509_REQ_verify() function to eliminate the warning message.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
@t8m
Copy link
Member

t8m commented Apr 28, 2023

Please do not rebase the PR unless there is a conflict.

@paulidale paulidale 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 May 2, 2023
@openssl-machine openssl-machine removed the approval: done This pull request has the required number of approvals label May 3, 2023
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@openssl-machine openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label May 3, 2023
@t8m t8m removed branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 labels May 3, 2023
openssl-machine pushed a commit that referenced this pull request May 3, 2023
@uudiin @t8m
apps: silent warning when loading CSR files with vfyopt option
a75f707 
When verifying or signing a CSR file with the -vfyopt option,
a warning message similar to the following will appear:

  Warning: CSR self-signature does not match the contents

This happens especially when the SM2 algorithm is used and the
distid parameter is added. Pass the vfyopts parameter to the
do_X509_REQ_verify() function to eliminate the warning message.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #20799)
@t8m
Copy link
Member

t8m commented May 3, 2023

Merged to master branch. Thank you for your contribution.

Unfortunately this fix does not apply to 3.1 and 3.0 branches cleanly. Could you please submit a backported fix against the 3.1 branch if you're interested in having this fixed on 3.1 and 3.0 branches?

@t8m t8m closed this May 3, 2023
@uudiin uudiin deleted the nowarn branch May 4, 2023 02:36
@uudiin
Copy link
Contributor Author

uudiin commented May 4, 2023

Merged to master branch. Thank you for your contribution.

Unfortunately this fix does not apply to 3.1 and 3.0 branches cleanly. Could you please submit a backported fix against the 3.1 branch if you're interested in having this fixed on 3.1 and 3.0 branches?

In branches 3.0 and 3.1, only the cmp subcommand calls the load_csr_autofmt() function, which does not yet support the vfyopts feature, so this patch cannot and is not necessary to be backported to the 3.0 and 3.1 branches.

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

@paulidale paulidale paulidale approved these changes

Assignees
No one assigned
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch tests: exempted The PR is exempt from requirements for testing triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@uudiin @t8m @openssl-machine @paulidale