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. |
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. |
vancluever
force-pushed
the
vancluever-profile-default-promote
branch
from
90250b7 to
9ff0957
Compare
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).
vancluever
force-pushed
the
vancluever-profile-default-promote
branch
from
9ff0957 to
1a4fb49
Compare
vancluever
changed the title
|
@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 <[email protected]>
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).