Include challenge password attribute if required by EST server

[mobe1]

The changes introduced come after the issue/feature requrest #30 has been opened.
They allow us to enroll a CSR that includes the TLS-unique value as recommended by the RFC 7030

• Because each http client instantiation results in a new TLS-unique, one way of including it would be to make EST requests from the same http client.
• Because the standard crypto/x509 Go package does not handle the challenge password attribute (OID) the way an EST/CA server expects it, the CSR creation had to be wrapped.

[toddgaunt-gs]

Thanks for opening this PR, I'll forward this to my team for review.

[toddgaunt-gs]

[ ] Need to resolve conflicts after upgrading to Go 1.22.1...

[mobe1]

Thought the comment wasn't for me until I noticed the repo did get upgraded with a different version of go...
The conflicts should be resolved.

[toddgaunt-gs]

Ah it wasn't actually! Just a reminder for when I had time to get around to this again, but thank you for addressing the changes needed!

[DDvO]

Looks like work on this PR has stalled - I wonder when is it going to be resumed?

[DDvO]

BTW, copying to csr.go large chunks of code from the x509 package does not look right -
there should be a lean way of adding and reading the challengePassword attribute of a PKCS#10 CSR.

[toddgaunt-gs]

Hi DDvO, unfortunately my team hasn't been able to spare the cycles to work on this repository for a while, so while I hesitate to recommend it you may want to fork and patch a clone of this repository if you need this functionality right away.

Edit: I'll try to review this PR as soon as possible to avoid such scenario

[toddgaunt-gs]

BTW, copying to csr.go large chunks of code from the x509 package does not look right - there should be a lean way of adding and reading the challengePassword attribute of a PKCS#10 CSR.

I agree with this, I'd prefer to see this functionality without including this dependency if possible.

[toddgaunt-gs]

Sorry for such a long delay for reviewing this PR, I didn't prioritize it and it has been far to long so I apologize for that. Thank you for your contribution, but I don't think I can hit the merge button on this PR as it is now into the master, but I am happy to support it as a separate branch to support this use-case for now.

Some critiques that are blocking this:

• I'd prefer the smaller style changes, such as using io over ioutil and changing the code to call EqualFold be done in a separate PR as these changes are unrelated to the challenge password.
• As @DDvO mentions above, avoiding copying csr.go would be preferred
• The samples and resources directory added I'll have to give some more thought to. While I like seeing a way to try out the changes, I think I'd prefer this just as an automated test of the feature instead. I think adding the case to est_test.go would be the way to go.

[mobe1]

Hi guys,

Thank you @DDvO for closing the previous PR, that's on me.

BTW, copying to csr.go large chunks of code from the x509 package does not look right -
there should be a lean way of adding and reading the challengePassword attribute of a PKCS#10 CSR.

I too agree, I actually borrowed that code snippet from another repository, credit goes to micromdm.

Meanwhile, unfortunately Go has still not taken action to address it, there is an open issue for it already.

[mobe1]

@toddgaunt-gs
Nah acutally, you're right. I shouldn't have mixed code refactoring and the feature itself as it doesn't help telling what's necessary to make the feature available...

On a personal note: I thought I had to change the method signature but after a quick review, I start to think it can be done while keeping your methods signatures intact.

That being said,

• I'll revert those unrelated changes
• then I'll bring the method signature back
• finally, i'll see if there is a better way of adding that challengePassword attribute without copying the csr.go (spoiler alert, i'm not very confident on this one)

[DDvO]

Hi guys,

thank you for swiftly responding on this.

BTW, copying to csr.go large chunks of code from the x509 package does not look right -
there should be a lean way of adding and reading the challengePassword attribute of a PKCS#10 CSR.

I too agree, I actually borrowed that code snippet from another repository, credit goes to micromdm.

Looks like there is a glitch in the link you included and instead you meant
https://github.com/micromdm/scep/blob/main/cryptoutil/x509util/x509util.go

Meanwhile, unfortunately Go has still not taken action to address it, there is an open issue for it already.

I've meanwhile had a closer look.
Oh my goodness. The crypto/x509 package screwed up the implementation of the PKCS#10 CSR Attribute type.
This bug is known since long and unfortunately is hard to work around also due to its very restrictive API.
I just commented there accordingly.