Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AMBARI-24776. Allow the agent's SSL certificate data to be accessible by heartbeat handlers (amagyar) #2457

Closed
wants to merge 1 commit into from

Conversation

zeroflag
Copy link
Contributor

What changes were proposed in this pull request?

The agents certificate should be accessible from the heartbeat controller. The public key may be used to encrypt data for the agent.

We get the certificate from the servlet request in a HandshakeInterceptor and store it in a map which is accessible via headerAccessor.getSessionAttributes().

How was this patch tested?

  • enabled two way ssl
  • checked the value of headerAccessor.getSessionAttributes().get(X509_CERTIFICATE_ATTRIBUTE) from the heartbeatcontroller

@zeroflag zeroflag self-assigned this Oct 15, 2018
@zeroflag zeroflag requested review from a user, rlevas and smolnar82 October 15, 2018 09:11
@asfgit
Copy link

asfgit commented Oct 15, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/4270/
Test FAILed.
Test FAILured.

@zeroflag
Copy link
Contributor Author

retest this please

@asfgit
Copy link

asfgit commented Oct 15, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/4272/
Test PASSed.

@rlevas
Copy link
Contributor

rlevas commented Oct 15, 2018

@mpapirkovskyy , Can you review this? I think you have been working with this interface for a while.

@mpapirkovskyy
Copy link
Contributor

I have few design related discussion points:

  1. Why not to allow agent encrypt shared secret itself before storing? data transfer is already encrypted and we will need agent logic anyway. But this is matter of taste.
  2. Transient shared secret will add a lot of complexities in server restart process:
    • we should remember that configs are sent to agent when modified, separately from execution commands
    • server send execution commands without configs, configs are propagated from cache on agent
    • server restart will reset and update shared secret, so it won't fit for configs sent earlier anymore
    • this means that we will need additional logic to determine configs with passwords and resend them, as configs don't change from server point of view.

Do we really need transient shared secret? If yes we should consider another key regeneration strategy which will allow to decrypt older configs also.
@zeroflag @rlevas

@rlevas
Copy link
Contributor

rlevas commented Oct 16, 2018

@mpapirkovskyy , Thanks for taking a look.

1. Why not to allow agent encrypt shared secret itself before storing? data transfer is already encrypted and we will need agent logic anyway. But this is matter of taste.

This is a great idea. Initially, I was under the impression that the agent wrote the command.json file out to disk before looking at it. However if the agent processes the file (which I think is a newer concept), then going this route would be ideal for many reasons.

2. Transient shared secret will add a lot of complexities in server restart process:
* we should remember that configs are sent to agent when modified, separately from execution commands
* server send execution commands without configs, configs are propagated from cache on agent
* server restart will reset and update shared secret, so it won't fit for configs sent earlier anymore
* this means that we will need additional logic to determine configs with passwords and resend them, as configs don't change from server point of view.

This is a newer concept that I was not aware of. Thanks for pointing it out.

@zeroflag , with this information in mind, let's rethink the architecture.

Copy link
Contributor

@rlevas rlevas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let rethink this...

@adoroszlai adoroszlai changed the title AMBARI-24776. Allow the agent's SSL certificate data to be accessible by heartbeat handers (amagyar) AMBARI-24776. Allow the agent's SSL certificate data to be accessible by heartbeat handlers (amagyar) Oct 22, 2018
@zeroflag zeroflag closed this Nov 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants