ca: allow the promotion of first domain/IP to CN in profile#491
Conversation
|
PS: let me know if any additional tests are needed too (from what I saw, the profile functionality is not really tested so I didn't add anything in that regard). Main rationale/motive for this PR: after I upgraded https://github.com/vancluever/terraform-provider-acme to test using the latest pebble, all of my existing tests broke, since they don't supply a profile. I'm also adding profile support, so having these additional options help me emulate Let's Encrypt specifically a bit better. |
aarongable
left a comment
There was a problem hiding this comment.
Adding functionality to control CN promotion looks good to me.
I'd prefer not to add default profile functionality, however. The purpose of Pebble is not to emulate other CAs, it is to provide chaotic behavior to stress-test clients and make sure they can handle the full gamut of possible CA behaviors. The logic that a CA uses to select a profile for requests that don't specify one can be arbitrarily complex -- even Let's Encrypt's current "just pick the default" behavior is in the process of changing. So I don't think that supporting this behavior is in the interest of Pebble's goals.
|
@aarongable I guess I can take that out and I'll just set up different instances on my side with different profiles (I already do that for EAB tests). I'll adjust and update when I have time tomorrow! PS: Do you have details on this?
Does this mean the classic profile is going away or will profile by required in the future? |
|
We're in the process of launching IP-address certs, and if the new-order request contains an IP identifier, it will be defaulted to the shortlived profile rather than defaulted to the classic profile. So the defaulting logic is going to be more complex than just "pick the one marked as the default". Clients shouldn't assume that there's one default that applies across all requests, which is why Pebble chooses a profile at random. |
|
@aarongable updates complete; let me know if you need anything else! |
| case len(ips) > 0: | ||
| cn = ips[0].String() |
There was a problem hiding this comment.
Off the top of my head I'm not 100% confident that using net.IP.String() will always be compliant with BRs Section 7.1.4.3, but this is Pebble so I'm not super concerned about it.
There was a problem hiding this comment.
I think we should just not put an IP in a CN. Strategically, I want people to not rely on CNs, so I think it makes sense to omit them in Pebble
There was a problem hiding this comment.
I mean, I agree, which is why Boulder will never put an IP in the CN. But the purpose of Pebble is to behave differently from Boulder, to ensure that ACME clients don't over-index on Boulder's specific behavior.
Pebble drops the CN since letsencrypt/pebble#420 Support may be added back in letsencrypt/pebble#491 Signed-off-by: Richard Wall <richard.wall@cyberark.com>
|
In #491 (comment) @aarongable wrote:
I was surprised to read here that Pebble chooses a random profile. The sample configuration gave me a false impression that the profile named "default" would be used for clients which do not specify a profile. pebble/test/config/pebble-config.json Lines 16 to 25 in 39dbb64 Lines 106 to 114 in 39dbb64 |
Pebble drops the CN since letsencrypt/pebble#420 Support may be added back in letsencrypt/pebble#491 Signed-off-by: Richard Wall <richard.wall@cyberark.com> (cherry picked from commit 487a54f)
Pebble drops the CN since letsencrypt/pebble#420 Support may be added back in letsencrypt/pebble#491 Signed-off-by: Richard Wall <richard.wall@cyberark.com> (cherry picked from commit 487a54f)
Pebble drops the CN since letsencrypt/pebble#420 Support may be added back in letsencrypt/pebble#491 Signed-off-by: Richard Wall <richard.wall@cyberark.com> (cherry picked from commit 487a54f)
Pebble drops the CN since letsencrypt/pebble#420 Support may be added back in letsencrypt/pebble#491 Signed-off-by: Richard Wall <richard.wall@cyberark.com> (cherry picked from commit 487a54f)
This adds the ability to flag a profile to promote the first domain/IP to the common name. This was previously removed when promotion was deprecated, but is still allowed in the Let's Encrypt "classic" profile, so this helps mock this behavior (and also allows it to be mocked in CAs that still do the same).
|
Thanks for the merge y'all! |
Under the real CA, CNs can be missing. Add a new method to retrieve a certificate name obtained from certain sources. Pebble does no longer issue certificates with common name to encourage awareness of this possible behavior. ref. letsencrypt/pebble#491 (review)
Under the real CA, CNs can be missing. Add a new method to retrieve a certificate name obtained from certain sources. Pebble does no longer issue certificates with common name to encourage awareness of this possible behavior. ref. letsencrypt/pebble#491 (review)
This adds the ability to flag a profile to promote the first domain/IP to the common name. This was previously removed when promotion was deprecated, but is still allowed in the Let's Encrypt "classic" profile, so this helps mock this behavior (and also allows it to be mocked in CAs that still do the same).