{Storage} Validate blob copy source endpoint (#34138)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: YangAn-microsoft

src/azure-cli/azure/cli/command_modules/storage/_params.py

@@ -1396,6 +1396,7 @@ def load_arguments(self, _):  # pylint: disable=too-many-locals, too-many-statem
         c.register_precondition_options()
         c.register_precondition_options(prefix='source_')
         c.register_source_uri_arguments(validator=validate_source_url)
+        c.argument('source_is_validated_same_account', ignore_type)
 
         c.ignore('incremental_copy')
         c.argument('if_match', options_list=['--destination-if-match'])

src/azure-cli/azure/cli/command_modules/storage/_validators.py

@@ -43,6 +43,11 @@ def _query_account_key(cli_ctx, account_name):
     return scf.storage_accounts.list_keys(rg, account_name, logging_enable=False).keys_property[0].value  # pylint: disable=no-member
 
 
+def _is_valid_storage_account_name(account_name):
+    import re
+    return bool(account_name and re.fullmatch(r'[a-z0-9]{3,24}', account_name))
+
+
 def _query_account_rg(cli_ctx, account_name):
     """Query the storage account's resource group, which the mgmt sdk requires."""
     scf = storage_client_factory(cli_ctx)
@@ -422,6 +427,7 @@ def validate_source_url(cmd, namespace):  # pylint: disable=too-many-statements,
         '\n\tOR --source-share --source-path [--source-account-name & key]'
 
     ns = vars(namespace)
+    ns['source_is_validated_same_account'] = False
 
     # source as blob
     container = ns.pop('source_container', None)
@@ -487,6 +493,9 @@ def validate_source_url(cmd, namespace):  # pylint: disable=too-many-statements,
             except ValueError:
                 raise RequiredArgumentMissingError('Source storage account {} not found.'.format(source_account_name))
 
+    ns['source_is_validated_same_account'] = bool(
+        valid_blob_source and same_account and _is_valid_storage_account_name(source_account_name))
+
     # if oauth, use user delegation key to generate sas
     source_user_delegation_key = None
     if is_oauth:

src/azure-cli/azure/cli/command_modules/storage/operations/blob.py

@@ -1103,14 +1103,59 @@ def query_blob(client, query_expression, input_config=None, output_config=None,
     return reader.readall().decode("utf-8")
 
 
+def _normalize_url_origin(url):
+    from ipaddress import ip_address
+    from urllib.parse import urlsplit
+    import idna
+
+    try:
+        parsed = urlsplit(url)
+        scheme = parsed.scheme.lower()
+        hostname = parsed.hostname
+        if scheme not in ('http', 'https') or not hostname:
+            return None
+
+        hostname = hostname.rstrip('.')
+        if not hostname:
+            return None
+        try:
+            hostname = ip_address(hostname).compressed
+        except ValueError:
+            hostname = idna.encode(hostname, uts46=True).decode('ascii').lower().rstrip('.')
+            if not hostname:
+                return None
+
+        port = parsed.port
+        if port is None:
+            port = 443 if scheme == 'https' else 80
+        return scheme, hostname, port
+    except (UnicodeError, ValueError):
+        return None
+
+
+def _same_url_origin(first_url, second_url):
+    first_origin = _normalize_url_origin(first_url)
+    return first_origin is not None and first_origin == _normalize_url_origin(second_url)
+
+
+def _account_names_match(first_client, second_client):
+    first_account = first_client.account_name
+    second_account = second_client.account_name
+    return bool(first_account and second_account and first_account.lower() == second_account.lower())
+
+
 def copy_blob(cmd, client, source_url, metadata=None, **kwargs):
     if not kwargs['requires_sync']:
         kwargs.pop('requires_sync')
     blob_type = kwargs.pop('destination_blob_type', None)
+    source_is_validated_same_account = kwargs.pop('source_is_validated_same_account', False)
     src_client = kwargs.pop('source_client', None)
     if src_client is None:
         src_client = client.from_blob_url(source_url)
-        if src_client.account_name == client.account_name:
+        source_matches_destination = _same_url_origin(source_url, client.url) and \
+            _account_names_match(src_client, client)
+        can_reuse_destination_credential = source_is_validated_same_account or source_matches_destination
+        if can_reuse_destination_credential:
             src_client = client.from_blob_url(source_url, credential=client.credential)
     StandardBlobTier = cmd.get_models('_models#StandardBlobTier')
     if blob_type is not None and blob_type != 'Detect':

src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_copy_scenarios.py

@@ -3,10 +3,192 @@
 # Licensed under the MIT License. See License.txt in the project root for license information.
 # --------------------------------------------------------------------------------------------
 
+import base64
+from unittest import TestCase, mock
+
+from azure.storage.blob import BlobServiceClient
+
 from azure.cli.testsdk import LiveScenarioTest, ResourceGroupPreparer, StorageAccountPreparer, JMESPathCheck
+from ...operations import blob as blob_operations
 from ..storage_test_util import StorageScenarioMixin
 
 
+class StorageBlobCopySecurityTests(TestCase):
+    def test_storage_blob_copy_preserves_source_sas_for_custom_endpoint(self):
+        account_key = base64.b64encode(b'account-key').decode()
+        service_client = BlobServiceClient(
+            account_url='https://storage.internal.example',
+            credential={'account_name': 'account', 'account_key': account_key})
+        destination_client = service_client.get_blob_client(container='dst', blob='output')
+        source_url = 'https://storage.internal.example/src/input?sp=r&sig=existing'
+
+        self.assertIsNone(destination_client.from_blob_url(source_url).account_name)
+
+        with mock.patch.object(destination_client, 'upload_blob_from_url') as upload_blob_from_url:
+            blob_operations.copy_blob(
+                mock.MagicMock(), destination_client, source_url,
+                requires_sync=False, destination_blob_type='BlockBlob')
+
+        upload_blob_from_url.assert_called_once_with(
+            source_url=source_url, overwrite=True, tags=None, destination_lease=None,
+            standard_blob_tier=None, source_if_modified_since=None, source_if_unmodified_since=None,
+            if_modified_since=None, if_unmodified_since=None, timeout=None)
+
+    def test_storage_blob_copy_source_origin_normalization(self):
+        equivalent_origins = [
+            ('https://account.blob.core.windows.net/container/source',
+             'https://ACCOUNT.blob.core.windows.net:443/container/destination'),
+            ('https://account.blob.core.usgovcloudapi.net./container/source',
+             'https://account.blob.core.usgovcloudapi.net/container/destination'),
+            ('https://account.blob.core.windows.net。/container/source',
+             'https://account.blob.core.windows.net/container/destination'),
+            ('https://xn--bcher-kva.example/container/source',
+             'https://bücher.example:443/container/destination'),
+            ('http://127.0.0.1:10000/account/container/source',
+             'http://127.0.0.1:10000/account/container/destination'),
+            ('http://[::1]:10000/account/container/source',
+             'http://[0:0:0:0:0:0:0:1]:10000/account/container/destination'),
+        ]
+        for source_url, destination_url in equivalent_origins:
+            with self.subTest(source_url=source_url, destination_url=destination_url):
+                self.assertTrue(blob_operations._same_url_origin(source_url, destination_url))
+
+        different_origins = [
+            ('https://account.blob.core.windows.net.attacker.example/container/source',
+             'https://account.blob.core.windows.net/container/destination'),
+            ('https://account.blob.core.windows.net@attacker.example/container/source',
+             'https://account.blob.core.windows.net/container/destination'),
+            ('http://account.blob.core.windows.net/container/source',
+             'https://account.blob.core.windows.net/container/destination'),
+            ('https://account.blob.core.windows.net:444/container/source',
+             'https://account.blob.core.windows.net/container/destination'),
+            ('https://faß.example/container/source',
+             'https://fass.example/container/destination'),
+            ('https://account.blob.core.windows.net/container/source',
+             'https://custom.example/container/destination'),
+            ('not-a-url', 'https://account.blob.core.windows.net/container/destination'),
+        ]
+        for source_url, destination_url in different_origins:
+            with self.subTest(source_url=source_url, destination_url=destination_url):
+                self.assertFalse(blob_operations._same_url_origin(source_url, destination_url))
+
+    @mock.patch.object(blob_operations, 'generate_sas_blob_uri')
+    def test_storage_blob_copy_does_not_reuse_credentials_for_untrusted_source(self, generate_sas):
+        source_url = 'https://account.blob.core.windows.net.attacker.example/container/source'
+
+        for blob_type in ('BlockBlob', 'AppendBlob', 'PageBlob'):
+            with self.subTest(blob_type=blob_type):
+                destination_client = mock.MagicMock()
+                destination_client.account_name = 'account'
+                destination_client.url = 'https://account.blob.core.windows.net/container/destination'
+                destination_client.credential = mock.sentinel.destination_credential
+
+                source_client = mock.MagicMock()
+                source_client.account_name = 'account'
+                source_client.get_blob_properties.return_value.size = 512
+                source_service_client = source_client._get_container_client.return_value. \
+                    _get_blob_service_client.return_value
+                source_service_client.credential = None
+                destination_client.from_blob_url.return_value = source_client
+
+                cmd = mock.MagicMock()
+                blob_operations.copy_blob(cmd, destination_client, source_url,
+                                          requires_sync=False, destination_blob_type=blob_type)
+
+                self.assertTrue(all('credential' not in call.kwargs
+                                    for call in destination_client.from_blob_url.call_args_list))
+                generate_sas.assert_not_called()
+                generate_sas.reset_mock()
+
+    @mock.patch.object(blob_operations, 'generate_sas_blob_uri')
+    def test_storage_blob_copy_reuses_credentials_for_same_source_origin(self, generate_sas):
+        source_url = 'https://ACCOUNT.blob.core.windows.net:443/container/source'
+        signed_source_url = source_url + '?sig=generated'
+        generate_sas.return_value = signed_source_url
+
+        destination_client = mock.MagicMock()
+        destination_client.account_name = 'account'
+        destination_client.url = 'https://account.blob.core.windows.net/container/destination'
+        destination_client.credential = mock.MagicMock(account_key='key')
+
+        anonymous_source_client = mock.MagicMock()
+        anonymous_source_client.account_name = 'ACCOUNT'
+        authenticated_source_client = mock.MagicMock()
+        authenticated_source_service = authenticated_source_client._get_container_client.return_value. \
+            _get_blob_service_client.return_value
+        authenticated_source_service.credential = destination_client.credential
+        destination_client.from_blob_url.side_effect = [anonymous_source_client, authenticated_source_client]
+
+        cmd = mock.MagicMock()
+        blob_operations.copy_blob(cmd, destination_client, source_url,
+                                  requires_sync=False, destination_blob_type='BlockBlob')
+
+        destination_client.from_blob_url.assert_has_calls([
+            mock.call(source_url),
+            mock.call(source_url, credential=destination_client.credential)
+        ])
+        generate_sas.assert_called_once()
+        destination_client.upload_blob_from_url.assert_called_once_with(
+            source_url=signed_source_url, overwrite=True, tags=None, destination_lease=None,
+            standard_blob_tier=None, source_if_modified_since=None, source_if_unmodified_since=None,
+            if_modified_since=None, if_unmodified_since=None, timeout=None)
+
+    @mock.patch.object(blob_operations, 'generate_sas_blob_uri')
+    def test_storage_blob_copy_reuses_credentials_for_validated_same_account_source(self, generate_sas):
+        source_url = 'https://account.blob.core.windows.net/src/input'
+        signed_source_url = source_url + '?sig=generated'
+        generate_sas.return_value = signed_source_url
+
+        destination_client = mock.MagicMock()
+        destination_client.url = 'https://storage.internal.example/dst/output'
+        destination_client.credential = mock.MagicMock(account_key='key')
+
+        anonymous_source_client = mock.MagicMock()
+        authenticated_source_client = mock.MagicMock()
+        source_service_client = authenticated_source_client._get_container_client.return_value. \
+            _get_blob_service_client.return_value
+        source_service_client.credential = destination_client.credential
+        destination_client.from_blob_url.side_effect = [anonymous_source_client, authenticated_source_client]
+
+        cmd = mock.MagicMock()
+        blob_operations.copy_blob(
+            cmd, destination_client, source_url, source_is_validated_same_account=True,
+            requires_sync=False, destination_blob_type='BlockBlob')
+
+        destination_client.from_blob_url.assert_has_calls([
+            mock.call(source_url),
+            mock.call(source_url, credential=destination_client.credential)
+        ])
+        generate_sas.assert_called_once()
+        destination_client.upload_blob_from_url.assert_called_once_with(
+            source_url=signed_source_url, overwrite=True, tags=None, destination_lease=None,
+            standard_blob_tier=None, source_if_modified_since=None, source_if_unmodified_since=None,
+            if_modified_since=None, if_unmodified_since=None, timeout=None)
+
+    @mock.patch.object(blob_operations, 'generate_sas_blob_uri')
+    def test_storage_blob_copy_does_not_reuse_credentials_across_path_style_accounts(self, generate_sas):
+        source_url = 'http://127.0.0.1:10000/account2/src/input'
+
+        destination_client = mock.MagicMock()
+        destination_client.account_name = 'account1'
+        destination_client.url = 'http://127.0.0.1:10000/account1/dst/output'
+        destination_client.credential = mock.sentinel.destination_credential
+
+        source_client = mock.MagicMock()
+        source_client.account_name = 'account2'
+        source_service_client = source_client._get_container_client.return_value. \
+            _get_blob_service_client.return_value
+        source_service_client.credential = None
+        destination_client.from_blob_url.return_value = source_client
+
+        cmd = mock.MagicMock()
+        blob_operations.copy_blob(
+            cmd, destination_client, source_url, requires_sync=False, destination_blob_type='BlockBlob')
+
+        destination_client.from_blob_url.assert_called_once_with(source_url)
+        generate_sas.assert_not_called()
+
+
 class StorageBlobCopyTests(StorageScenarioMixin, LiveScenarioTest):
     @ResourceGroupPreparer()
     @StorageAccountPreparer(parameter_name='source_account')

src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_validators.py

@@ -19,6 +19,7 @@
                                                            process_blob_source_uri, get_char_options_validator,
                                                            get_source_file_or_blob_service_client_track2,
                                                            validate_encryption_source, validate_source_uri,
+                                                           validate_source_url,
                                                            validate_encryption_services, as_user_validator,
                                                            get_not_none_validator, validate_upload_blob)
 
@@ -162,6 +163,62 @@ def test_validate_source_uri(self):
         validate_source_uri(MockCmd(self.cli), ns)
         self.assertEqual(ns.copy_source, 'https://other_name.file.core.windows.net/share2?some_sas_token')
 
+    @mock.patch('azure.cli.command_modules.storage._validators.validate_client_parameters')
+    def test_validate_source_url_marks_structured_same_account_blob(self, _):
+        ns = Namespace(
+            source_url=None, source_sas=None, source_container='src', source_blob='input',
+            source_snapshot=None, source_share=None, source_path=None, file_snapshot=None,
+            source_account_name='account', source_account_key=None, token_credential=None,
+            account_name='account', account_key='key', account_url='https://storage.internal.example',
+            connection_string=None, sas_token=None, location_mode=None, connection_timeout=None,
+            container_name='dst', share_name=None)
+
+        validate_source_url(MockCmd(self.cli), ns)
+
+        self.assertTrue(ns.source_is_validated_same_account)
+        self.assertEqual(ns.source_url, 'https://account.blob.core.windows.net/src/input')
+
+    def test_validate_source_url_does_not_mark_arbitrary_uri_as_same_account(self):
+        source_url = 'https://account.blob.core.windows.net.attacker.example/src/input'
+        ns = Namespace(
+            source_url=source_url, source_sas=None, source_container=None, source_blob=None,
+            source_snapshot=None, source_share=None, source_path=None, file_snapshot=None,
+            source_account_name=None, source_account_key=None)
+        validate_source_url(MockCmd(self.cli), ns)
+        validate_source_url(MockCmd(self.cli), ns)
+
+        self.assertFalse(ns.source_is_validated_same_account)
+        self.assertEqual(ns.source_url, source_url)
+
+    @mock.patch('azure.cli.command_modules.storage._validators.validate_client_parameters')
+    @mock.patch('azure.cli.command_modules.storage.util.create_short_lived_file_sas_v2', return_value='sas')
+    def test_validate_source_url_does_not_mark_structured_file_as_same_account(self, _, __):
+        ns = Namespace(
+            source_url=None, source_sas=None, source_container=None, source_blob=None,
+            source_snapshot=None, source_share='src', source_path='input', file_snapshot=None,
+            source_account_name='account', source_account_key=None, token_credential=None,
+            account_name='account', account_key='key', sas_token=None,
+            container_name='dst', share_name=None)
+
+        validate_source_url(MockCmd(self.cli), ns)
+
+        self.assertFalse(ns.source_is_validated_same_account)
+
+    @mock.patch('azure.cli.command_modules.storage._validators.validate_client_parameters')
+    def test_validate_source_url_requires_valid_account_name_for_same_account(self, _):
+        for account_name in (None, 'same@attacker.example:443/'):
+            with self.subTest(account_name=account_name):
+                ns = Namespace(
+                    source_url=None, source_sas=None, source_container='src', source_blob='input',
+                    source_snapshot=None, source_share=None, source_path=None, file_snapshot=None,
+                    source_account_name=account_name, source_account_key=None, token_credential=None,
+                    account_name=account_name, account_key='key', sas_token=None,
+                    container_name='dst', share_name=None)
+
+                validate_source_url(MockCmd(self.cli), ns)
+
+                self.assertFalse(ns.source_is_validated_same_account)
+
     def test_get_not_none_validator(self):
         from azure.cli.core.azclierror import InvalidArgumentValueError
         from knack.arguments import CLICommandArgument, CLIArgumentType