Skip to content

Comments

Update CA.pl podpage, and script#11338

Closed
richsalz wants to merge 2 commits intoopenssl:masterfrom
richsalz:extra-ca-params
Closed

Update CA.pl podpage, and script#11338
richsalz wants to merge 2 commits intoopenssl:masterfrom
richsalz:extra-ca-params

Conversation

@richsalz
Copy link
Contributor

Merge the NOTES section into the relevant parts of the manpage.
Add the $EXTRA parameter in consistent places (the end) to call
commands. Document that multiple -extra-XXX might be needed.

This is needed for #11124

@richsalz richsalz mentioned this pull request Mar 17, 2020
Merge the NOTES section into the relevant parts of the manpage.
Add the $EXTRA parameter in consistent places (the end) to call
commands.  Document that multiple -extra-XXX might be needed.
apps/CA.pl.in Outdated
my @dirs = ( "${CATOP}", "${CATOP}/certs", "${CATOP}/crl",
"${CATOP}/newcerts", "${CATOP}/private" );
foreach my $d ( @dirs ) {
die "Directory $d exists" if -d $d;
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer just printing a warning and continuing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The manpage says it stops. I would be VERY VERY concerned if someone accidentally ran the -newca flag and clobbered their old tree.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, but the current code does not stop. And this will break at least Fedora/RHEL installations where the directories are pre-created and preinstalled by the rpm package. Could you make the script stop if it encounters pre-existing index.txt or serial files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixup commit pushed.

Warn if directories exist.
Exit if index.txt or serial exists.
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.

Good work!

@t8m t8m self-assigned this Mar 19, 2020
@t8m t8m added approval: review pending This pull request needs review by a committer branch: master Applies to master branch labels Mar 19, 2020
@richsalz richsalz mentioned this pull request Mar 20, 2020
@t8m
Copy link
Member

t8m commented Mar 21, 2020

ping for the second review

@beldmit beldmit 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 Mar 22, 2020
@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 Mar 23, 2020
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

beldmit pushed a commit that referenced this pull request Mar 23, 2020
Merge the NOTES section into the relevant parts of the manpage.
Add the $EXTRA parameter in consistent places (the end) to call
commands.  Document that multiple -extra-XXX might be needed.

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Dmitry Belyavskiy <[email protected]>
(Merged from #11338)
@beldmit
Copy link
Member

beldmit commented Mar 23, 2020

Merged. Thanks!

@beldmit beldmit closed this Mar 23, 2020
@richsalz richsalz deleted the extra-ca-params branch March 23, 2020 17:37
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants