[
https://issues.apache.org/jira/browse/HDDS-580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16667618#comment-16667618
]
Ajay Kumar commented on HDDS-580:
---------------------------------
[~anu] thanks for detailed review.
{quote}I feel that the changes in the Security config are not needed.
Let us keep a Key Store Path – and secure it only to "hdfs" user.
Then we can support a well-known path under it(which is what the current patch
seems to do)
So if you want to read the public and private keys for SCM.
We can get to ../keystore/scm/keys. The scm/keys is a storage location we know
from the convention. The user needs to only specify where the key material is
stored.
I think if we do that, we can avoid passing component to the security config,
we can handle all of this logic in SecurityUtils.
In other words, I think this logic is better suited for
SecurityUtils.java.{quote}
Adding keystore to this path is good idea. Added it in new patch. However
looking at current use and API's of SecurityConfig it is difficult to move
current and component part to util class. I see following downsides:
* Right now for a given component (SCM, OM, DN etc) SecurityConfig is an
excellent wrapper for all security related information including key path.
API's like getCurrentKeyLocation, getPrivateKeyFilePath, getPublicKeyFilePath
returns correct path to user without having user to construct any part of it.
* Moving part of key path (component, current) will result in path
construction logic being split b/w util and config class. It will be ideal to
keep it in single wrapper.
* Util class is stateless so it is not well suited to maintain logic for
"current" & "previous" keys.
Let us know if you still think otherwise.
{quote}StorageContainerManager.java: Line 498:
"SCM cannot be started in secure mode", this message will be misleading once we
have HDDS-4 branch merged.{quote}
Good catch. Updated the error message.
{quote}StorageContainerManager.java: Line 514:
if (SecurityUtils.isOzoneSecurityEnabled(conf)) { // Bootstrap security
components as well. bootstrapSecurity(conf); }
We initialize the Storage Directories for the SCM after we
create an store public and private keys. (scmInit
Should we do key generation after the basic storage is initialized.{quote}
Make sense, done!
{quote}Does it make sense to map the default SCM keys to be mapped to this
directory path?{quote}
Sorry, this suggestion is not very clear. In latest patch default SCM keys are
at following path "HDDS_METADATA_DIR_NAME/scm/keystore/keys/current/".
ScmStorage uses same root.
{quote}Eventually, we will need to generate the Certificate, and that needs the
Cluster ID and SCM ID. So it might make sense to make this call after the SCM
identity is generated.{quote}
agree, done.
{quote}Why do we have a command line called INIT_SECURITY since it will check
if the secure config is setup? if we always need config then INIT should be
enough? I am not sure I understand why we need two different ways to do
this.{quote}
Idea is to allow admins to do security bootstrap separately in an existing
cluster. Having it part of initial INIT will allow security bootstrap in new
clusters where security is enabled from start. I see with current logic we can
just work with INIT but having them separate decouples them which will be handy
in long term as both are different steps/tasks.
{quote}We might not be able to do this in this patch, but we should file a JIRA
to track this. We should write an Audit Entry to who created the Private Keys,
Who created the certificates etc. that is a security audit log of everything we
did in SCM init.{quote}
Done, [HDDS-750]
{quote}We don't authenticate the user who is running the scm --init. Should we
read the Keytab file and find out if the user is an admin group user?{quote}
Shall we handle it in separate jira?
{quote}We can lose the component from the Self-signedCertificate.java too. If
we isolate SecurityConfig.java from having this change, it will work
consistently. We might have to pass paths to SecurityConfig.java to get various
objects, but it is better than having to pass a string everywhere we use
that.{quote}
This will mean user will determine the path. Comparatively i think it's easy to
pass component to SecurityConfig and than have it as a wrapper for security
related configs, paths. Do you feel strongly about it?
{quote}Rotating keys and certificates need more work. Should we separate that
to a different JIRA and not be part of this one?{quote}
Created [HDDS-752] for this. Removed unused function from {{SecurityUtils}}.
Left {{HDDSKeyPEMHandler#rotateKeyPair}} which can be improved in new jira.
{quote}We seem to have removed some Kerberos related keys in this patch, was
that intentional?{quote}
Yes, configs removed from ozone-default.xml are not used anymore.
{quote}nit: unused keypair variable in OzoneManager.java{quote}
We will use this key pair in delegation tokens. [HDDS-8]
{quote}Should this function "isOzoneSecurityEnabled" be in
SecurityConfig.java?{quote}
IMO this looks a function for util class. SecurityConfig has its own state and
usage. Happy to move it if you still feel this should be added as static
function to {{SecurityConfig}}.
{quote}SecurityUtils.java#bootstrapKeyPair - This function we fail if the
keyPair already exists. I am fine with the failure, but we should define what
we need to do fix this.
Do you want the user to delete these key files?
if the key files already exist, should we not be able to use them instead of
failing? that way user can provide a key if he/she so desires. Not something we
need to support now, just a thought.{quote}
Idea is to allow bootstrap only once and than use
{{SecurityUtils#getKeyPairFromFile}} for subsequent usages. Updated javadoc for
function. Earlier patches were reading existing keyfile instead of throwing
exception.
{quote}nit: There are a couple of unused functions in SecurityUtils.java.{quote}
Removed it.
> Bootstrap OM/SCM with private/public key pair
> ---------------------------------------------
>
> Key: HDDS-580
> URL: https://issues.apache.org/jira/browse/HDDS-580
> Project: Hadoop Distributed Data Store
> Issue Type: Sub-task
> Reporter: Xiaoyu Yao
> Assignee: Ajay Kumar
> Priority: Major
> Attachments: HDDS-4-HDDS-580.00.patch, HDDS-580-HDDS-4.00.patch,
> HDDS-580-HDDS-4.01.patch, HDDS-580-HDDS-4.02.patch, HDDS-580-HDDS-4.03.patch,
> HDDS-580-HDDS-4.04.patch, HDDS-580-HDDS-4.05.patch, HDDS-580-HDDS-4.06.patch,
> HDDS-580-HDDS-4.07.patch, HDDS-580-HDDS-4.08.patch, HDDS-580-HDDS-4.09.patch
>
>
> We will need to add API that leverage the key generator from HDDS-100 to
> generate public/private key pair for OM/SCM, this will be called by the
> scm/om admin cli with "-init" cmd.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
