sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Abraham Fine <...@abrahamfine.com>
Subject Re: Review Request 44984: [WIP] Sqoop2: Encrypt sensitive information in the repository
Date Wed, 23 Mar 2016 23:50:52 GMT


> On March 23, 2016, 8:27 p.m., Jarek Cecho wrote:
> > common/src/main/java/org/apache/sqoop/model/MMasterKey.java, line 36
> > <https://reviews.apache.org/r/44984/diff/5/?file=1312405#file1312405line36>
> >
> >     Do we want to be leaking the secret when serializing to String?

this is the same encrypted secret that is stored in the database (i'll rename some things
so it is clearer). so i dont think that it is a problem. i can change it if you feel strongly
about printing out the encrypted data.


> On March 23, 2016, 8:27 p.m., Jarek Cecho wrote:
> > core/src/main/java/org/apache/sqoop/repository/RepositoryEncryptionManager.java,
line 46
> > <https://reviews.apache.org/r/44984/diff/5/?file=1312410#file1312410line46>
> >
> >     I'm wondering - what is the value of introducing a new manager over merging
that functionality to RepositoryManager itself?

I was thinking about this. I think it may be better to rename RepositoryEncryptionManager
to MasterKeyManager (or something like that). It seemed like a reasonable separation of concerns
to keep it apart from RepositoryManager (due to all of its state).


> On March 23, 2016, 8:27 p.m., Jarek Cecho wrote:
> > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java,
lines 439-443
> > <https://reviews.apache.org/r/44984/diff/5/?file=1312415#file1312415line439>
> >
> >     I would probably recommend to use the same query to retrieve both encrypted/unecrypted
variant - it will be less headache to maintain all those queries.

this is not retrieving inputs, it is inserting inputs


> On March 23, 2016, 8:27 p.m., Jarek Cecho wrote:
> > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java,
line 2277
> > <https://reviews.apache.org/r/44984/diff/5/?file=1312415#file1312415line2277>
> >
> >     For data verification I would use normal exception rathern then assert. We're
using assert more to verify code rather then data.

forgot to make a change from the proof of concept phase. good catch


- Abraham


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44984/#review125114
-----------------------------------------------------------


On March 23, 2016, 11:49 p.m., Abraham Fine wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44984/
> -----------------------------------------------------------
> 
> (Updated March 23, 2016, 11:49 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2887
>     https://issues.apache.org/jira/browse/SQOOP-2887
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> proof of concept regarding sqoop repository encryption, exceptions are not handled properly
and there is plenty of cleanup to do. but you should get the basic idea
> 
> 
> Diffs
> -----
> 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DerbyProvider.java 839e5618e3cd86a1df24b53b04aae3721b56ed27

>   common/src/main/java/org/apache/sqoop/error/code/CommonRepositoryError.java 37eb04aaa30f24577ff755e361e6d0a9d47be37c

>   common/src/main/java/org/apache/sqoop/model/MMasterKey.java PRE-CREATION 
>   core/src/main/java/org/apache/sqoop/core/SqoopServer.java 80a7b88ac4704972f1de0ddaf21869feedba4aa8

>   core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 5b70f951c98dec7d20ff446f52418234fd98e454

>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java feab2ad8360b7d978b263dc0da1d2ff2578f0555

>   core/src/main/java/org/apache/sqoop/repository/Repository.java 03989a3c053edfc51364e9fb368ec970c1dfecee

>   core/src/main/java/org/apache/sqoop/repository/RepositoryEncryptionManager.java PRE-CREATION

>   core/src/main/java/org/apache/sqoop/security/SecurityConstants.java 0241c8692fef65866160e0bec53cc5c2da1c17e5

>   core/src/main/java/org/apache/sqoop/security/SecurityError.java 988e425a041a5829ad95508a1e9b20d0992c868e

>   core/src/test/java/org/apache/sqoop/repository/TestRepositoryEncryptionManager.java
PRE-CREATION 
>   dev-support/upload-patch.py 5c17e587bd64bc8fa98ae90058d99da2bd4230a5 
>   dist/src/main/conf/sqoop.properties 767d3f2b3ddf8ca978830e37a321d9defbafc57d 
>   docs/src/site/sphinx/user/examples/S3Import.rst d75b23e06da29f16ffe35d4e18043e10add8ce62

>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
15cc41b83adc59c266cb899a19edfcc5fc2ee343 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
ae16b8517daf4d40fd2ebf4a473e16e9083740d6 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java
d1940e82c02039c144c58a80b3da07f6d2e38b27 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
ee5e8d10f16365fa55574a819501b6ea48ffe30a 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaCreateQuery.java
177003671863244a094abdb6f4a0184d38e79823 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaUpgradeQuery.java
5081b82ae2cbbd06cef9061b333dd4ec0b61b815 
>   repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java
e4cca07fd062a64ac161929943dbd70b23a01c7f 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java
2c74c323bf64f5c4b120aa745b1e7208e4ddfb3d 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaCreateQuery.java
47f12fe457bc84fc36444e9aeb0b5fbc5a16356f 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MysqlRepositoryInsertUpdateDeleteSelectQuery.java
4c295c017195bd3b5ee79a6961814f3d8c8f9e17 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
400d706c76bbb3e45f535556e925d1debf557928 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java
8358df0d90ef31fad02cf48df198f5c263a3915e 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaUpgradeQuery.java
52954e6946772d4cff268d88aa0bfbc4a9d11aa9 
>   test/src/main/java/org/apache/sqoop/test/minicluster/SqoopMiniCluster.java c7a4db871c2e147f81455585aaeef71660c45922

>   test/src/test/java/org/apache/sqoop/integration/repository/derby/upgrade/DerbyRepositoryUpgradeTest.java
44f4a5bcc3bfd45bcfd3c5a8fecb665b6a34b450 
> 
> Diff: https://reviews.apache.org/r/44984/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message