You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/09/07 12:33:00 UTC

[GitHub] [apisix] tokers opened a new pull request #2179: bugfix: supported the encryption/decryption for multi priv keys

tokers opened a new pull request #2179:
URL: https://github.com/apache/apisix/pull/2179


   ### What this PR does / why we need it:
   
   Previously I added the support for setting multiple certs and pkeys for a single domain, but the encryption for private keys  was forgotten, so here i fixed it.
   
   ### Pre-submission checklist:
   
   * [x] Did you explain what problem does this PR solve? Or what new features have been added?
   * [x] Have you added corresponding test cases?
   * [x] Have you modified the corresponding document?
   * [x] Is this PR backward compatible?
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #2179: bugfix: supported the encryption/decryption for multi priv keys

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #2179:
URL: https://github.com/apache/apisix/pull/2179#discussion_r489240353



##########
File path: apisix/http/router/radixtree_sni.lua
##########
@@ -95,14 +109,22 @@ local function create_router(ssl_items)
             end
 
             -- decrypt private key
-            if aes_128_cbc_with_iv ~= nil and
-                not core.string.has_prefix(ssl.value.key, "---") then
-                local decrypted = aes_128_cbc_with_iv:decrypt(ngx_decode_base64(ssl.value.key))
-                if decrypted == nil then
-                    core.log.error("decrypt ssl key failed. key[", ssl.value.key, "] ")
-                else
+            if aes_128_cbc_with_iv ~= nil then
+                local decrypted = decrypt_priv_pkey(aes_128_cbc_with_iv,

Review comment:
       sure.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers removed a comment on pull request #2179: bugfix: supported the encryption/decryption for multi priv keys

Posted by GitBox <gi...@apache.org>.
tokers removed a comment on pull request #2179:
URL: https://github.com/apache/apisix/pull/2179#issuecomment-693219910


   @moonming OK.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] moonming commented on pull request #2179: bugfix: supported the encryption/decryption for multi priv keys

Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #2179:
URL: https://github.com/apache/apisix/pull/2179#issuecomment-693203064


   @tokers please rebase master for this PR, thx


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] membphis commented on a change in pull request #2179: bugfix: supported the encryption/decryption for multi priv keys

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2179:
URL: https://github.com/apache/apisix/pull/2179#discussion_r489233605



##########
File path: apisix/http/router/radixtree_sni.lua
##########
@@ -62,6 +62,20 @@ local function parse_pem_priv_key(sni, pkey)
 end
 
 
+local function decrypt_priv_pkey(iv, key)
+    if str_find(key, "---") then

Review comment:
       should we use `core.string.has_prefix` here?

##########
File path: apisix/http/router/radixtree_sni.lua
##########
@@ -95,14 +109,22 @@ local function create_router(ssl_items)
             end
 
             -- decrypt private key
-            if aes_128_cbc_with_iv ~= nil and
-                not core.string.has_prefix(ssl.value.key, "---") then
-                local decrypted = aes_128_cbc_with_iv:decrypt(ngx_decode_base64(ssl.value.key))
-                if decrypted == nil then
-                    core.log.error("decrypt ssl key failed. key[", ssl.value.key, "] ")
-                else
+            if aes_128_cbc_with_iv ~= nil then
+                local decrypted = decrypt_priv_pkey(aes_128_cbc_with_iv,

Review comment:
       the `ssl.value.key` maybe nil. how about this code style?
   
   ```lua
   if ssl.value.key then
      ...
   end
   
   if ssl.value.keys then
       ...
   end
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on pull request #2179: bugfix: supported the encryption/decryption for multi priv keys

Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2179:
URL: https://github.com/apache/apisix/pull/2179#issuecomment-693767787


   > do we need to add some new test cases?
   
   Will supply some test cases.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on pull request #2179: bugfix: supported the encryption/decryption for multi priv keys

Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2179:
URL: https://github.com/apache/apisix/pull/2179#issuecomment-693219910


   @moonming OK.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on pull request #2179: bugfix: supported the encryption/decryption for multi priv keys

Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2179:
URL: https://github.com/apache/apisix/pull/2179#issuecomment-693221215


   @moonming Rebased.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] moonming merged pull request #2179: bugfix: supported the encryption/decryption for multi priv keys

Posted by GitBox <gi...@apache.org>.
moonming merged pull request #2179:
URL: https://github.com/apache/apisix/pull/2179


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] moonming merged pull request #2179: bugfix: supported the encryption/decryption for multi priv keys

Posted by GitBox <gi...@apache.org>.
moonming merged pull request #2179:
URL: https://github.com/apache/apisix/pull/2179


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #2179: bugfix: supported the encryption/decryption for multi priv keys

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #2179:
URL: https://github.com/apache/apisix/pull/2179#discussion_r489240639



##########
File path: apisix/http/router/radixtree_sni.lua
##########
@@ -62,6 +62,20 @@ local function parse_pem_priv_key(sni, pkey)
 end
 
 
+local function decrypt_priv_pkey(iv, key)
+    if str_find(key, "---") then

Review comment:
       OK, actually i just copy and paste the original codes.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org