Skip to content

Conversation

@inteon
Copy link
Contributor

@inteon inteon commented Jun 21, 2023

In TPP, the ObjectDN value is built using either the FriendlyName or the CommonName (if no FriendlyName is provided).
During processing Location, VCert logic incorrectly assumes that the object DN is always based on the CommonName.

…contain this value instead of the CommonName

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
@luispresuelVenafi
Copy link
Contributor

Hi there @inteon ,

This is not a bug. This behavior was desired as we wanted to enforce the best practice to make the ObjectName (object DN) be the same as the CommonName. That said, this is a enhancement we want to discuss within the team. Could you elaborate why you had the need of these changes?

@luispresuelVenafi luispresuelVenafi changed the title BUGFIX: in case a FriendlyName value is provided, the object DN will contain this value instead of the CommonName Enhancement: in case a FriendlyName value is provided, the object DN will contain this value instead of the CommonName Jun 21, 2023
@luispresuelVenafi luispresuelVenafi added the enhancement New feature or request label Jun 21, 2023
@inteon
Copy link
Contributor Author

inteon commented Jun 22, 2023

Hi there @inteon ,

This is not a bug. This behavior was desired as we wanted to enforce the best practice to make the ObjectName (object DN) be the same as the CommonName. That said, this is a enhancement we want to discuss within the team. Could you elaborate why you had the need of these changes?

We want to provide the option to use a FriendlyName that differs from the CommonName, so users don't have certificates that collide due to them having the same CommonName (eg. loadbalancer1 could have certificate-lb1 as Friendlyname but share a CommonName with certificate-lb2).
For now, we are not using the Location option yet. However, getCertificateDN is also used in #295. This will help users of the Golang API that are using FriendlyName + the Location option, which will yield incorrect results if their FriendlyName does not match their CommonName (without returning any kind of error).

@luispresuelVenafi
Copy link
Contributor

@inteon you are right, this is a bug. The description is misleading and made me think you were requesting this feature to be enabled. I found that the feature as described in current title "in case a FriendlyName value is provided, the object DN will contain this value instead of the CommonName" itself already is, that is, you can make a request with the FriendlyName and it will override the CommonName writting to TPP's object name for the Certificate; but you are right, this hasn't taken into account the Friendly when you process the Location option, which is a bug. I'll update the description and title accordingly

@luispresuelVenafi luispresuelVenafi added bug Something isn't working and removed enhancement New feature or request labels Jun 22, 2023
@luispresuelVenafi luispresuelVenafi changed the title Enhancement: in case a FriendlyName value is provided, the object DN will contain this value instead of the CommonName Bugfix: updates processing Location to handle FriendlyName Jun 22, 2023
@inteon
Copy link
Contributor Author

inteon commented Jun 22, 2023

@inteon you are right, this is a bug. The description is misleading and made me think you were requesting this feature to be enabled. I found that the feature as described in current title "in case a FriendlyName value is provided, the object DN will contain this value instead of the CommonName" itself already is, that is, you can make a request with the FriendlyName and it will override the CommonName writting to TPP's object name for the Certificate; but you are right, this hasn't taken into account the Friendly when you process the Location option, which is a bug. I'll update the description and title accordingly

Sorry for it not being clear right away. Thanks for updating the PR so it is more clear what the issue is.

@luispresuelVenafi luispresuelVenafi merged commit 9a5f83f into Venafi:master Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants