Details

      Description

      In DFSClient, we need to decrypt EDEK before creating CryptoInputStream/CryptoOutputStream, currently edek is used directly.

      1. hdfs-6724.001.patch
        13 kB
        Andrew Wang
      2. hdfs-6724.002.patch
        13 kB
        Andrew Wang
      3. hdfs-6724.003.patch
        11 kB
        Andrew Wang

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open In Progress In Progress
        2d 13h 44m 1 Andrew Wang 24/Jul/14 23:10
        In Progress In Progress Resolved Resolved
        20h 23m 1 Andrew Wang 25/Jul/14 19:33
        Andrew Wang made changes -
        Status In Progress [ 3 ] Resolved [ 5 ]
        Fix Version/s fs-encryption (HADOOP-10150 and HDFS-6134) [ 12326852 ]
        Resolution Fixed [ 1 ]
        Andrew Wang made changes -
        Attachment hdfs-6724.003.patch [ 12657876 ]
        Hide
        Andrew Wang added a comment -

        Thanks again for the reviews guys, attaching a final patch removing the KPCE change. Committed this to the branch.

        Show
        Andrew Wang added a comment - Thanks again for the reviews guys, attaching a final patch removing the KPCE change. Committed this to the branch.
        Hide
        Yi Liu added a comment -

        Look good to me, +1, Thanks Andrew.

        Show
        Yi Liu added a comment - Look good to me, +1, Thanks Andrew.
        Hide
        Charles Lamb added a comment -

        +1. Thanks Andrew.

        Show
        Charles Lamb added a comment - +1. Thanks Andrew.
        Andrew Wang made changes -
        Attachment hdfs-6724.002.patch [ 12657726 ]
        Hide
        Andrew Wang added a comment -

        New patch attached, still incorporating changes from HADOOP-10891 for now (git mirror hasn't updated). Fixed Charles' comments, thanks for reviewing Charles.

        Show
        Andrew Wang added a comment - New patch attached, still incorporating changes from HADOOP-10891 for now (git mirror hasn't updated). Fixed Charles' comments, thanks for reviewing Charles.
        Hide
        Charles Lamb added a comment -

        Hi Andrew Wang,

        I only have a few little nits. In general I'm +1, but I'd like to hear what Yi has to say.

        DFSUtil.java:

        @throws java.io.IOException.
        

        You don't need java.io. since it's imported.

        KeyProviderCryptoExtension.java:

             * @param encryptedKeyIv           Initialization vector of the encrypted
             *                                 key. The IV of the encryption key used to
             *                                 encrypt the encrypted key is derived from
             *                                 this IV.
        

        In this comment would it be possible to add the word "data" as in "data encryption key" to help clarify the difference between the two keys? I realize you've already got "encrypted" and "encryption", but that's a subtle difference and likely to be lost on an unfamiliar reader.

        TestEncryptionZones.java:

        I don't see a lot of System.out.printlns in unit tests. I suppose it's because it's harder to find the output. Would it be more vogue to use logging?

        Show
        Charles Lamb added a comment - Hi Andrew Wang , I only have a few little nits. In general I'm +1, but I'd like to hear what Yi has to say. DFSUtil.java: @ throws java.io.IOException. You don't need java.io. since it's imported. KeyProviderCryptoExtension.java: * @param encryptedKeyIv Initialization vector of the encrypted * key. The IV of the encryption key used to * encrypt the encrypted key is derived from * this IV. In this comment would it be possible to add the word "data" as in "data encryption key" to help clarify the difference between the two keys? I realize you've already got "encrypted" and "encryption", but that's a subtle difference and likely to be lost on an unfamiliar reader. TestEncryptionZones.java: I don't see a lot of System.out.printlns in unit tests. I suppose it's because it's harder to find the output. Would it be more vogue to use logging?
        Andrew Wang made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Andrew Wang made changes -
        Attachment hdfs-6724.001.patch [ 12657688 ]
        Hide
        Andrew Wang added a comment -

        Sorry about the delay here, had to do some KeyProvider API improvements along the way. Still waiting on HADOOP-10981 to be committed to trunk (the EncryptedKeyVersion factory method), but you can look at this combined patch for an idea.

        Show
        Andrew Wang added a comment - Sorry about the delay here, had to do some KeyProvider API improvements along the way. Still waiting on HADOOP-10981 to be committed to trunk (the EncryptedKeyVersion factory method), but you can look at this combined patch for an idea.
        Yi Liu made changes -
        Field Original Value New Value
        Assignee Andrew Wang [ andrew.wang ]
        Yi Liu created issue -

          People

          • Assignee:
            Andrew Wang
            Reporter:
            Yi Liu
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development