Skip to content

Comments

X509: Flag modified when X509_req_info_st updated - follow-up for 3.0+#19090

Closed
DDvO wants to merge 3 commits intoopenssl:masterfrom
siemens:fix_x509_req_v30
Closed

X509: Flag modified when X509_req_info_st updated - follow-up for 3.0+#19090
DDvO wants to merge 3 commits intoopenssl:masterfrom
siemens:fix_x509_req_v30

Conversation

@DDvO
Copy link
Contributor

@DDvO DDvO commented Aug 29, 2022

This is a variant of #18879 for 3.0 and master, rebasing that PR from 1.1.1.

On this occasion, I added a further commit that to add some NULL parameter checks and improve coding style in crypto/x509/{x509_req,x_all}.c.

@DDvO DDvO added branch: master Applies to master branch approval: review pending This pull request needs review by a committer approval: otc review pending triaged: bug The issue/pr is/fixes a bug triaged: refactor The issue/pr requests/implements refactoring branch: 3.0 Applies to openssl-3.0 branch labels Aug 29, 2022
@DDvO DDvO requested a review from t8m August 29, 2022 12:03
@DDvO DDvO mentioned this pull request Aug 29, 2022
Closed
@t8m
Copy link
Member

t8m commented Aug 29, 2022

This would not be eligible for 3.0 IMO.

@t8m
Copy link
Member

t8m commented Aug 29, 2022

Or maybe it would, although I am not a big fan of such cleanup kind of changes on older branches.

@t8m t8m added triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly and removed triaged: refactor The issue/pr requests/implements refactoring labels Aug 29, 2022
@DDvO
Copy link
Contributor Author

DDvO commented Sep 8, 2022

Asking @openssl/committers for 2nd approval.

@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 Sep 9, 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 Sep 10, 2022
@openssl-machine
Copy link
Collaborator

openssl-machine commented Sep 10, 2022

This pull request is ready to merge

@DDvO
Copy link
Contributor Author

DDvO commented Sep 10, 2022

Merged to master and 3.0 - thanks @t8m and @hlandau for the approvals.

openssl-machine pushed a commit that referenced this pull request Sep 10, 2022
@gibeom-gwon @DDvO
X509 x509_req.c: Set 'modified' flag when X509_req_info_st member dat…
39d356e 
…a updated

We need to reencode X509_req_info_st if member data updated.

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from #19090)
openssl-machine pushed a commit that referenced this pull request Sep 10, 2022
@gibeom-gwon @DDvO
X509 x_all.c: Set 'modified' flag when ASN1_item_sign{,_ctx} call was…
9249a34 
… successful

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from #19090)
openssl-machine pushed a commit that referenced this pull request Sep 10, 2022
@DDvO
crypto/x509/{x509_req,x_all}.c: add some NULL parameter checks, impro…
8e39049 
…ve coding style

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Hugo Landau <[email protected]>
(Merged from #19090)
openssl-machine pushed a commit that referenced this pull request Sep 10, 2022
@gibeom-gwon @DDvO
X509 x509_req.c: Set 'modified' flag when X509_req_info_st member dat…
fe342fa 
…a updated

We need to reencode X509_req_info_st if member data updated.

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from #19090)

(cherry picked from commit 39d356e)
openssl-machine pushed a commit that referenced this pull request Sep 10, 2022
@gibeom-gwon @DDvO
X509 x_all.c: Set 'modified' flag when ASN1_item_sign{,_ctx} call was…
9bc8fe3 
… successful

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from #19090)

(cherry picked from commit 9249a34)
openssl-machine pushed a commit that referenced this pull request Sep 10, 2022
@DDvO
crypto/x509/{x509_req,x_all}.c: add some NULL parameter checks, impro…
684ba5c 
…ve coding style

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

(cherry picked from commit 8e39049)
@DDvO DDvO closed this Sep 10, 2022
sftcd pushed a commit to sftcd/openssl that referenced this pull request Sep 24, 2022
@gibeom-gwon @sftcd
X509 x509_req.c: Set 'modified' flag when X509_req_info_st member dat…
ad080f8 
…a updated

We need to reencode X509_req_info_st if member data updated.

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from openssl#19090)
sftcd pushed a commit to sftcd/openssl that referenced this pull request Sep 24, 2022
@gibeom-gwon @sftcd
X509 x_all.c: Set 'modified' flag when ASN1_item_sign{,_ctx} call was…
cc62e05 
… successful

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from openssl#19090)
sftcd pushed a commit to sftcd/openssl that referenced this pull request Sep 24, 2022
@DDvO @sftcd
crypto/x509/{x509_req,x_all}.c: add some NULL parameter checks, impro…
867520a 
…ve coding style

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Hugo Landau <[email protected]>
(Merged from openssl#19090)
beldmit pushed a commit to beldmit/openssl that referenced this pull request Dec 26, 2022
@gibeom-gwon @beldmit
X509 x509_req.c: Set 'modified' flag when X509_req_info_st member dat…
abe6b45 
…a updated

We need to reencode X509_req_info_st if member data updated.

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from openssl#19090)
beldmit pushed a commit to beldmit/openssl that referenced this pull request Dec 26, 2022
@gibeom-gwon @beldmit
X509 x_all.c: Set 'modified' flag when ASN1_item_sign{,_ctx} call was…
51a9d16 
… successful

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from openssl#19090)
beldmit pushed a commit to beldmit/openssl that referenced this pull request Dec 26, 2022
@DDvO @beldmit
crypto/x509/{x509_req,x_all}.c: add some NULL parameter checks, impro…
cd965e2 
…ve coding style

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Hugo Landau <[email protected]>
(Merged from openssl#19090)
kafei-cy added a commit to kafei-cy/Tongsuo that referenced this pull request Jan 11, 2026
@kafei-cy
X509: Flag modified when X509_req_info_st updated - follow-up for 3.0+
88db4ae 
This is a variant of openssl/openssl#18879 for 3.0 and master, rebasing that PR from 1.1.1.

On this occasion, I added a further commit that to add some NULL parameter checks and improve coding style in crypto/x509/{x509_req,x_all}.c

Include 3 commits:
    X509 x509_req.c: Set 'modified' flag when X509_req_info_st member data updated
    X509 x_all.c: Set 'modified' flag when ASN1_item_sign{,_ctx} call was successful
    crypto/x509/{x509_req,x_all}.c: add some NULL parameter checks, improve coding style

(Merged from openssl/openssl#19090)
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

+1 more reviewer

@hlandau hlandau hlandau approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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 triaged: bug The issue/pr is/fixes a bug triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@DDvO @t8m @openssl-machine @hlandau @gibeom-gwon