openssl icon indicating copy to clipboard operation
openssl copied to clipboard

CMP app: fix combination of -certout and -chainout with equal filename arg

Open DDvO opened this issue 9 months ago • 2 comments

When using the same filename argument with the cmp app options -certout and -chainout, the file so far contains just the chain - the new cert is lost because it happens to get overwritten. This is fixed here in a useful way without touching the API, namely by prepending the cert to its chain. On this occasion also the documentation is improved.

Checklist
  • [x] documentation is added or updated
  • [x] tests are added or updated

DDvO avatar Apr 25 '24 18:04 DDvO

Would it be hard to add a test?

t8m avatar Apr 26 '24 07:04 t8m

Would it be hard to add a test?

I had already started thinking about that, and meanwhile have added a test each on successfully producing and using the combined output file.

DDvO avatar Apr 26 '24 08:04 DDvO

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

openssl-machine avatar May 27 '24 00:05 openssl-machine

Ping @openssl/committers for 2nd review - it's not a big PR, just some 10-15 lines of new/changed code.

DDvO avatar Jun 17 '24 09:06 DDvO

This pull request is ready to merge

openssl-machine avatar Jun 18 '24 16:06 openssl-machine

Merged into master, and cherry-picked into openssl-3.3. The cherry-pick to openssl-3.2 was non-trivial, so another PR will have to be opened @DDvO.

tmshort avatar Jun 18 '24 17:06 tmshort

Merged into master, and cherry-picked into openssl-3.3. The cherry-pick to openssl-3.2 was non-trivial, so another PR will have to be opened @DDvO.

It turns out that this was a simple (whitespace-only) merge conflict, which may have been handled during pick-to-branch. Update: turns out that there was also a semantic issue in one of the test cases to be fixed.

Anyway, here is the separate backporting PR: #24696.

DDvO avatar Jun 21 '24 06:06 DDvO